[
https://issues.apache.org/jira/browse/TIKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104084#comment-18104084
]
ASF GitHub Bot commented on TIKA-4793:
--------------------------------------
nddipiazza commented on code in PR #3009:
URL: https://github.com/apache/tika/pull/3009#discussion_r3767328069
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ServerProtocolIO.java:
##########
@@ -51,20 +53,75 @@ public class ServerProtocolIO {
private final DataInputStream input;
private final DataOutputStream output;
+ private final int maxPayloadBytes;
- public ServerProtocolIO(DataInputStream input, DataOutputStream output) {
+ public ServerProtocolIO(DataInputStream input, DataOutputStream output,
int maxPayloadBytes) {
this.input = input;
this.output = output;
+ this.maxPayloadBytes = maxPayloadBytes;
}
/**
* Writes a FINISHED message with the serialized result and waits for ACK.
+ * <p>
+ * Three-layer protection against oversized payloads:
+ * <ol>
+ * <li>Pre-check: if the estimated content size already exceeds the
limit, skip
+ * serialization entirely (prevents OOM for very large
documents).</li>
+ * <li>OOM catch: if serialization exhausts heap despite the pre-check
(e.g. when
+ * the limit is uncapped or the estimate is imprecise), the error is
caught and
+ * a lightweight PAYLOAD_LIMIT_EXCEEDED result is returned instead
of crashing.</li>
+ * <li>Post-check: if the serialized byte count exceeds the limit, the
oversized
+ * bytes are discarded before touching the wire, avoiding stream
desynchronization
+ * on the client side.</li>
+ * </ol>
*
* @throws ShutDownReceivedException if SHUT_DOWN is received instead of
ACK
* @throws IOException on serialization or I/O errors
*/
public void writeFinished(PipesResult pipesResult) throws IOException {
- byte[] bytes = JsonPipesIpc.toBytes(pipesResult);
+ // Pre-check: avoid allocating a huge byte[] when content is obviously
over-limit.
+ // estimateSizeInBytes() uses string.length() bytes (≈ UTF-8 Smile
bytes for ASCII),
+ // so this is a lower-bound estimate — safe to use as an early-exit
gate.
+ if (pipesResult.emitData() instanceof EmitDataImpl emitData) {
+ long estimated = emitData.getEstimatedSizeBytes();
+ if (estimated > maxPayloadBytes) {
+ LOG.warn("Skipping serialization: estimated payload {} bytes
exceeds maxIpcPayloadBytes {}",
+ estimated, maxPayloadBytes);
+ doWritePayloadLimitExceeded(String.format(Locale.ROOT,
+ "Estimated content size %d bytes exceeds IPC limit %d
bytes", estimated, maxPayloadBytes));
+ return;
+ }
+ }
+
+ byte[] bytes;
+ try {
+ bytes = JsonPipesIpc.toBytes(pipesResult);
+ } catch (OutOfMemoryError oom) {
+ // The large byte-builder segments are now GC-eligible; the tiny
error result below
+ // should serialize cleanly even on a depleted heap.
+ LOG.error("OOM during result serialization; returning
PAYLOAD_LIMIT_EXCEEDED", oom);
+ doWritePayloadLimitExceeded("OOM during result serialization: " +
oom.getMessage());
+ return;
+ }
+
+ // Post-check: serialized size may exceed the limit when content is
Unicode-heavy
+ // (the pre-check uses a 1 byte/char estimate; CJK chars use 3 bytes
in UTF-8 Smile).
+ if (bytes.length > maxPayloadBytes) {
+ LOG.warn("Serialized payload {} bytes exceeds maxIpcPayloadBytes
{}; returning PAYLOAD_LIMIT_EXCEEDED",
+ bytes.length, maxPayloadBytes);
+ doWritePayloadLimitExceeded(String.format(Locale.ROOT,
+ "Serialized payload %d bytes exceeds IPC limit %d bytes",
bytes.length, maxPayloadBytes));
+ return;
+ }
+
+ PipesMessage.finished(bytes).write(output);
+ awaitAck();
+ }
+
+ private void doWritePayloadLimitExceeded(String message) throws
IOException {
+ byte[] bytes = JsonPipesIpc.toBytes(
+ new
PipesResult(PipesResult.RESULT_STATUS.PAYLOAD_LIMIT_EXCEEDED, message));
PipesMessage.finished(bytes).write(output);
Review Comment:
The fallback frame also needs to fit `maxPayloadBytes` (or config validation
needs to enforce a minimum that guarantees it will). `setMaxIpcPayloadBytes()`
currently accepts any positive value, so with a small configured limit this
serialized `PAYLOAD_LIMIT_EXCEEDED` result is itself larger than the limit. The
production client then rejects it in `PipesMessage.read(...,
maxIpcPayloadBytes)`, closes the connection, and never receives the clean
status this path promises. `ServerProtocolIOTest.exchange()` masks this by
reading with `Integer.MAX_VALUE`. Please add a test where the client reads the
fallback using the same configured limit and either use a bounded/minimal error
payload or reject limits too small to carry it.
> 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)
