diff --git a/components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaSagaIT.java b/components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaSagaIT.java index 187477f4424ba..1c0073220d7d1 100644 --- a/components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaSagaIT.java +++ b/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; diff --git a/components/camel-lra/src/main/java/org/apache/camel/service/lra/LRASagaService.java b/components/camel-lra/src/main/java/org/apache/camel/service/lra/LRASagaService.java index e10eed2b07c98..0d3acb834eb59 100644 --- a/components/camel-lra/src/main/java/org/apache/camel/service/lra/LRASagaService.java +++ b/components/camel-lra/src/main/java/org/apache/camel/service/lra/LRASagaService.java @@ -66,6 +66,15 @@ public CompletableFuture 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 getSaga(String id) { CompletableFuture coordinator; diff --git a/components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/PlatformHttpEndpoint.java b/components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/PlatformHttpEndpoint.java index c2b729e1eaab2..58195f2c4fc31 100644 --- a/components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/PlatformHttpEndpoint.java +++ b/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 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 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 caseInsensitiveSet(String... names) { + Set 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. + *

+ * 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() { diff --git a/components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointHeaderEchoTest.java b/components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointHeaderEchoTest.java new file mode 100644 index 0000000000000..b428d62098364 --- /dev/null +++ b/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)); + } + } +} diff --git a/components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointProxyPathTest.java b/components/camel-platform-http/src/test/java/org/apache/camel/component/platform/http/PlatformHttpEndpointProxyPathTest.java new file mode 100644 index 0000000000000..5d2e4b39d5265 --- /dev/null +++ b/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(); + } + } +} diff --git a/components/camel-thrift/pom.xml b/components/camel-thrift/pom.xml index d1f4c2ea94cb9..f68c07cd1e556 100644 --- a/components/camel-thrift/pom.xml +++ b/components/camel-thrift/pom.xml @@ -87,6 +87,11 @@ + + org.assertj + assertj-core + test + diff --git a/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java b/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java index 3ab4a716be25b..83530bfafef20 100644 --- a/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java +++ b/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") diff --git a/components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java b/components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java new file mode 100644 index 0000000000000..a736676c60bc7 --- /dev/null +++ b/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); + } + }; + } +} diff --git a/components/camel-tika/pom.xml b/components/camel-tika/pom.xml index 821a6e2d9a6db..0c1b82a3c625a 100644 --- a/components/camel-tika/pom.xml +++ b/components/camel-tika/pom.xml @@ -86,6 +86,11 @@ ${hamcrest-version} test + + org.assertj + assertj-core + test + diff --git a/components/camel-tika/src/main/java/org/apache/camel/component/tika/TikaProducer.java b/components/camel-tika/src/main/java/org/apache/camel/component/tika/TikaProducer.java index 9328dc47d3445..6d8f2c2625c2c 100644 --- a/components/camel-tika/src/main/java/org/apache/camel/component/tika/TikaProducer.java +++ b/components/camel-tika/src/main/java/org/apache/camel/component/tika/TikaProducer.java @@ -35,6 +35,8 @@ import org.xml.sax.SAXException; import org.apache.camel.Exchange; +import org.apache.camel.spi.HeaderFilterStrategy; +import org.apache.camel.support.DefaultHeaderFilterStrategy; import org.apache.camel.support.DefaultProducer; import org.apache.tika.config.TikaConfig; import org.apache.tika.detect.Detector; @@ -53,6 +55,8 @@ public class TikaProducer extends DefaultProducer { private static final Logger LOG = LoggerFactory.getLogger(TikaProducer.class); + private static final HeaderFilterStrategy HEADER_FILTER_STRATEGY = createHeaderFilterStrategy(); + private final TikaConfiguration tikaConfiguration; private final Parser parser; @@ -127,11 +131,15 @@ private void convertMetadataToHeaders(Metadata metadata, Exchange exchange) { if (metadata != null) { for (String metaname : metadata.names()) { String[] values = metadata.getValues(metaname); - if (values.length == 1) { - exchange.getIn().setHeader(metaname, values[0]); - } else { - exchange.getIn().setHeader(metaname, values); + Object value = values.length == 1 ? values[0] : values; + // The names come out of the parsed document, so they are chosen by whoever produced it. + // Filter them the same way a consumer filters names supplied by an external sender, so a + // document cannot declare a metadata name that lands in the Camel-internal namespace. + if (HEADER_FILTER_STRATEGY.applyFilterToExternalHeaders(metaname, value, exchange)) { + LOG.debug("Skipping parsed metadata {} as the name is in the Camel-internal namespace", metaname); + continue; } + exchange.getIn().setHeader(metaname, value); } } } @@ -178,4 +186,13 @@ private TransformerHandler getTransformerHandler( return handler; } + + private static HeaderFilterStrategy createHeaderFilterStrategy() { + DefaultHeaderFilterStrategy strategy = new DefaultHeaderFilterStrategy(); + // Match case-insensitively, and cover the fully qualified form as well as the Camel prefix + strategy.setLowerCase(true); + strategy.setInFilterStartsWith("Camel", "camel", "org.apache.camel."); + return strategy; + } + } diff --git a/components/camel-tika/src/test/java/org/apache/camel/component/tika/TikaMetadataHeaderFilterTest.java b/components/camel-tika/src/test/java/org/apache/camel/component/tika/TikaMetadataHeaderFilterTest.java new file mode 100644 index 0000000000000..ba4b85e9b44b0 --- /dev/null +++ b/components/camel-tika/src/test/java/org/apache/camel/component/tika/TikaMetadataHeaderFilterTest.java @@ -0,0 +1,74 @@ +/* + * 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.tika; + +import java.nio.charset.StandardCharsets; + +import org.apache.camel.EndpointInject; +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.test.junit5.CamelTestSupport; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The metadata names handed to {@code convertMetadataToHeaders} come out of the parsed document, so they are chosen by + * whoever produced it. An HTML {@code } is the most direct way to demonstrate that: the name attribute + * reaches Tika's metadata verbatim, so a document can ask for any header name at all. + */ +class TikaMetadataHeaderFilterTest extends CamelTestSupport { + + @EndpointInject("mock:result") + protected MockEndpoint resultEndpoint; + + @Test + void documentMetadataCannotSetCamelInternalHeaders() throws Exception { + String html = "" + + "" + + "" + + "" + + "" + + "" + + "thi"; + + resultEndpoint.setExpectedMessageCount(1); + template.sendBody("direct:start", html.getBytes(StandardCharsets.UTF_8)); + resultEndpoint.assertIsSatisfied(); + + Exchange exchange = resultEndpoint.getExchanges().get(0); + assertThat(exchange.getIn().getHeader(Exchange.FILE_NAME)).isNull(); + assertThat(exchange.getIn().getHeader("camelfilename")).isNull(); + assertThat(exchange.getIn().getHeader("CAMELHttpUri")).isNull(); + assertThat(exchange.getIn().getHeader("org.apache.camel.internal")).isNull(); + + // metadata outside the internal namespace is still mapped, so the filter has not simply dropped everything + assertThat(exchange.getIn().getHeader("author")).isEqualTo("kept"); + assertThat(exchange.getIn().getHeader("dc:title")).isEqualTo("t"); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("direct:start").to("tika:parse").to("mock:result"); + } + }; + } +} diff --git a/components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java b/components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java index 80f9ebfe30d3c..10d5cf59a3581 100644 --- a/components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java +++ b/components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java @@ -941,7 +941,8 @@ private void logNamespaces(Exchange exchange) { // fetch all namespaces if (document instanceof InputSource) { InputSource inputSource = (InputSource) document; - answer = (NodeList) xpathExpression.evaluate(inputSource, XPathConstants.NODESET); + answer = (NodeList) xpathExpression.evaluate(toHardenedDocument(exchange, inputSource), + XPathConstants.NODESET); } else if (document instanceof DOMSource) { DOMSource source = (DOMSource) document; answer = (NodeList) xpathExpression.evaluate(source.getNode(), XPathConstants.NODESET); @@ -949,7 +950,8 @@ private void logNamespaces(Exchange exchange) { SAXSource source = (SAXSource) document; // since its a SAXSource it may not return an NodeList (for // example if using Saxon) - Object result = xpathExpression.evaluate(source.getInputSource(), XPathConstants.NODESET); + Object result = xpathExpression.evaluate(toHardenedDocument(exchange, source.getInputSource()), + XPathConstants.NODESET); if (result instanceof NodeList) { answer = (NodeList) result; } else { @@ -1017,7 +1019,7 @@ protected Object doInEvaluateAs(XPathExpression xpathExpression, Exchange exchan } if (document instanceof InputSource) { InputSource inputSource = (InputSource) document; - answer = xpathExpression.evaluate(inputSource, resultQName); + answer = xpathExpression.evaluate(toHardenedDocument(exchange, inputSource), resultQName); } else if (document instanceof DOMSource) { DOMSource source = (DOMSource) document; answer = xpathExpression.evaluate(source.getNode(), resultQName); @@ -1027,7 +1029,7 @@ protected Object doInEvaluateAs(XPathExpression xpathExpression, Exchange exchan } else { if (document instanceof InputSource) { InputSource inputSource = (InputSource) document; - answer = xpathExpression.evaluate(inputSource); + answer = xpathExpression.evaluate(toHardenedDocument(exchange, inputSource)); } else if (document instanceof DOMSource) { DOMSource source = (DOMSource) document; answer = xpathExpression.evaluate(source.getNode()); @@ -1228,6 +1230,29 @@ protected boolean isInputStreamNeededForObject(Object obj) { return false; } + /** + * Parses an {@link InputSource} into a DOM document before it is evaluated. + *

+ * {@link XPathExpression#evaluate(InputSource)} and its overloads build a {@link javax.xml.parsers.DocumentBuilder} + * of their own using the JDK defaults, which accept a {@code DOCTYPE} declaration and resolve external entities. + * Routing the source through the type converter instead reuses the hardened {@code DocumentBuilderFactory} that the + * default {@code documentType} of {@link Document} already goes through, so both document types are parsed with the + * same configuration. The XPath engine builds a full DOM from the source either way, so this does not add a parse + * that was not already happening. + */ + protected Document toHardenedDocument(Exchange exchange, InputSource inputSource) { + Document document = null; + if (inputSource != null) { + document = exchange.getContext().getTypeConverter().convertTo(Document.class, exchange, inputSource); + } + if (document == null) { + throw new RuntimeCamelException( + "Cannot convert the InputSource to a org.w3c.dom.Document for evaluating the XPath expression: " + + getText()); + } + return document; + } + /** * Strategy method to extract the document from the exchange. */ diff --git a/core/camel-core-processor/src/main/java/org/apache/camel/processor/saga/SagaProcessor.java b/core/camel-core-processor/src/main/java/org/apache/camel/processor/saga/SagaProcessor.java index 1e28a1f3a4fb0..02a0aefb65bf3 100644 --- a/core/camel-core-processor/src/main/java/org/apache/camel/processor/saga/SagaProcessor.java +++ b/core/camel-core-processor/src/main/java/org/apache/camel/processor/saga/SagaProcessor.java @@ -54,8 +54,11 @@ public SagaProcessor(CamelContext camelContext, Processor childProcessor, CamelS protected CompletableFuture getCurrentSagaCoordinator(Exchange exchange) { // try internal state first (survives removeHeaders("*")) String currentSaga = exchange.getExchangeExtension().getSagaLongRunningAction(); - if (currentSaga == null) { - // fall back to header for interoperability (e.g., LRA protocol) + if (currentSaga == null && sagaService.isLongRunningActionHeaderSupported()) { + // fall back to header for interoperability (e.g., LRA protocol), but only for a service that takes part + // in such a protocol. Long-Running-Action is outside the Camel namespace that consumers filter, and the + // id is written back onto responses, so consulting it where no external coordinator exists would let a + // message pick which saga its exchange joins. currentSaga = exchange.getIn().getHeader(Exchange.SAGA_LONG_RUNNING_ACTION, String.class); } if (currentSaga != null) { diff --git a/core/camel-core/src/test/java/org/apache/camel/builder/xml/XPathFeatureTest.java b/core/camel-core/src/test/java/org/apache/camel/builder/xml/XPathFeatureTest.java index d40fdc35177df..2422701d920e8 100644 --- a/core/camel-core/src/test/java/org/apache/camel/builder/xml/XPathFeatureTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/builder/xml/XPathFeatureTest.java @@ -16,8 +16,13 @@ */ package org.apache.camel.builder.xml; +import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import org.xml.sax.InputSource; import org.xml.sax.SAXParseException; import org.apache.camel.ContextTestSupport; @@ -26,17 +31,23 @@ import org.apache.camel.RuntimeCamelException; import org.apache.camel.TypeConversionException; import org.apache.camel.converter.jaxp.XmlConverter; +import org.apache.camel.language.xpath.XPathBuilder; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.ResourceLock; import org.junit.jupiter.api.parallel.Resources; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.camel.language.xpath.XPathBuilder.xpath; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.*; @ResourceLock(Resources.SYSTEM_PROPERTIES) public class XPathFeatureTest extends ContextTestSupport { public static final String DOM_BUILDER_FACTORY_FEATURE = XmlConverter.DOCUMENT_BUILDER_FACTORY_FEATURE; + private static final String CANARY = "CANARY-SHOULD-NOT-BE-READ"; + public static final String XML_DATA = " ]> &xxe; "; public static final String XML_DATA_INVALID @@ -76,6 +87,67 @@ public void testXPath() { } } + /** + * {@code documentType=InputSource} used to hand the payload straight to {@link javax.xml.xpath.XPathExpression}, + * which builds a DocumentBuilder of its own with the JDK defaults - so the DOCTYPE that + * {@link #testXPathDocTypeDisallowed()} pins as refused on the default document type was accepted here, and the + * external entity was resolved and expanded into the evaluated document. The two document types must agree on the + * parser configuration. + */ + @Test + void docTypeIsAlsoDisallowedForAnInputSourceDocumentType() throws Exception { + Path secret = Files.createTempFile("camel-xpath-entity", ".txt"); + try { + Files.writeString(secret, CANARY); + // an InputStream body, since that is what converts to an InputSource - and what a streaming + // documentType=InputSource deployment actually receives + String xml = " ]> &xxe; "; + + // both branches of doInEvaluateAs: with a result QName and without one + for (XPathBuilder builder : List.of(xpath("/").documentType(InputSource.class).stringResult(), + xpath("/test").documentType(InputSource.class))) { + assertThatThrownBy(() -> builder.evaluate(createExchange(new ByteArrayInputStream(xml.getBytes(UTF_8))))) + .as("a DOCTYPE must be refused for documentType=InputSource, as it is for the default type") + .hasRootCauseInstanceOf(SAXParseException.class) + .rootCause().hasMessageContaining("DOCTYPE"); + } + } finally { + Files.deleteIfExists(secret); + } + } + + /** + * The {@code InputSource} document type now shares the default type's parser, so it also shares its escape hatch: + * the same system properties that {@link #testXPath()} uses relax it. Points at a file that does not exist, so a + * {@code FileNotFoundException} is what proves the DOCTYPE was accepted and resolution attempted. + */ + @Test + void theDocumentBuilderFactoryFeaturesAlsoRelaxTheInputSourceDocumentType() { + System.setProperty(DOM_BUILDER_FACTORY_FEATURE + ":" + "http://xml.org/sax/features/external-general-entities", "true"); + System.setProperty(DOM_BUILDER_FACTORY_FEATURE + ":" + "http://apache.org/xml/features/disallow-doctype-decl", "false"); + try { + assertThatThrownBy(() -> xpath("/").documentType(InputSource.class).stringResult() + .evaluate(createExchange(new ByteArrayInputStream(XML_DATA.getBytes(UTF_8))))) + .hasRootCauseInstanceOf(FileNotFoundException.class); + } finally { + System.clearProperty(DOM_BUILDER_FACTORY_FEATURE + ":" + "http://xml.org/sax/features/external-general-entities"); + System.clearProperty(DOM_BUILDER_FACTORY_FEATURE + ":" + "http://apache.org/xml/features/disallow-doctype-decl"); + } + } + + /** + * Guards the assumption the test above rests on: an {@code InputStream} body really does reach the + * {@code InputSource} branch, rather than failing earlier for want of a type converter. + */ + @Test + void anInputStreamBodyConvertsToAnInputSourceDocumentType() { + Object result = xpath("/test/text()").documentType(InputSource.class).stringResult() + .evaluate(createExchange(new ByteArrayInputStream("ok".getBytes(UTF_8)))); + + assertThat(result).isEqualTo("ok"); + } + @Test public void testXPathNoTypeConverter() { // define a class without type converter as document type diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/SagaHeaderCannotSelectCoordinatorTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/SagaHeaderCannotSelectCoordinatorTest.java new file mode 100644 index 0000000000000..a9487b289e86e --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/processor/SagaHeaderCannotSelectCoordinatorTest.java @@ -0,0 +1,99 @@ +/* + * 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.processor; + +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.model.SagaPropagation; +import org.apache.camel.saga.CamelSagaCoordinator; +import org.apache.camel.saga.CamelSagaStep; +import org.apache.camel.saga.InMemorySagaService; +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; + +/** + * The saga id normally travels in the exchange's internal state. It is also readable from the + * {@code Long-Running-Action} header so a coordinator started elsewhere can be joined, which is how the LRA protocol + * carries it - but that header sits outside the {@code Camel} namespace consumers filter, and the id is written back + * onto responses, so under a service with no external coordinator it would let a message choose which saga its exchange + * joins. + *

+ * Asserted by watching which ids reach {@code getSaga}, rather than by joining a live saga: a saga started by another + * route has already completed by the time a second exchange could present its id, so that would fail for the wrong + * reason. + */ +public class SagaHeaderCannotSelectCoordinatorTest extends ContextTestSupport { + + private final RecordingSagaService sagaService = new RecordingSagaService(); + + @Test + public void aMessageSuppliedIdIsNotLookedUp() { + Exchange exchange = template.request("direct:mandatory", e -> { + e.getIn().setHeader(Exchange.SAGA_LONG_RUNNING_ACTION, "a-saga-id-from-the-wire"); + e.getIn().setBody("hello"); + }); + + assertTrue(exchange.isFailed(), "MANDATORY has no saga to join, so the exchange must fail"); + assertEquals(List.of(), sagaService.lookedUp, + "the coordinator must not be looked up from an id supplied by the message"); + } + + @Test + public void theInMemoryServiceDoesNotAdvertiseHeaderSupport() { + assertFalse(new InMemorySagaService().isLongRunningActionHeaderSupported()); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() throws Exception { + context.addService(sagaService); + + from("direct:mandatory").saga().propagation(SagaPropagation.MANDATORY) + .transform().constant("joined"); + } + }; + } + + /** + * Delegates to the in-memory service, recording every id it is asked to resolve. + */ + private static final class RecordingSagaService extends InMemorySagaService { + + private final List lookedUp = new CopyOnWriteArrayList<>(); + + @Override + public CompletableFuture getSaga(String id) { + lookedUp.add(id); + return super.getSaga(id); + } + + @Override + public void registerStep(CamelSagaStep step) { + super.registerStep(step); + } + } +} diff --git a/core/camel-support/src/main/java/org/apache/camel/saga/CamelSagaService.java b/core/camel-support/src/main/java/org/apache/camel/saga/CamelSagaService.java index e37201da7c569..c7e97bb0ee84b 100644 --- a/core/camel-support/src/main/java/org/apache/camel/saga/CamelSagaService.java +++ b/core/camel-support/src/main/java/org/apache/camel/saga/CamelSagaService.java @@ -33,4 +33,22 @@ public interface CamelSagaService extends Service, CamelContextAware { void registerStep(CamelSagaStep step); + /** + * Whether a saga coordinator may be selected from the {@code Long-Running-Action} message header. + *

+ * The saga id normally travels in the exchange's internal state, which survives {@code removeHeaders("*")}. The + * header is consulted as well so that a coordinator started elsewhere can be joined - the LRA protocol carries the + * id that way. That only makes sense for a service which actually participates in such a protocol: the header sits + * outside the {@code Camel} namespace that consumers filter, and the id is written back onto responses, so where no + * external coordinator exists it lets a message choose which saga its exchange joins. + *

+ * Defaults to false. A service that takes part in a distributed saga protocol overrides it. + * + * @return true if the {@code Long-Running-Action} header may select a coordinator + * @since 4.23 + */ + default boolean isLongRunningActionHeaderSupported() { + return false; + } + }