CAMEL-24423/24442/24449/24453/24455/24475: untrusted input handling fixes, backport to camel-4.18.x

components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaSagaIT.java

@@ -45,7 +45,7 @@ protected RouteBuilder createRouteBuilder() {
         return new RouteBuilder() {
             @Override
             public void configure() throws Exception {
-                getCamelContext().addService(new InMemorySagaService());
+                getCamelContext().addService(new KafkaInteropSagaService());
 
                 from("direct:saga")
                         .saga()
@@ -64,6 +64,20 @@ public void configure() throws Exception {
     }
 }
 
+/**
+ * The saga id is stored in the exchange's internal state, which does not survive the Kafka produce/consume round-trip -
+ * only the {@code Long-Running-Action} header does. Advertise header support so the consumer route can join the saga
+ * started by the producer route, as documented for a {@code CamelSagaService} that relies on the header to join sagas
+ * started by another participant.
+ */
+final class KafkaInteropSagaService extends InMemorySagaService {
+
+    @Override
+    public boolean isLongRunningActionHeaderSupported() {
+        return true;
+    }
+}
+
 final class SagaBean {
     public static String id;
     public static Boolean isSame = false;

components/camel-lra/src/main/java/org/apache/camel/service/lra/LRASagaService.java

@@ -66,6 +66,15 @@ public CompletableFuture<CamelSagaCoordinator> newSaga(Exchange exchange) {
                 .thenApply(url -> new LRASagaCoordinator(LRASagaService.this, url));
     }
 
+    /**
+     * The LRA protocol carries the coordinator id in the {@code Long-Running-Action} header, so a saga started by
+     * another participant is joined through it.
+     */
+    @Override
+    public boolean isLongRunningActionHeaderSupported() {
+        return true;
+    }
+
     @Override
     public CompletableFuture<CamelSagaCoordinator> getSaga(String id) {
         CompletableFuture<CamelSagaCoordinator> coordinator;

components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/PlatformHttpEndpoint.java

@@ -16,7 +16,10 @@
  */
 package org.apache.camel.component.platform.http;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.apache.camel.AsyncEndpoint;
 import org.apache.camel.Category;
@@ -49,7 +52,12 @@ public class PlatformHttpEndpoint extends DefaultEndpoint
 
     private static final String PROXY_PATH = "proxy";
 
-    private static final Set<String> COMMON_HTTP_REQUEST_HEADERS = Set.of(
+    /**
+     * Request headers that must not be echoed back on the response. Compared without regard to case: exchange headers
+     * keep the casing of the inbound request, and HTTP/2 requires field names to be lower case, so an exact-case lookup
+     * against these canonical spellings never matches an HTTP/2 request.
+     */
+    private static final Set<String> COMMON_HTTP_REQUEST_HEADERS = caseInsensitiveSet(
             "A-IM",
             "Accept",
             "Accept-Charset",
@@ -79,6 +87,12 @@ public class PlatformHttpEndpoint extends DefaultEndpoint
             "TE",
             "User-Agent");
 
+    private static Set<String> caseInsensitiveSet(String... names) {
+        Set<String> set = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
+        set.addAll(Arrays.asList(names));
+        return Collections.unmodifiableSet(set);
+    }
+
     @UriPath(description = "The path under which this endpoint serves the HTTP requests, for proxy use 'proxy'")
     @Metadata(required = true)
     private final String path;
@@ -301,8 +315,16 @@ PlatformHttpEngine getOrCreateEngine() {
                 : getComponent().getOrCreateEngine();
     }
 
+    /**
+     * Whether this endpoint is the documented {@code platform-http:proxy} endpoint.
+     * <p>
+     * Compared for equality rather than as a prefix. Proxy mode makes {@link #getPath()} return {@code "/"}, turning
+     * the endpoint into a catch-all, and the consumer then takes the forward target from the request's own {@code Host}
+     * header - so a path that merely begins with "proxy", such as {@code proxyStats}, would become a forwarding proxy
+     * its author never asked for.
+     */
     public boolean isHttpProxy() {
-        return this.path.startsWith(PROXY_PATH);
+        return PROXY_PATH.equals(this.path);
     }
 
     public boolean isReturnHttpRequestHeaders() {

components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointHeaderEchoTest.java

@@ -0,0 +1,53 @@
+/*
+ * 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.camel.component.platform.http;
+
+import org.apache.camel.impl.DefaultCamelContext;
+import org.apache.camel.spi.HeaderFilterStrategy;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Exchange headers keep the casing of the inbound request, and HTTP/2 requires field names to be lower case. An
+ * exact-case lookup against canonically capitalised names therefore never suppresses anything on an HTTP/2 request,
+ * which is the traffic most likely to carry the credentials this is meant to keep out of the response.
+ */
+class PlatformHttpEndpointHeaderEchoTest {
+
+    @Test
+    void requestHeadersAreSuppressedWhateverTheirCasing() throws Exception {
+        try (DefaultCamelContext context = new DefaultCamelContext()) {
+            context.start();
+            PlatformHttpComponent component = new PlatformHttpComponent(context);
+            PlatformHttpEndpoint endpoint
+                    = (PlatformHttpEndpoint) component.createEndpoint("platform-http:/test");
+
+            HeaderFilterStrategy strategy = endpoint.getHeaderFilterStrategy();
+
+            // canonical, as sent over HTTP/1.1
+            assertTrue(strategy.applyFilterToCamelHeaders("Authorization", "Bearer x", null));
+            assertTrue(strategy.applyFilterToCamelHeaders("Cookie", "a=b", null));
+            // lower case, as required by HTTP/2
+            assertTrue(strategy.applyFilterToCamelHeaders("authorization", "Bearer x", null));
+            assertTrue(strategy.applyFilterToCamelHeaders("cookie", "a=b", null));
+            assertTrue(strategy.applyFilterToCamelHeaders("proxy-authorization", "Basic x", null));
+            // and any other casing a client might send
+            assertTrue(strategy.applyFilterToCamelHeaders("AUTHORIZATION", "Bearer x", null));
+        }
+    }
+}

components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointProxyPathTest.java

@@ -0,0 +1,65 @@
+/*
+ * 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.camel.component.platform.http;
+
+import org.apache.camel.impl.DefaultCamelContext;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Proxy mode makes the endpoint a catch-all whose forward target comes from the request's own Host header. Selecting it
+ * by prefix meant any path merely beginning with "proxy" became a forwarding proxy its author never asked for.
+ */
+class PlatformHttpEndpointProxyPathTest {
+
+    @Test
+    void onlyTheProxyPathSelectsProxyMode() throws Exception {
+        assertTrue(isProxy("platform-http:proxy"));
+
+        // a leading slash did not select proxy mode before the check was tightened, and still does not:
+        // narrowing the check must never turn an endpoint into a proxy that was not already one
+        assertFalse(isProxy("platform-http:/proxy"));
+
+        assertFalse(isProxy("platform-http:proxyStats"));
+        assertFalse(isProxy("platform-http:proxy-health"));
+        assertFalse(isProxy("platform-http:proxying"));
+        assertFalse(isProxy("platform-http:/orders"));
+    }
+
+    @Test
+    void aNonProxyPathIsNotTurnedIntoACatchAll() throws Exception {
+        try (DefaultCamelContext context = new DefaultCamelContext()) {
+            context.start();
+            PlatformHttpComponent component = new PlatformHttpComponent(context);
+            PlatformHttpEndpoint endpoint
+                    = (PlatformHttpEndpoint) component.createEndpoint("platform-http:proxyStats");
+
+            assertEquals("proxyStats", endpoint.getPath());
+        }
+    }
+
+    private static boolean isProxy(String uri) throws Exception {
+        try (DefaultCamelContext context = new DefaultCamelContext()) {
+            context.start();
+            PlatformHttpComponent component = new PlatformHttpComponent(context);
+            return ((PlatformHttpEndpoint) component.createEndpoint(uri)).isHttpProxy();
+        }
+    }
+}

components/camel-thrift/pom.xml

@@ -87,6 +87,11 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java

@@ -158,23 +158,31 @@ public void marshal(final Exchange exchange, final Object graph, final OutputStr
     }
 
     @Override
+    @SuppressWarnings("rawtypes")
     public Object unmarshal(final Exchange exchange, final InputStream inputStream) throws Exception {
         TDeserializer deserializer;
         ObjectHelper.notNull(defaultInstance, "defaultInstance or instanceClassName must be set", this);
 
+        // The data format is shared by every exchange on the route and TBase.read() only assigns the
+        // fields present in the incoming bytes, so deserializing into defaultInstance would let one
+        // message read or overwrite another's fields. Deserialize into a copy instead, clearing it first
+        // so values set on defaultInstance are not inherited when they are absent from the input.
+        TBase instance = defaultInstance.deepCopy();
+        instance.clear();
+
         if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_JSON)) {
             deserializer = new TDeserializer(new TJSONProtocol.Factory());
-            deserializer.deserialize(defaultInstance, IOUtils.toByteArray(inputStream));
+            deserializer.deserialize(instance, IOUtils.toByteArray(inputStream));
         } else if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_BINARY)) {
             deserializer = new TDeserializer(new TBinaryProtocol.Factory());
-            deserializer.deserialize(defaultInstance, IOUtils.toByteArray(inputStream));
+            deserializer.deserialize(instance, IOUtils.toByteArray(inputStream));
         } else if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_SIMPLE_JSON)) {
             throw new CamelException("Simple JSON format is avalable for the message marshalling only");
         } else {
             throw new CamelException("Invalid thrift content type format: " + contentTypeFormat);
         }
 
-        return defaultInstance;
+        return instance;
     }
 
     @SuppressWarnings("rawtypes")

