You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "oscerd (via GitHub)" <gi...@apache.org> on 2026/09/22 13:12:38 UTC
[PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
oscerd opened a new pull request, #26748:
URL: https://github.com/apache/camel/pull/26748
## Motivation
The `jcr` producer's `CamelJcrGetById` operation mapped **every** property of the retrieved node onto the Exchange via `message.setHeader(property.getName(), ...)` without running the names through a `HeaderFilterStrategy`, and the insert path persisted all message headers (bar three JCR control keys) as node properties. Unlike most components, `camel-jcr` defined no `HeaderFilterStrategy`, so Camel internal header names (e.g. `CamelHttpUri`) were copied through unfiltered in both directions.
This continues the header-filtering alignment done for `camel-coap` (CAMEL-24655).
Jira: https://issues.apache.org/jira/browse/CAMEL-24900
## Changes
- `JcrEndpoint` is now `HeaderFilterStrategyAware`, exposing a `headerFilterStrategy` endpoint option that defaults to `DefaultHeaderFilterStrategy` (filters names starting with `Camel`/`camel`, case-insensitively).
- `CamelJcrGetById` runs each property name through `applyFilterToExternalHeaders` before `setHeader`; ordinary document properties are preserved.
- The insert path runs each header through `applyFilterToCamelHeaders`, so Camel internal headers carried on the message are no longer persisted as node properties.
- Adds `JcrGetNodeByIdHeaderInjectionTest`: a `Camel`-prefixed property name, in four casings, is not mapped onto the Exchange, while an ordinary document property still is.
- Regenerates the catalog and endpoint-DSL metadata for the new option, and adds an upgrade-guide note (changed default).
## Testing
`mvn -f components/camel-jcr/pom.xml test` â all 14 tests pass, including the new injection test and the existing insert / `getById` round-trip tests.
---
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
đ¤ Generated with [Claude Code](https://claude.com/claude-code)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5799822463
Thanks both. Addressed the round in `0e8c49a`:
- Added `JcrInsertHeaderInjectionTest` for the insert-path filter (the coverage gap you both flagged).
- Scoped the option to `producer,filter` â the consumer DSL overloads are gone.
- Eager-initialised `headerFilterStrategy` (removes the lazy-getter race). One correction inline: it does not add a catalog `defaultValue`, since the option is object-typed.
- Kept `filterComponentHeaders` (now with a comment) and the producer null-guards, with rationale in the threads.
Also merged `main` (`c678202`) to resolve an upgrade-guide conflict with the newly-landed `camel-mustache`/`camel-chunk` entry â both sections retained. CI is green.
Ready for another look.
_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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #26748:
URL: https://github.com/apache/camel/pull/26748
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5873220760
<!-- camel-backport-bot -->
### đ Backport Bot
This bugfix targets `main` and may need porting to:
- `camel-4.22.x`
Add a `port/<branch>` label to automatically create a backport PR for that branch.
Backport PRs will be created automatically on merge (once a port label is added). Comment `/port` to create them immediately.
âšī¸ If you push additional commits after `/port`, use `/port` again to update the port PRs.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "oscerd (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5811307219
Thanks for the thorough re-review @davsclaus. On the two non-blocking notes:
- đĄ **default not shown in the catalog** â agreed the object-typed option can't carry a `defaultValue`. The new default is documented in the 4.23 upgrade guide (the canonical place for a changed default). Folding the same sentence into the `@UriParam` description is a good idea, but it triggers a full catalog/DSL regen and the shared local `~/.m2` is currently mixed with newer-main components, so I'm holding it as a trivial follow-up rather than churn this otherwise-ready branch â happy to add it before merge if you'd rather it also live in the component docs.
- đĩ **null guards** â agreed, and intended: with the eager init an explicit `setHeaderFilterStrategy(null)` now sticks and disables filtering entirely, which is coherent with supplying a permissive custom strategy. Left as-is.
_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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5885698344
<!-- camel-backport-bot -->
### đ Backport Bot
**Port PRs:**
- đ `camel-4.22.x` â #27043 (by @oscerd)
â ī¸ Port to `camel-4.22.x` (#27043) has differences from the merged version:
- `docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc`
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5873540081
<!-- camel-backport-bot -->
### đ Backport Bot
**Conflicts (đ¤ agent dispatched to resolve):**
- â ī¸ `camel-4.22.x` â Cherry-pick conflict in: docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
- â ī¸ `camel-4.18.x` â Cherry-pick conflict in: catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components/jcr.json, components/camel-jcr/src/generated/resources/META-INF/org/apache/camel/component/jcr/jcr.json, docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc, dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/JcrEndpointBuilderFactory.java
âšī¸ If you push additional commits after `/port`, use `/port` again to update the port PRs.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5777192941
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: Apache Camel Committers, please review the following items:
* First-time contributors **require MANUAL approval** for the GitHub Actions to run
* You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot although they are normally detected and executed by CI.
* You can label PRs using `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
* Build and test logs are available in the summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
:warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26748:
URL: https://github.com/apache/camel/pull/26748#issuecomment-5778197543
<!-- ci-tested-modules -->
:test_tube: **CI tested the following changed modules:**
- `catalog/camel-catalog`
- `components/camel-jcr`
- `docs`
- `dsl/camel-endpointdsl`
---
<details><summary>:microscope: Scalpel shadow comparison â Scalpel: 9 of 692 tested, 24 compile-only â current: 9 all tested</summary>
[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **36 affected modules** (current approach: 9).
**Skip-tests mode would test 9 modules** (4 direct + 8 downstream), **skip tests for 24** (generated code, meta-modules)
<details><summary>:warning: Modules only in Scalpel (27)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `camel-endpointdsl`
- `camel-endpointdsl-support`
- `camel-itest`
- `camel-jbang-core`
- `camel-jbang-it`
- `camel-jbang-main`
- `camel-jbang-plugin-edit`
- `camel-jbang-plugin-generate`
- `camel-jbang-plugin-kubernetes`
- `camel-jbang-plugin-test`
- `camel-kamelet-main`
- `camel-launcher`
- `camel-report-maven-plugin`
- `camel-route-parser`
- `camel-yaml-dsl`
- `camel-yaml-dsl-deserializers`
- `camel-yaml-dsl-maven-plugin`
- `coverage`
- `docs`
- `dummy-component`
</details>
<details><summary>Modules Scalpel would test (9)</summary>
- `camel-catalog` â catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components/jcr.json
- `camel-endpointdsl` â dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/JcrEndpointBuilderFactory.java
- `camel-jbang-mcp` â downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-mcp` â downstream of org.apache.camel:camel-jbang-core
- `camel-jbang-plugin-route-parser` â downstream of org.apache.camel:camel-route-parser
- `camel-jbang-plugin-tui` â downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-validate` â downstream of org.apache.camel:camel-yaml-dsl-validator
- `camel-jcr` â components/camel-jcr/src/generated/java/org/apache/camel/component/jcr/JcrEndpointConfigurer.java, components/camel-jcr/src/generated/java/org/apache/camel/component/jcr/JcrEndpointUriFactory.java, components/camel-jcr/src/generated/resources/META-INF/org/apache/camel/component/jcr/jcr.json, components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrEndpoint.java, components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrProducer.java, components/camel-jcr/src/test/java/org/apache/camel/component/jcr/JcrGetNodeByIdHeaderInjectionTest.java
- `camel-launcher-container` â downstream of org.apache.camel:camel-launcher
- `camel-yaml-dsl-validator` â downstream of org.apache.camel:camel-catalog
- `camel-yaml-dsl-validator-maven-plugin` â downstream of org.apache.camel:camel-yaml-dsl-validator
- `docs` â docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
</details>
<details><summary>Modules with tests skipped (24)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `camel-endpointdsl-support`
- `camel-itest`
- `camel-jbang-core`
- `camel-jbang-it`
- `camel-jbang-main`
- `camel-jbang-plugin-edit`
- `camel-jbang-plugin-generate`
- `camel-jbang-plugin-kubernetes`
- `camel-jbang-plugin-test`
- `camel-kamelet-main`
- `camel-launcher`
- `camel-report-maven-plugin`
- `camel-route-parser`
- `camel-yaml-dsl`
- `camel-yaml-dsl-deserializers`
- `camel-yaml-dsl-maven-plugin`
- `coverage`
- `dummy-component`
</details>
> :information_source: Shadow mode â Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
</details>
<details><summary>All tested modules (36 modules, 5m 13s total)</summary>
**Total reactor time:** 5m 13s
| Module | Duration | Status |
| --- | --- | --- |
| Camel :: Launcher | 55.0s | SUCCESS |
| Camel :: JBang :: MCP | 40.3s | SUCCESS |
| Camel :: JBang :: Plugin :: TUI | 31.2s | SUCCESS |
| Camel :: Catalog :: Camel Catalog | 23.8s | SUCCESS |
| Camel :: Component DSL | 21.7s | SUCCESS |
| Camel :: JCR | 19.9s | SUCCESS |
| Camel :: Docs | 15.8s | SUCCESS |
| Camel :: JBang :: Plugin :: Kubernetes | 15.2s | SUCCESS |
| Camel :: YAML DSL :: Validator | 11.4s | SUCCESS |
| Camel :: Kamelet Main | 10.6s | SUCCESS |
| Camel :: Catalog :: Camel Route Parser | 7.8s | SUCCESS |
| Camel :: JBang :: Plugin :: Testing | 7.7s | SUCCESS |
| Camel :: Catalog :: Camel Report Maven Plugin | 7.5s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 6.3s | SUCCESS |
| Camel :: All Components Sync point | 5.3s | SUCCESS |
| Camel :: JBang :: Plugin :: Validate | 5.2s | SUCCESS |
| Camel :: YAML DSL :: Maven Plugins | 4.0s | SUCCESS |
| Camel :: YAML DSL :: Validator Maven Plugin | 3.8s | SUCCESS |
| Camel :: Catalog :: Maven | 3.2s | SUCCESS |
| Camel :: Coverage | 2.7s | SUCCESS |
| Camel :: Catalog :: Suggest (deprecated) | 2.3s | SUCCESS |
| Camel :: JBang :: Plugin :: Edit | 1.8s | SUCCESS |
| Camel :: Assembly | 1.7s | SUCCESS |
| Camel :: Catalog :: Dummy Component | 1.2s | SUCCESS |
| Camel :: JBang :: Plugin :: Generate | 1.2s | SUCCESS |
| Camel :: Catalog :: Console | 1.1s | SUCCESS |
| Camel :: JBang :: Main | 1.1s | SUCCESS |
| Camel :: JBang :: Integration tests | 1.0s | SUCCESS |
| Camel :: Endpoint DSL :: Support | 1.0s | SUCCESS |
| Camel :: JBang :: Plugin :: MCP | 0.9s | SUCCESS |
| Camel :: Launcher :: Container | 0.7s | SUCCESS |
| Camel :: JBang :: Plugin :: Route Parser | 0.6s | SUCCESS |
| Camel :: Endpoint DSL | n/a | |
| Camel :: Integration Tests | n/a | |
| Camel :: JBang :: Core | n/a | |
| Camel :: YAML DSL | n/a | |
**Top 20 slowest modules:**
- `Camel :: Launcher` (55.0s)
- `Camel :: JBang :: MCP` (40.3s)
- `Camel :: JBang :: Plugin :: TUI` (31.2s)
- `Camel :: Catalog :: Camel Catalog` (23.8s)
- `Camel :: Component DSL` (21.7s)
- `Camel :: JCR` (19.9s)
- `Camel :: Docs` (15.8s)
- `Camel :: JBang :: Plugin :: Kubernetes` (15.2s)
- `Camel :: YAML DSL :: Validator` (11.4s)
- `Camel :: Kamelet Main` (10.6s)
- `Camel :: Catalog :: Camel Route Parser` (7.8s)
- `Camel :: JBang :: Plugin :: Testing` (7.7s)
- `Camel :: Catalog :: Camel Report Maven Plugin` (7.5s)
- `Camel :: YAML DSL :: Deserializers` (6.3s)
- `Camel :: All Components Sync point` (5.3s)
- `Camel :: JBang :: Plugin :: Validate` (5.2s)
- `Camel :: YAML DSL :: Maven Plugins` (4.0s)
- `Camel :: YAML DSL :: Validator Maven Plugin` (3.8s)
- `Camel :: Catalog :: Maven` (3.2s)
- `Camel :: Coverage` (2.7s)
</details>
---
:gear: [View full build and test results](https://github.com/apache/camel/actions/runs/35731984515)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-24900: camel-jcr - apply header filtering when mapping node properties to Exchange headers [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #26748:
URL: https://github.com/apache/camel/pull/26748#discussion_r4072418076
##########
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:
đ **This direction has no test.**
The upgrade guide documents both halves of the change, and `JcrGetNodeByIdHeaderInjectionTest` covers the `getById` half thoroughly (four casings, plus an ordinary property still mapping). But nothing asserts the insert half: that a `CamelHttpUri` header carried on the message is no longer persisted as a node property.
There's an existing insert test to extend â setting a `Camel`-prefixed header on the message, running the insert, then reading the node back and asserting the property is absent while an ordinary header is present, would mirror the `getById` test nicely.
##########
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:
đ **Lazy init in the getter is a data race, and it hides the default from the catalog.**
`getHeaderFilterStrategy()` assigns to this non-volatile field, and `JcrProducer.process()` calls it on every exchange â so concurrent producers can race on the assignment. In practice it's benign (the instances are equivalent and effectively immutable), but there's no reason to take the race, and the Java memory model doesn't formally rule out publishing a partially-constructed object.
More practically: because the field starts null, the generated `jcr.json` carries no `defaultValue` for this option, so nothing in the catalog or the component docs tells a user that `Camel*` filtering is now applied by default â which is the whole behavioural change this PR makes.
Initialising at the declaration, the way `NatsConfiguration:119` does, fixes both at once and lets the getter become a plain accessor:
```suggestion
@UriParam(label = "filter",
description = "To use a custom org.apache.camel.spi.HeaderFilterStrategy to filter header to and from Camel message.")
private HeaderFilterStrategy headerFilterStrategy = new DefaultHeaderFilterStrategy();
```
##########
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:
đĄ Nit: `headerFilterStrategy` can't be null here â `getJcrEndpoint().getHeaderFilterStrategy()` lazy-inits it â so both this `== null` and the `!= null` on the insert path are dead. Harmless, but they suggest a nullable contract that doesn't exist. (Initialising the field at its declaration, as suggested on `JcrEndpoint`, makes that unambiguous.)
##########
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:
đĄ `filterComponentHeaders` is now redundant. All three keys it strips are `Camel`-prefixed â `JCR_NODE_NAME` = `CamelJcrNodeName`, `JCR_OPERATION` = `CamelJcrOperation`, `JCR_NODE_TYPE` = `CamelJcrNodeType` â so `applyFilterToCamelHeaders` on the next line already excludes every one of them under the default strategy.
Worth removing so there's one filtering mechanism rather than two overlapping ones. The one thing to weigh first: a user who configures a *custom* `headerFilterStrategy` that doesn't filter `Camel*` would then start persisting the JCR control headers as node properties, which `filterComponentHeaders` currently prevents unconditionally. If that's a case you want to keep guarding, leaving it is defensible â but then it deserves a comment saying so, because it reads as dead code.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org