[
https://issues.apache.org/jira/browse/TIKA-4825?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105988#comment-18105988
]
ASF GitHub Bot commented on TIKA-4825:
--------------------------------------
Copilot commented on code in PR #3039:
URL: https://github.com/apache/tika/pull/3039#discussion_r3814943057
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/PipesWorker.java:
##########
@@ -516,7 +510,33 @@ protected ParseDataOrPipesResult parseFromTuple() throws
TikaException, Interrup
}
}
-
+ /**
+ * Carries the caller-supplied detection hints from the tuple metadata
across the
+ * fresh-metadata boundary into the metadata used for fetch and detection.
+ * <p>
+ * Only the resource name and the {@code Content-Type} soft hint are
carried.
+ * {@code Content-Type} is applied by {@code MimeTypes.detect} via {@code
applyHint},
+ * which keeps it only when it equals or specializes the magic-detected
type (e.g.
+ * {@code image/tiff} -> {@code image/x-raw-nikon} for a NEF supplied
without a
+ * 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.
+ *
+ * @param tupleMetadata the caller-supplied metadata (may be null)
+ * @param target the fresh metadata used for fetch and detection
+ */
+ static void carryCallerHints(Metadata tupleMetadata, Metadata target) {
+ if (tupleMetadata == null) {
+ return;
+ }
+ 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);
+ }
Review Comment:
Consider normalizing the carried hints before setting them on `target`. As
written, leading/trailing whitespace (or other non-canonical formatting) will
be preserved, which can reduce the effectiveness of detection hints. A concrete
improvement is to trim the values (e.g., use a trim-to-null approach) before
`target.set(...)`; for `Content-Type`, optionally parse/validate and only carry
it when it parses as a media type.
##########
tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/server/PipesWorkerCallerHintsTest.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.pipes.core.server;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.junit.jupiter.api.Test;
+
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TikaCoreProperties;
+
+/**
+ * Unit tests for {@link PipesWorker#carryCallerHints(Metadata, Metadata)},
which carries the
+ * caller-supplied detection hints across the worker's fresh-metadata boundary.
+ */
+public class PipesWorkerCallerHintsTest {
+
+ @Test
+ public void testCarriesResourceNameAndContentType() {
+ Metadata tuple = new Metadata();
+ tuple.set(TikaCoreProperties.RESOURCE_NAME_KEY, "photo.nef");
+ tuple.set(HttpHeaders.CONTENT_TYPE, "image/x-raw-nikon");
+
+ Metadata target = new Metadata();
+ PipesWorker.carryCallerHints(tuple, target);
+
+ assertEquals("photo.nef",
target.get(TikaCoreProperties.RESOURCE_NAME_KEY));
+ assertEquals("image/x-raw-nikon",
target.get(HttpHeaders.CONTENT_TYPE));
+ }
+
+ /**
+ * 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));
Review Comment:
The PR description mentions explicitly not carrying
`CONTENT_TYPE_PARSER_OVERRIDE` as well as `CONTENT_TYPE_USER_OVERRIDE`, but the
tests only assert the user-override key is not propagated. Add a regression
assertion (or a separate test) that
`TikaCoreProperties.CONTENT_TYPE_PARSER_OVERRIDE` is never carried, to lock in
the intended security contract and prevent future regressions.
##########
CHANGES.txt:
##########
@@ -493,6 +493,13 @@ Release 4.0.0 - 8/18/2026
that exceed the limit return PAYLOAD_LIMIT_EXCEEDED instead of causing
heap exhaustion; crash messages are also size-capped (TIKA-4793).
+ * 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).
Review Comment:
The PR description calls out `/unpack/all` in addition to `/unpack`, but the
CHANGES entry lists only `/unpack, /async and /pipes`. If `/unpack/all` is also
affected by the same pipes/worker path, consider updating this entry to include
it (or reword to cover all relevant endpoints consistently).
> tika-pipes drops the caller-supplied Content-Type before detection, so
> /unpack routing needs a filename
> -------------------------------------------------------------------------------------------------------
>
> Key: TIKA-4825
> URL: https://issues.apache.org/jira/browse/TIKA-4825
> Project: Tika
> Issue Type: Improvement
> Reporter: Dominik Schmidt
> Priority: Major
>
> When parsing through tika-pipes (e.g. the tika-server /unpack and /unpack/all
> endpoints, /async, /pipes), the parser is selected by content detection
> inside an out-of-process worker. A caller can influence which parser runs by
> supplying a filename (Content-Disposition / resource name), but supplying the
> correct Content-Type header alone does not work.
> Concrete case: NEF (Nikon raw) is TIFF-based and has no content magic of its
> own; image/x-raw-nikon is defined by a *.nef glob plus sub-class-of
> image/tiff. So by data alone a NEF detects as image/tiff and routes to
> TiffParser; only the filename yields image/x-raw-nikon and routes to the
> dedicated raw parser. A client that streams a NEF with Content-Type:
> image/x-raw-nikon but no filename still gets TiffParser.
> Root cause: PipesWorker.parseFromTuple starts a fresh Metadata for the
> fetch/detect step (deliberately isolated from the caller's tuple metadata,
> which is re-applied only at the very end) and carries only
> TikaCoreProperties.RESOURCE_NAME_KEY across that boundary. The caller's
> Content-Type, which TikaResource.fillMetadata does place into
> HttpHeaders.CONTENT_TYPE (and CONTENT_TYPE_USER_OVERRIDE), is dropped before
> detection. Core detection would actually honor it: MimeTypes.detect applies a
> Content-Type hint via applyHint, keeping it when it equals or specializes the
> magic-detected type. The failure is purely that the hint never reaches the
> worker's detection metadata. The masking is worsened by
> EmitHandler.injectUserMetadata re-writing the caller's Content-Type into the
> output afterwards, so the returned Content-Type reads correct even though the
> wrong parser ran.
> Proposed change: in PipesWorker, carry HttpHeaders.CONTENT_TYPE across the
> fresh-metadata boundary alongside the resource name, as a soft hint.
> Security consideration: carry only the soft hint (HttpHeaders.CONTENT_TYPE),
> NOT CONTENT_TYPE_USER_OVERRIDE / CONTENT_TYPE_PARSER_OVERRIDE. The soft hint
> is constrained by applyHint to types that equal or specialize the
> magic-detected type, so a caller can refine within the hierarchy (image/tiff
> -> image/x-raw-nikon) but cannot force an unrelated type. This grants no more
> routing power than the already-carried filename, and detection overwrites
> HttpHeaders.CONTENT_TYPE with the detected type before parsing, so no parser
> sees an unvalidated caller value. Carrying an override key, by contrast,
> would let a caller bypass detection unconditionally.
> Note: this softens the worker's deliberate metadata isolation for two fields;
> input welcome on whether the soft-hint scope is the right contract.
> Implemented with a unit test covering the carry (resource name + Content-Type
> carried, override key never carried, null/blank no-ops).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)