[
https://issues.apache.org/jira/browse/TIKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18098606#comment-18098606
]
ASF GitHub Bot commented on TIKA-4793:
--------------------------------------
tballison commented on PR #2962:
URL: https://github.com/apache/tika/pull/2962#issuecomment-5063614163
I concur with my :robot:
```
Thanks for iterating on this — the threaded limit + `PAYLOAD_LIMIT_EXCEEDED`
status read cleanly. Two small doc mismatches and then I think it's good:
**1. `PipesConfig.setMaxIpcPayloadBytes` javadoc.** It says the limit is
configured "on both ends automatically," but in the final design the configured
value is only consulted on the client's read of server responses (`PipesClient`
line ~376). Server-side reads still use the built-in default. Suggest:
```java
/**
* Sets the maximum IPC payload size in bytes. Must be a positive value.
* This bounds the size of a message the client will accept back from the
* forked server (chiefly the FINISHED result). Request payloads
* (client to server) are small and use the built-in default.
*
* @param maxIpcPayloadBytes positive payload limit in bytes
* @throws IllegalArgumentException if the value is not positive
*/
```
**2. `PayloadLimitExceededException` javadoc.** Two things drift here: it's
thrown for the *configured* per-read limit now, not `MAX_PAYLOAD_BYTES`; and
"the server process itself remains healthy" only holds for a shared server —
with the default per-client forked server the process still exits on the broken
write and the client reconnects next task. Suggest:
```java
/**
* Thrown when an incoming IPC payload's declared length exceeds the
configured
* limit (see {@link
org.apache.tika.pipes.core.PipesConfig#getMaxIpcPayloadBytes()};
* default {@link PipesMessage#MAX_PAYLOAD_BYTES}). The payload bytes were
not
* consumed, so the stream is desynchronized and the connection must be
closed.
* With a shared server the process keeps running (only this connection
ends);
* with the default per-client forked server the process may still exit on
the
* failed write, and the client reconnects on the next task.
*/
```
(The same "server is healthy" wording is in the `PipesClient` catch block —
worth trimming there too.)
Heads up, CI will be red until `PipesParsingHelper.mapStatusToHttpResponse`
(tika-server-core) gets the new `PAYLOAD_LIMIT_EXCEEDED` case — it's an
exhaustive `switch` with no default, so the added enum constant breaks its
compile. Adding it to the `INTERNAL_SERVER_ERROR` arm (alongside the other
TASK_EXCEPTION statuses) is the consistent fix.
```
> Make the Pipes IPC max payload size configurable (currently hard-coded to 100
> MB)
> ---------------------------------------------------------------------------------
>
> Key: TIKA-4793
> URL: https://issues.apache.org/jira/browse/TIKA-4793
> Project: Tika
> Issue Type: Improvement
> Components: tika-pipes
> Reporter: Srinivasarao Daruna
> Priority: Major
>
> PipesMessage.MAX_PAYLOAD_BYTES (tika-pipes-core) is a compile-time constant
> set to 100 MB:
> //
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/protocol/PipesMessage.java:44
> public static final int MAX_PAYLOAD_BYTES = 100 * 1024 * 1024;
> This cap is enforced on the read side of every IPC message in the
> PipesClient↔PipesServer socket protocol. It does not limit the file size
> being parsed (files are fetched server-side by a Fetcher); it limits the size
> of the serialized JSON payload — most critically the PipesResult (parsed
> metadata + extracted text) returned in FINISHED messages.
> Problems with the current implementation:
> 1. Hard-coded, not configurable. Users with very large documents that produce
> large parse results (and no MetadataWriteLimiterFactory configured) have no
> way to raise the cap short of forking the code. There is no corresponding
> field in PipesConfig.
> 2. No write-side guard. PipesMessage.write() applies no limit before writing.
> When the server serializes a PipesResult exceeding 100 MB and sends it, the
> client's PipesMessage.read() throws IOException("Payload length X exceeds
> maximum of 104857600 bytes"). This is caught by the catch-all Exception block
> in PipesClient.waitForServer() and surfaced to the caller as
> UNSPECIFIED_CRASH — a misleading status that provides no indication of the
> root cause.
> Proposed fix:
> 1. Add maxIpcPayloadBytes to PipesConfig with a default of 100 * 1024 * 1024,
> loaded from the "pipes" JSON config section (consistent with all other
> PipesConfig fields).
> 2. Thread the configured value through to both PipesMessage.read() and
> PipesMessage.write(), replacing the hard-coded constant.
> 3. Add a write-side guard in PipesMessage.write() so oversized results are
> caught server-side with a descriptive IOException rather than failing
> silently at the client with UNSPECIFIED_CRASH.
> Example config (proposed):
> {
> "pipes": {
> "maxIpcPayloadBytes": 209715200
> }
> }
> Note: Users hitting this limit should first consider configuring a
> MetadataWriteLimiterFactory to bound extracted-text size, which is the right
> long-term solution for very large documents. But the limit should still be
> configurable for cases where the full content is legitimately needed.
> Affected files:
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/protocol/PipesMessage.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PipesConfig.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PipesClient.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/PipesServer.java
--
This message was sent by Atlassian Jira
(v8.20.10#820010)