components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java

@@ -0,0 +1,89 @@
+/*
+ * 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.camel.dataformat.thrift;
+
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.dataformat.thrift.generated.Operation;
+import org.apache.camel.dataformat.thrift.generated.Work;
+import org.apache.camel.test.junit5.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * A data format instance is shared by every exchange on the route, and Thrift's {@code TBase.read()} assigns only the
+ * fields present in the incoming bytes. Two messages unmarshalled through the same data format must therefore not be
+ * able to observe each other's fields - including through an optional field that the second message omits.
+ */
+class ThriftUnmarshalIsolationTest extends CamelTestSupport {
+
+    @Test
+    void anOmittedOptionalFieldDoesNotInheritThePreviousMessageValue() {
+        Work withComment = new Work();
+        withComment.num1 = 1;
+        withComment.num2 = 2;
+        withComment.op = Operation.ADD;
+        withComment.comment = "first message";
+
+        Work withoutComment = new Work();
+        withoutComment.num1 = 3;
+        withoutComment.num2 = 4;
+        withoutComment.op = Operation.SUBTRACT;
+
+        Object firstBytes = template.requestBody("direct:marshal", withComment);
+        Object secondBytes = template.requestBody("direct:marshal", withoutComment);
+
+        Work first = (Work) template.requestBody("direct:unmarshal", firstBytes);
+        Work second = (Work) template.requestBody("direct:unmarshal", secondBytes);
+
+        assertThat(first.getComment()).isEqualTo("first message");
+        assertThat(second.getComment()).isNull();
+        assertThat(second.getNum1()).isEqualTo(3);
+        assertThat(first).isNotSameAs(second);
+    }
+
+    @Test
+    void anOmittedOptionalFieldDoesNotInheritTheDefaultInstanceValue() {
+        Work withoutComment = new Work();
+        withoutComment.num1 = 3;
+        withoutComment.num2 = 4;
+        withoutComment.op = Operation.SUBTRACT;
+
+        Object bytes = template.requestBody("direct:marshal", withoutComment);
+
+        Work result = (Work) template.requestBody("direct:unmarshal-populated-default", bytes);
+
+        assertThat(result.getComment()).isNull();
+        assertThat(result.getNum1()).isEqualTo(3);
+    }
+
+    @Override
+    protected RouteBuilder createRouteBuilder() {
+        return new RouteBuilder() {
+            @Override
+            public void configure() {
+                ThriftDataFormat format = new ThriftDataFormat(new Work());
+                Work populatedDefault = new Work();
+                populatedDefault.comment = "default value";
+                ThriftDataFormat populatedDefaultFormat = new ThriftDataFormat(populatedDefault);
+                from("direct:marshal").marshal(format);
+                from("direct:unmarshal").unmarshal(format);
+                from("direct:unmarshal-populated-default").unmarshal(populatedDefaultFormat);
+            }
+        };
+    }
+}

components/camel-tika/pom.xml

@@ -86,6 +86,11 @@
             <version>${hamcrest-version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>