[
https://issues.apache.org/jira/browse/TIKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18098202#comment-18098202
]
ASF GitHub Bot commented on TIKA-4793:
--------------------------------------
tballison commented on PR #2962:
URL: https://github.com/apache/tika/pull/2962#issuecomment-5046676738
I agree with my claude on this one. Seriously, thank you for opening this
and iterating on it.
```
Thanks for iterating. The last two commits trade the threaded read() param
for a mutable global static (PipesMessage.MAX_PAYLOAD_BYTES un-final'd,
set as a side effect of the config setter). I'd push back on that — the
global loses per-config semantics (multiple PipesConfig in one JVM → last
load() wins, so getMaxIpcPayloadBytes() can disagree with what's
enforced), pollutes tests (testMaxIpcPayloadBytesFromJson sets 200 MB and never
resets), and races (non-volatile, read from the IPC loops). The earlier
threaded approach (e2c987f) was verbose but correct. (The long → int switch
is a good call, though.)
Two things still open regardless:
1. Over-limit payloads throw a bare IOException that PipesClient reports
as UNSPECIFIED_CRASH/process_crash — reason dropped from the HTTP response
and the healthy server gets restarted. Deserves a distinct TASK_EXCEPTION
status (e.g. OUTPUT_TOO_LARGE).
2. Add a test that loads {"pipes":{"maxIpcPayloadBytes":0}} via
PipesConfig.load and asserts rejection.
```
> 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)
