oscerd commented on code in PR #26748: URL: https://github.com/apache/camel/pull/26748#discussion_r4082290737
########## components/camel-jcr/src/test/java/org/apache/camel/component/jcr/JcrGetNodeByIdHeaderInjectionTest.java: ########## @@ -0,0 +1,95 @@ +/* + * 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.jcr; + +import javax.jcr.Node; +import javax.jcr.RepositoryException; +import javax.jcr.Session; + +import org.apache.camel.EndpointInject; +import org.apache.camel.Exchange; +import org.apache.camel.Message; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * The JCR producer's {@code CamelJcrGetById} operation maps the properties of the retrieved node into Exchange headers. + * A content author must not be able to use a property named after a Camel internal header (in any casing) to inject + * that header, as those steer downstream processing (HTTP target URI, bean method dispatch, file names, and so on). + * Ordinary document properties must still be mapped. + */ +public class JcrGetNodeByIdHeaderInjectionTest extends JcrRouteTestSupport { + + private static final String CONTENT = "content is here"; + + private static final String[] CAMEL_HEADER_VARIANTS = { + "CamelHttpUri", "camelHttpUri", "caMELHttpUri", "CAMELHTTPURI" }; + + @EndpointInject("mock:result") + private MockEndpoint result; + + private String identifier; + + @Override + public void doPreSetup() throws RepositoryException { + Session session = openSession(); + Node node = session.getRootNode().addNode("injectionRoot").addNode("test"); + node.setProperty("my.contents.property", CONTENT); + for (String variant : CAMEL_HEADER_VARIANTS) { + node.setProperty(variant, "malicious"); + } + identifier = node.getIdentifier(); + + session.save(); + session.logout(); + } + + @Test + public void camelHeadersInNodePropertiesAreFilteredRegardlessOfCase() throws Exception { + result.expectedMessageCount(1); + + Exchange exchange = createExchangeWithBody(identifier); + template.send("direct:a", exchange); + MockEndpoint.assertIsSatisfied(context); + + Message in = result.getReceivedExchanges().get(0).getIn(); + for (String variant : CAMEL_HEADER_VARIANTS) { + // the Camel header map is case-insensitive, so this lookup also catches the other spellings + assertNull(in.getHeader(variant), + "a Camel internal header must not be injectable through a JCR node property: " + variant); + } + assertEquals(CONTENT, in.getHeader("my.contents.property", String.class), + "an ordinary document property must still be mapped to a header"); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("direct:a") + .setHeader(JcrConstants.JCR_OPERATION, constant(JcrConstants.JCR_GET_BY_ID)) + .to("jcr://user:pass@repository") + .to("mock:result"); + } + }; + } +} Review Comment: Done in `0e8c49a` — added `JcrInsertHeaderInjectionTest`: it sets a `Camel`-prefixed header (four casings) plus an ordinary header on the insert exchange, reads the stored node back by identifier, and asserts `node.hasProperty(variant)` is `false` while the ordinary property is present. 15/15 module tests pass. _Claude Code on behalf of Andrea Cosentino (@oscerd)_ ########## components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrEndpoint.java: ########## @@ -72,6 +75,9 @@ public class JcrEndpoint extends DefaultEndpoint { private long sessionLiveCheckInterval = 60000L; @UriParam private String workspaceName; + @UriParam(label = "filter", + description = "To use a custom org.apache.camel.spi.HeaderFilterStrategy to filter header to and from Camel message.") + private HeaderFilterStrategy headerFilterStrategy; Review Comment: Done in `0e8c49a` — changed to `label = "producer,filter"`. The regenerated `JcrEndpointBuilderFactory` no longer emits `headerFilterStrategy()` on `JcrEndpointConsumerBuilder` (it is now only on the producer builder, inherited by the combined `JcrEndpointBuilder`), and `jcr.json` records `"label": "producer,filter"`. _Claude Code on behalf of Andrea Cosentino (@oscerd)_ ########## components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrProducer.java: ########## @@ -57,6 +59,10 @@ public void process(Exchange exchange) throws Exception { Map<String, Object> headers = filterComponentHeaders(message.getHeaders()); for (String key : headers.keySet()) { Object header = message.getHeader(key); + if (headerFilterStrategy != null + && headerFilterStrategy.applyFilterToCamelHeaders(key, header, exchange)) { Review Comment: Done in `0e8c49a` — added `JcrInsertHeaderInjectionTest` mirroring the getById test for the insert direction (a `Camel`-prefixed header is not persisted as a node property, an ordinary property still is). _Claude Code on behalf of Andrea Cosentino (@oscerd)_ ########## components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrEndpoint.java: ########## @@ -72,6 +75,9 @@ public class JcrEndpoint extends DefaultEndpoint { private long sessionLiveCheckInterval = 60000L; @UriParam private String workspaceName; + @UriParam(label = "filter", + description = "To use a custom org.apache.camel.spi.HeaderFilterStrategy to filter header to and from Camel message.") + private HeaderFilterStrategy headerFilterStrategy; Review Comment: Eager-initialised the field at its declaration in `0e8c49a`, so the getter is now a plain accessor and the per-exchange race is gone. One correction on the second half: eager-init does **not** add a catalog `defaultValue` here. The option is object-typed (`HeaderFilterStrategy`), and the catalog only emits `defaultValue` for primitive/string/enum options — I verified `jcr.json` still carries none after the change. The default `Camel*` filtering is documented in the 4.23 upgrade guide instead. _Claude Code on behalf of Andrea Cosentino (@oscerd)_ ########## components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrProducer.java: ########## @@ -57,6 +59,10 @@ public void process(Exchange exchange) throws Exception { Map<String, Object> headers = filterComponentHeaders(message.getHeaders()); Review Comment: Kept it, and added a comment in `0e8c49a` explaining why. It strips the three JCR control keys unconditionally, independent of the configured strategy, so a user who supplies a custom `HeaderFilterStrategy` that does not filter `Camel*` still won't persist `CamelJcrOperation` / `CamelJcrNodeName` / `CamelJcrNodeType` as node properties — the guarding case you flagged. _Claude Code on behalf of Andrea Cosentino (@oscerd)_ ########## components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrProducer.java: ########## @@ -81,7 +87,11 @@ public void process(Exchange exchange) throws Exception { } else { value = converter.convertTo(aClass, exchange, property.getValue()); } - message.setHeader(property.getName(), value); + String name = property.getName(); + if (headerFilterStrategy == null + || !headerFilterStrategy.applyFilterToExternalHeaders(name, value, exchange)) { Review Comment: Kept the guards deliberately. With the field eager-initialised the getter won't return null on its own, but the option has a public setter (and URI/DSL config), so `setHeaderFilterStrategy(null)` is reachable; the guard then falls back to no filtering rather than NPE-ing per exchange. This matches the `strategy == null || ...` idiom in `CamelCoapResource`. _Claude Code on behalf of Andrea Cosentino (@oscerd)_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
