TIKA-4763 improve serialization

Author: tballison

tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/serialization/WireRestrictedFetchEmitTupleTest.java

@@ -23,6 +23,8 @@
 
 import java.io.StringReader;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.dataformat.smile.SmileFactory;
 import org.junit.jupiter.api.Test;
 
 import org.apache.tika.metadata.Metadata;
@@ -38,9 +40,9 @@
  */
 public class WireRestrictedFetchEmitTupleTest {
 
-    private static final String EXPLOIT_PARSE_CONTEXT =
+    private static final String WIRE_BLOCKED_PARSE_CONTEXT =
             "\"parse-context\":{\"typed\":{\"external-parser\":{\"config\":{" +
-            "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo pwned\"]," +
+            "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo x\"]," +
             "\"supportedTypes\":[\"text/plain\"]}}}}";
 
     private static String tuple(String parseContextField) {
@@ -52,15 +54,15 @@ private static String tuple(String parseContextField) {
     @Test
     public void pipesEndpointRejectsParserInjection() {
         Exception e = assertThrows(Exception.class,
-                () -> JsonFetchEmitTuple.fromJson(new StringReader(tuple(EXPLOIT_PARSE_CONTEXT))));
+                () -> JsonFetchEmitTuple.fromJson(new StringReader(tuple(WIRE_BLOCKED_PARSE_CONTEXT))));
         assertTrue(root(e).contains("may not be supplied via a request parseContext"),
                 "expected wire-blocked rejection, got: " + root(e));
     }
 
     @Test
     public void asyncEndpointRejectsParserInjection() {
         Exception e = assertThrows(Exception.class,
-                () -> JsonFetchEmitTupleList.fromJson(new StringReader("[" + tuple(EXPLOIT_PARSE_CONTEXT) + "]")));
+                () -> JsonFetchEmitTupleList.fromJson(new StringReader("[" + tuple(WIRE_BLOCKED_PARSE_CONTEXT) + "]")));
         assertTrue(root(e).contains("may not be supplied via a request parseContext"),
                 "expected wire-blocked rejection, got: " + root(e));
     }
@@ -86,6 +88,17 @@ public void ipcRoundTripsSafeTuple() throws Exception {
         assertEquals(t, back);
     }
 
+    @Test
+    public void forkIpcRejectsParserInjection() throws Exception {
+        // The fork-IPC path uses Smile but shares the same restricted FetchEmitTupleDeserializer.
+        byte[] smile = new ObjectMapper(new SmileFactory())
+                .writeValueAsBytes(new ObjectMapper().readTree(tuple(WIRE_BLOCKED_PARSE_CONTEXT)));
+        Exception e = assertThrows(Exception.class,
+                () -> JsonPipesIpc.fromBytes(smile, FetchEmitTuple.class));
+        assertTrue(root(e).contains("may not be supplied via a request parseContext"),
+                "expected wire-blocked rejection at fork IPC, got: " + root(e));
+    }
+
     private static String root(Throwable t) {
         Throwable r = t;
         while (r.getCause() != null && r.getCause() != r) {

tika-serialization/src/main/java/org/apache/tika/serialization/ParseContextUtils.java

@@ -209,8 +209,10 @@ private static boolean resolveArrayConfig(String configName, JsonConfig jsonConf
                             configName + "': " + item);
                 }
 
-                Object component = ComponentInstantiator.instantiate(
-                        typeName, configNode, MAPPER, classLoader);
+                // Check assignability before constructing, so an element of the wrong type is
+                // rejected up front rather than instantiated and later discarded.
+                Object component = ComponentInstantiator.instantiateComponent(
+                        typeName, configNode, MAPPER, classLoader, configInfo.componentInterface());
                 components.add(component);
                 LOG.debug("Instantiated '{}' for '{}'", typeName, configName);
             }

tika-serialization/src/main/java/org/apache/tika/serialization/serdes/ParseContextDeserializer.java

@@ -168,8 +168,8 @@ private static void checkForDuplicateContextKey(String friendlyName,
      * {@code {"pdf-parser":{...}}}) is an inert per-request config that resolveAll skips, so it is
      * allowed and its opaque config subtree is not scanned; every other key is checked and its
      * subtree scanned, refusing any blocked object key at any depth. Bare-string references are not
-     * scanned -- they name a defaults instance with no config. Running before resolution also
-     * covers ExternalParser, which execs from getSupportedTypes().
+     * scanned -- they name a defaults instance with no config. Running before resolution matters
+     * because merely constructing a blocked component can have side effects.
      */
     private static void assertNoBlockedComponents(JsonNode contextNode) throws IOException {
         if (contextNode == null || !contextNode.isObject()) {

tika-serialization/src/test/java/org/apache/tika/serialization/WireRestrictedParseContextTest.java

@@ -65,10 +65,9 @@ public void everyContextKeyInterfaceIsClassifiedExactlyOnce() {
 
     @Test
     public void restrictedRejectsTypedParserInjection() {
-        // The historical RCE payload (external-parser under a "typed" wrapper); the nested scan
-        // refuses it regardless of the wrapper key.
+        // A blocked component nested under a wrapper key must be refused regardless of the wrapper.
         String json = "{\"typed\":{\"external-parser\":{\"config\":{" +
-                "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo pwned\"]," +
+                "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo x\"]," +
                 "\"supportedTypes\":[\"text/plain\"]}}}}";
         Exception e = assertThrows(Exception.class,
                 () -> restrictedMapper().readValue(json, ParseContext.class));
@@ -90,7 +89,7 @@ public void restrictedAllowsFlatSelfConfiguringParserConfig() throws Exception {
     public void restrictedRejectsNestedParserObject() {
         // A blocked parser nested inside an allowed component's config (compact object form).
         String json = "{\"basic-content-handler-factory\":{\"nested\":{\"external-parser\":{" +
-                "\"config\":{\"commandLine\":[\"/bin/sh\",\"-c\",\"echo pwned\"]}}}}}";
+                "\"config\":{\"commandLine\":[\"/bin/sh\",\"-c\",\"echo x\"]}}}}}";
         Exception e = assertThrows(Exception.class,
                 () -> restrictedMapper().readValue(json, ParseContext.class));
         assertTrue(rootMessage(e).contains("may not be supplied via a request parseContext"),
@@ -116,6 +115,19 @@ public void restrictedRejectsParserSmuggledInArrayConfig() {
                 "expected array-nested wire-blocked rejection, got: " + rootMessage(e));
     }
 
+    @Test
+    public void arrayElementOfWrongTypeRejectedBeforeConstruction() {
+        // A bare-string element whose type doesn't match the array's component interface must be
+        // refused at resolution, before it is constructed.
+        String json = "{\"metadata-filters\":[\"external-parser\"]}";
+        Exception e = assertThrows(Exception.class, () -> {
+            ParseContext ctx = restrictedMapper().readValue(json, ParseContext.class);
+            ParseContextUtils.resolveAll(ctx, ParseContextUtils.class.getClassLoader());
+        });
+        assertTrue(rootMessage(e).contains("not assignable to"),
+                "expected assignability rejection at resolution, got: " + rootMessage(e));
+    }
+
     @Test
     public void restrictedAllowsSafeConfig() throws Exception {
         // Allowed component (metadata filter) + bounded config DTOs (handler, timeout) must pass.

tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java

@@ -337,7 +337,8 @@ private static void loadAllProviders(TikaServerConfig tikaServerConfig, ServerSt
 
     }
 
-    private static List<ResourceProvider> loadCoreProviders(TikaServerConfig tikaServerConfig, ServerStatus serverStatus) throws TikaException, IOException, SAXException {
+    // package-private so the pipes/async start-guard can be exercised directly in tests
+    static List<ResourceProvider> loadCoreProviders(TikaServerConfig tikaServerConfig, ServerStatus serverStatus) throws TikaException, IOException, SAXException {
         List<ResourceProvider> resourceProviders = new ArrayList<>();
         boolean addAsyncResource = false;
         boolean addPipesResource = false;

tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerPipesIntegrationTest.java

@@ -267,21 +267,21 @@ public void testPipesRejectsParserInjection() throws Exception {
                 .toAbsolutePath()
                 .toString())});
         awaitServerStartup();
-        // A request whose parseContext binds a process-spawning ExternalParser must be refused at
+        // A request whose parseContext binds a wire-blocked component must be refused at
         // deserialization, before any fork/parse -- so the request fails and nothing is emitted.
-        String malicious = "{\"id\":\"hello_world.xml\"," +
+        String requestJson = "{\"id\":\"hello_world.xml\"," +
                 "\"fetcher\":\"" + CXFTestBase.FETCHER_ID + "\"," +
                 "\"fetchKey\":\"hello_world.xml\"," +
                 "\"emitter\":\"" + CXFTestBase.EMITTER_JSON_ID + "\"," +
                 "\"emitKey\":\"\"," +
                 "\"onParseException\":\"emit\"," +
                 "\"parse-context\":{\"typed\":{\"external-parser\":{\"config\":{" +
-                "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo pwned\"]," +
+                "\"commandLine\":[\"/bin/sh\",\"-c\",\"echo x\"]," +
                 "\"supportedTypes\":[\"text/plain\"]}}}}}";
         Response response = WebClient
                 .create(endPoint + "/pipes")
                 .accept("application/json")
-                .post(malicious);
+                .post(requestJson);
         assertTrue(response.getStatus() != 200,
                 "parser injection must be rejected, got HTTP " + response.getStatus());
         assertFalse(Files.isRegularFile(TEMP_OUTPUT_DIR.resolve("hello_world.xml.json")),

tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTest.java

@@ -0,0 +1,54 @@
+/*
+ * 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.server.core;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import org.apache.tika.exception.TikaConfigException;
+
+public class TikaServerProcessTest {
+
+    private static TikaServerConfig config(boolean unsecure, String... endpoints) {
+        TikaServerConfig c = new TikaServerConfig();
+        c.setEndpoints(new ArrayList<>(List.of(endpoints)));
+        c.setEnableUnsecureFeatures(unsecure);
+        return c;
+    }
+
+    @Test
+    public void pipesAndAsyncRequireUnsecureFeatures() {
+        // The pipes/async endpoints fork processes and read/write via fetchers/emitters; the
+        // start-guard must refuse them unless enableUnsecureFeatures is set, even when listed.
+        assertThrows(TikaConfigException.class,
+                () -> TikaServerProcess.loadCoreProviders(config(false, "pipes"), null));
+        assertThrows(TikaConfigException.class,
+                () -> TikaServerProcess.loadCoreProviders(config(false, "async"), null));
+    }
+
+    @Test
+    public void ordinaryEndpointIsAllowedWithoutUnsecureFeatures() {
+        // The guard must not false-fire on a non-forking endpoint.
+        assertDoesNotThrow(
+                () -> TikaServerProcess.loadCoreProviders(config(false, "meta"), null));
+    }
+}