[ 
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} -&gt; {@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)

Reply via email to