[ 
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)

Reply via email to