TIKA-4793: make Pipes IPC payload limit configurable

[srujana-kuntumalla]

The hard-coded 100 MB ceiling in PipesMessage was not operator-tunable. Add PipesConfig.maxIpcPayloadBytes (default 100 MB) and thread it through PipesClient, PipesServer, ConnectionHandler, and ServerProtocolIO so the limit is applied on every read() call. The write path is unchanged. Includes unit tests for default value, JSON loading, and validation.

Thanks for your contribution to Apache Tika! Your help is appreciated!

Before opening the pull request, please verify that

• there is an open issue on the Tika issue tracker which describes the problem or the improvement. We cannot accept pull requests without an issue because the change wouldn't be listed in the release notes.
• the issue ID (TIKA-XXXX)
  • is referenced in the title of the pull request
  • and placed in front of your commit messages surrounded by square brackets ([TIKA-XXXX] Issue or pull request title)
• commits are squashed into a single one (or few commits for larger changes)
• Tika is successfully built and unit tests pass by running ./mvnw clean test
• there should be no conflicts when merging the pull request branch into the recent main branch. If there are conflicts, please try to rebase the pull request branch on top of a freshly pulled main branch
• if you add new module that downstream users will depend upon add it to relevant group in tika-bom/pom.xml.

We will be able to faster integrate your pull request if these conditions are met. If you have any questions how to fix your problem or about using Tika in general, please sign up for the Tika mailing list. Thanks!

[srujana-kuntumalla]

@tballison @THausherr Can you please review this PR? TIA

[tballison]

My agent recommends these four small changes.

1. Kill the duplicated constant. Change PipesConfig.DEFAULT_MAX_IPC_PAYLOAD_BYTES = 100L * 1024 * 1024 to reference PipesMessage.MAX_PAYLOAD_BYTES
  directly, so the "must stay equal" invariant is compiler-enforced instead of comment-enforced. (Do before merge.)
  2. Add a JSON-invalid-value load test. Load {"pipes":{"maxIpcPayloadBytes":0}} through PipesConfig.load and assert it throws. This locks in that
  Jackson binds via the validating setter — behavior that's otherwise invisible and would silently break if the mapper's visibility config ever
  changed. (Do before merge.)
  3. Unify how the limit is accessed. PipesClient caches it into a private final field; ConnectionHandler and PipesServer call
  pipesConfig.getMaxIpcPayloadBytes() inline. Pick one for consistency (dropping the field matches the other two). (Polish.)
  4. Add a one-line javadoc note that the limit is a hard, connection-terminating check that must be configured identically on both ends, and is
  startup-only (restart to change). (Polish.)

wdyt?

[tballison]

Other thing is that tika-pipes was designed to emit rather than passing back to the pipesclient? What's your use case where you're passing back that much data?

[tballison]

K, this one is more important. We should surface the source of the problem when a payload is too big, and we don't want to have to restart a pipesserver when this is hit.

Recommendation this surfaces (I'd promote it above my #3/#4): give the limit breach a distinct, honest status — a TASK_EXCEPTION-category value like
  OUTPUT_TOO_LARGE / PAYLOAD_LIMIT_EXCEEDED — caught specifically in PipesClient so that: the client gets a clear, actionable error with the size in
  it, the server is not restarted, and tika-server surfaces the reason in the response body. Right now the PR makes the ceiling tunable but leaves the
  diagnosis when it trips as a mislabeled crash. Worth raising with the author — arguably the change isn't complete without it.

[srujana-kuntumalla]

I have updated the PR to keep the changes minimal to allow max payload size to be configurable.

[tballison]

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.

[srujana-kuntumalla]

Appreciate the feedback! I have updated the PR with threaded limit and PAYLOAD_LIMIT_EXCEEDED status

[tballison]

I concur with my 🤖

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:

/**
 * 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.

[srujana-kuntumalla]

@tballison @THausherr can i please get another review on this PR?