gnodet-bot commented on code in PR #26748: URL: https://github.com/apache/camel/pull/26748#discussion_r4072058781
########## 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: ⚠️ **Missing test: INSERT filtering path is not exercised.** This test validates `CamelJcrGetById` (read from JCR → Exchange headers), but the symmetric INSERT path — where `applyFilterToCamelHeaders` prevents Camel internal headers from being persisted as node properties — has no dedicated test. If that guard is removed or regressed in a future refactor, CI will not catch it. The existing `JcrProducerTest.testJcrProducer()` uses only `"my.contents.property"` (a non-Camel header), so it does not exercise the new filter. Add a test that: 1. Places a `CamelHttpUri` header (or similar Camel-prefixed header) on the exchange sent to the insert operation. 2. After insert, opens a JCR session and reads back the stored node's properties. 3. Asserts that the property was **not** stored (i.e. `node.hasProperty("CamelHttpUri")` returns `false`). 4. Asserts that ordinary headers are still stored as before. Without this, the insert-path filtering is covered only by the PR description. ########## 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: ⚠️ **`@UriParam` label should be `"producer,filter"`, not `"filter"`.** `JcrConsumer` is a pure JCR event listener — it registers an `EventListener` on the observation manager and never maps any JCR properties to Exchange headers. Setting `headerFilterStrategy` on a consumer endpoint silently has **zero effect** at runtime. Using `label = "filter"` without a role qualifier causes the DSL generator to emit `headerFilterStrategy()` on both `JcrEndpointConsumerBuilder` (lines 281/297 in the DSL factory) and `JcrEndpointProducerBuilder` — the consumer overload is a dead API. Fix: ```suggestion @UriParam(label = "producer,filter", description = "To use a custom org.apache.camel.spi.HeaderFilterStrategy to filter header to and from Camel message.") private HeaderFilterStrategy headerFilterStrategy; ``` This will suppress the consumer DSL methods and regenerate the catalog JSON with `"label": "producer,filter"`, accurately reflecting that the option is producer-only. -- 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]
