Skip to content

[TIKA-4825] carry caller Content-Type across the pipes worker boundary as a detection hint - #3039

Open
dschmidt wants to merge 3 commits into
apache:mainfrom
dschmidt:pipes-carry-content-type
Open

[TIKA-4825] carry caller Content-Type across the pipes worker boundary as a detection hint#3039
dschmidt wants to merge 3 commits into
apache:mainfrom
dschmidt:pipes-carry-content-type

Conversation

@dschmidt

@dschmidt dschmidt commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

When parsing through tika-pipes, a caller can steer parser selection by supplying a filename, but supplying the correct Content-Type header alone does not work. This affects every forked-parse endpoint (/tika, /meta, /rmeta, /unpack, /async, /pipes), plus tika-grpc and embedded PipesForkParser.

Root cause: PipesWorker.parseFromTuple builds a fresh Metadata for fetch/detection (deliberately isolated from the caller's tuple metadata) and carries only RESOURCE_NAME_KEY across that boundary. The caller's Content-Type, which TikaResource.fillMetadata does set, is dropped before detection. Core detection would honor it: MimeTypes.detect applies a Content-Type hint via applyHint, keeping it when it equals or specializes the content-detected type.

This change carries HttpHeaders.CONTENT_TYPE across the boundary alongside the resource name, as a soft hint. Example on current main: a plain TIFF sent with Content-Type: image/x-canon-cr2 (a registered sub-class of image/tiff) now refines from image/tiff to image/x-canon-cr2, whereas before the header was ignored. This is independent of #3037; raw formats such as NEF benefit once they are registered as image/tiff sub-types (which #3037 does), but this PR needs nothing from it.

Security: only the soft hint is carried, deliberately not CONTENT_TYPE_USER_OVERRIDE/CONTENT_TYPE_PARSER_OVERRIDE. applyHint keeps the hint only when it equals or specializes the content-detected type, so a caller can refine within the hierarchy but cannot force an unrelated type. For bytes with no magic everything specializes application/octet-stream, so the hint can win there, matching the routing power the filename already had. Detection then overwrites HttpHeaders.CONTENT_TYPE with the detected type before parsing, so no parser sees an unvalidated caller value.

Tests: PipesWorkerCallerHintsTest covers the carry (name and Content-Type carried, override keys never carried, null/blank no-ops) and detection (a specializing Content-Type refines, a non-specializing or garbage one is ignored). The migration guide gains a note on the 3.x hard-override to 4.x soft-hint change.

Behavior change (targeted for 4.0.1)

Because the caller Content-Type now reaches detection, it can change parser selection for any forked-parse request that sends one, wherever the type was previously ignored. Concretely it flips StackTraceTest.testMeta: a truncated application/mock+xml PUT to /meta/Author used to return 404 (truncation handled gracefully as field-not-found); now the explicit Content-Type routes the truncated document to the mock parser, which cannot parse the incomplete XML, so the container exception maps to 422 on the bare-field endpoint. testMeta now asserts 422; a new testMetaNoType keeps the graceful 404 case (no forcing type). Per the discussion thread this lands in 4.0.1.

@dschmidt
dschmidt force-pushed the pipes-carry-content-type branch from 0795864 to 7afec03 Compare August 19, 2026 16:18
@dschmidt
dschmidt marked this pull request as ready for review August 19, 2026 16:19
@THausherr
THausherr requested a lite review from Copilot August 19, 2026 16:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Carries the caller-supplied Content-Type header across the tika-pipes worker metadata boundary as a soft detection hint (alongside the resource name), so parser routing can use client-provided type information even when no filename is supplied.

Changes:

  • Add PipesWorker.carryCallerHints(...) to copy RESOURCE_NAME_KEY and HttpHeaders.CONTENT_TYPE into the worker’s fresh Metadata.
  • Update parseFromTuple to use the new carry method instead of copying only the resource name.
  • Add unit tests to verify carry behavior and non-carry of CONTENT_TYPE_USER_OVERRIDE, plus null/blank handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/PipesWorker.java Introduces centralized logic to carry caller detection hints (name + Content-Type) into worker metadata.
tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/server/PipesWorkerCallerHintsTest.java Adds tests covering which caller hints are (and aren’t) propagated across the boundary.
CHANGES.txt Documents the new soft-hint behavior for pipes-based endpoints.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +531 to +538
String suppliedName = tupleMetadata.get(TikaCoreProperties.RESOURCE_NAME_KEY);
if (!StringUtils.isBlank(suppliedName)) {
target.set(TikaCoreProperties.RESOURCE_NAME_KEY, suppliedName);
}
String suppliedContentType = tupleMetadata.get(HttpHeaders.CONTENT_TYPE);
if (!StringUtils.isBlank(suppliedContentType)) {
target.set(HttpHeaders.CONTENT_TYPE, suppliedContentType);
}
Comment on lines +48 to +59
* The Content-Type is carried only as a soft hint. The unconditional override key
* must never be carried, or a caller could force any type past detection.
*/
@Test
public void testDoesNotCarryUserOverride() {
Metadata tuple = new Metadata();
tuple.set(TikaCoreProperties.CONTENT_TYPE_USER_OVERRIDE, "image/x-raw-nikon");

Metadata target = new Metadata();
PipesWorker.carryCallerHints(tuple, target);

assertNull(target.get(TikaCoreProperties.CONTENT_TYPE_USER_OVERRIDE));
Comment thread CHANGES.txt Outdated
Comment on lines +496 to +501
* Pipes now carries the caller-supplied Content-Type across the worker's
fresh-metadata boundary as a soft detection hint, so /unpack, /async and
/pipes can route on a client Content-Type (e.g. image/x-raw-nikon for a NEF
sent without a filename), not only on the filename. The hint only refines
within the magic-detected type hierarchy; the CONTENT_TYPE_USER_OVERRIDE key
is deliberately not carried (TIKA-4825).
@dschmidt

Copy link
Copy Markdown
Contributor Author

@tballison @THausherr is 4.0.0 already released? This could be a somewhat breaking change that might be worth getting into the major release if you can still cut a rc2

…-Type routing

- also assert CONTENT_TYPE_PARSER_OVERRIDE is never carried
- mention /unpack/all in CHANGES
- testMeta now expects 422 when an explicit Content-Type routes a truncated
  mock doc to the mock parser; new testMetaNoType keeps the graceful 404 case
@tballison

Copy link
Copy Markdown
Contributor

I don't view this as breaking. It is a good catch, but I'm ok w 4.0.1.

Let me know if you disagree.

@dschmidt

Copy link
Copy Markdown
Contributor Author

Just wanted to bring it up, so you can decide because it's a slight behavioral change. Fine with me either way :)

@tballison

Copy link
Copy Markdown
Contributor

claude has some input. let me know what you think.

1. HIGH — the NEF example only works if PR #3037 merges first. At this
     PR's head, image/x-raw-nikon (tika-mimetypes.xml:7234) has no
     sub-class-of image/tiff — the PR description's claim that it does is
     wrong for main today; #3037 is what adds it. Standalone, applyHint
     rejects the hint and a NEF still detects as image/tiff, so the headline
     example in CHANGES and the carryCallerHints javadoc silently does
     nothing. Two reviewers found this independently; I verified both files.
     Fix: merge-order note (or land the sub-class-of here), and an e2e test
     of the real-mime-database refinement case — nothing currently catches
     this.
  2. MEDIUM — CHANGES endpoint list is materially incomplete. Every parse
     endpoint routes through parseFromTuple: /tika, /meta, /rmeta and
     variants (the PR's own testMeta flipping 404→422 proves /meta), plus
     tika-grpc and embedded PipesForkParser users. Listing only /unpack, 
     /unpack/all, /async, /pipes misleads upgraders — Copilot's /unpack/all
     fix was cosmetic; the real gap remains.
  3. MEDIUM — undocumented 3.x→4.x downgrade in the migration doc. In 3.x
     the header was a hard override (CONTENT_TYPE_USER_OVERRIDE); now it's
     refine-only. A 3.x client forcing text/plain onto arbitrary bytes gets
     silently different behavior. migrating-tika-server-4x.adoc:427-430 says
     Content-Type "still influences detection" — true post-PR but hides the
     override→hint change. One paragraph fixes it.
  4. MEDIUM — no negative-path tests for the security boundary. Nothing
     asserts a non-specializing or garbage Content-Type is ignored — the
     exact property the javadoc calls security-critical. A future applyHint
     change could turn the hint into an override with nothing failing. Also:
     for magic-less bytes everything specializes application/octet-stream,
     so the hint effectively wins there — "only refines within the
     hierarchy" is overstated in CHANGES (equal to filename power, so
     acceptable, but say it).
  5. MEDIUM — CHANGES entry sits in the already-rc'd "Release 4.0.0 - 
     8/18/2026" section, which rc1 froze without it (main is now
     4.0.1-SNAPSHOT). If rc1 passes, released 4.0.0 won't contain TIKA-4825
     while CHANGES claims it does. Correct only if you sink rc1 and respin —
     which is exactly what dschmidt is asking you in the PR thread.
  6. LOW — pre-existing, newly exposed: the carried value reaches
     MimeTypes.forName, which registers every distinct syntactically-valid
     unknown type into an unbounded map (slow memory growth under
     adversarial unique types). Pre-existing on direct server endpoints;
     this PR adds the pipes tuple path. Real fix is a non-registering lookup
     in MimeTypes.detect — separate JIRA, not this PR.
  7. LOW — cleanups: fillMetadata still stamps CONTENT_TYPE_USER_OVERRIDE
     (TikaResource.java:263,341) which the worker now deliberately discards
     — dead and misleading, natural companion cleanup. testMeta's 422 body
     is unasserted (old test asserted the body; one assertContains closes
     it). The 4-line call-site comment in parseFromTuple restates the
     javadoc — trim to one line per repo policy.

…st, negative-path test, 4.0.1 CHANGES, migration note

- CHANGES: move entry to a new Release 4.0.1 section (per maintainer call),
  list all forked-parse endpoints, use the image/tiff -> image/x-canon-cr2
  refinement that works on current main (independent of apache#3037), and note the
  no-magic case matches existing filename power
- test: assert a specializing Content-Type refines detection and a
  non-specializing/garbage one is ignored (the security boundary)
- migration guide: note the 3.x hard-override to 4.x soft-hint change
- trim the call-site comment to one line

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/PipesWorker.java:520

  • The Javadoc says only CONTENT_TYPE_USER_OVERRIDE is not carried, but carryCallerHints() also does not carry CONTENT_TYPE_PARSER_OVERRIDE (and the new unit test explicitly asserts that). Update the Javadoc to mention both override keys to avoid misleading future changes.
     * filename). The {@code CONTENT_TYPE_USER_OVERRIDE} key is deliberately NOT carried:
     * it short-circuits detection unconditionally and would let a caller force any type.
     *

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants