CAMEL-24625: camel-platform-http: add stripUriPrefix option for path-based reverse proxies - #26114
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
431386b to
4c22753
Compare
Croway
changed the title
✅ Generated files have been updatedA regen commit was automatically pushed to this branch. CI will re-run shortly. |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 80 tested, 25 compile-only — current: 78 all testedMaveniverse Scalpel detected 105 affected modules (current approach: 78).
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for this well-scoped PR, Federico! The problem is real and the approach is solid — a pure helper in camel-http-base, an opt-in boolean with false default (no behaviour change for existing routes), thorough unit tests for the helper, and a live integration test for the key reverse-proxy scenario. A couple of small things to consider before marking it ready for review:
1. camel-http-base dependency in camel-platform-http-vertx may be redundant (see inline comment)
2. Case-insensitive segment matching may be dead code in practice (see inline comment)
Questions
- No upgrade-guide entry is needed for a new opt-in option with
default=false— confirming this is intentionally omitted. - The integration test doesn't cover
{placeholder}consumer paths in the live Vert.x scenario (only the pure-function unit test does). Is that coverage considered sufficient? stripUriPrefix=truewithoutmatchOnUriPrefix=true: the docs and tests always combine the two. A brief note or test covering the exact-match case (e.g.GET /reverse-proxy→ backend receives/) would remove any ambiguity.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. This review does not replace CodeRabbit, Sourcery, SonarCloud, or similar tools.
4e6a1bc to
c98b120
Compare
…-based reverse proxies
platform-http is the only HTTP consumer (unlike camel-servlet, camel-jetty,
camel-netty-http and camel-undertow) that does not make CamelHttpPath
relative to the consumer's own registered path, so combining it with the
http producer's bridgeEndpoint option to build a path-based reverse proxy
forwards the full raw request path instead of the path relative to the
consumer.
Add a new consumer option, stripUriPrefix (default false), and a shared
HttpHelper.stripUriPrefix(requestPath, consumerPath) helper in
camel-http-base that other HTTP consumers could reuse. The helper is a
defensive pure function: it only strips a full, boundary-respecting match
of the consumer path (REST-DSL {name} placeholders included) and otherwise
returns the request path unchanged - in particular a consumer path of "/"
(the platform-http:proxy pseudo-path) is always a no-op, so the existing
platform-http:proxy forward-proxy mode is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tgb2b9DSh1ncG7C82rwzsE
Apply HttpHelper.stripUriPrefix to CamelHttpPath in VertxPlatformHttpConsumer.populateCamelMessage when the new consumer option is enabled, using the endpoint's own registered path as the consumer path. This is the single place headers are (re)populated for a request that reaches the route - the OAuth security handler path resets the message and lets processHttpRequest/populateCamelMessage repopulate headers afterwards, so no separate handling is needed there. CamelHttpUri/CamelHttpUrl are untouched, only CamelHttpPath is rewritten. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tgb2b9DSh1ncG7C82rwzsE
… reverse proxy Add a short section next to the existing reverse-proxy documentation showing the stripUriPrefix + bridgeEndpoint combination, with a before/after CamelHttpPath example and a cross-reference clarifying this is distinct from the platform-http:proxy Host-header forward proxy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tgb2b9DSh1ncG7C82rwzsE
…match docs/test Address review feedback: match literal path segments case-sensitively (the Vert.x router only ever routes case-exact prefixes, so this aligns the helper with reachable behaviour and HTTP path semantics), drop the two unit cases that asserted unreachable case-insensitive matching, verify exact-match stripping in the live Vert.x test without matchOnUriPrefix, and document that matchOnUriPrefix is not a prerequisite for stripUriPrefix. Co-authored-by: Claude Code <noreply@anthropic.com>
c98b120 to
3de5b67
Compare
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Re-review after the latest commits. This addresses the previous findings — summary below.
Previous findings — status:
-
✅
equalsIgnoreCasedead code — Addressed. The helper now uses plainequals()for segment matching, correctly reflecting actual case-sensitive router behaviour. -
⚠️ Redundantcamel-http-basedependency incamel-platform-http-vertx/pom.xml— Still present (see inline comment).camel-platform-httpalready pulls incamel-http-basetransitively; the explicit declaration remains unnecessary. -
✅ Upgrade guide — No entry needed for a new opt-in
false-default option. Correct. -
✅ Exact-match case coverage —
stripUriPrefixOnAnExactMatchLeavesTheRootPathintegration test added. Question answered. -
✅
matchOnUriPrefixindependence — Docs now explicitly state "matchOnUriPrefixis not required forstripUriPrefix." Good.
Static analysis: ast-grep — no findings. semgrep — 2 findings (cookie-missing-httponly, cookie-missing-secure-flag) at VertxPlatformHttpConsumer.java:598, pre-existing in main, not introduced by this PR.
New code quality: The helper logic, unit-test suite, Vert.x integration test, and endpoint wiring are all solid. No new issues beyond the residual nit below.
This review was generated by an AI agent, Hermès, on behalf of @gnodet.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.camel</groupId> | ||
| <artifactId>camel-http-base</artifactId> |
There was a problem hiding this comment.
camel-http-base dependency is redundant. camel-platform-http already depends on camel-http-base, so camel-platform-http-vertx gets it transitively via its existing camel-platform-http dependency. Camel's convention avoids redundant explicit deps. Consider removing this <dependency> block.
…sumer option The platform-http consumer option stripUriPrefix (added to PlatformHttpEndpoint in apache/camel via CAMEL-24625 / apache/camel#26114) was a no-op on the Spring Boot (servlet) platform-http engine: nothing stripped the registered consumer path from CamelHttpPath. Wire it up in SpringBootPlatformHttpConsumer, mirroring the Vert.x engine, so that combined with the http producer's bridgeEndpoint option a platform-http route becomes a path-based reverse proxy: from("platform-http:/reverse-proxy?matchOnUriPrefix=true&stripUriPrefix=true") .to("http://backend?bridgeEndpoint=true") // /reverse-proxy/get -> http://backend/get Only CamelHttpPath is rewritten; CamelHttpUri/CamelHttpUrl are left untouched.
3 participants
Problem
platform-httpis the one HTTP consumer that does not makeCamelHttpPathrelative to its own registered path -camel-servlet,camel-jetty,camel-netty-httpandcamel-undertowall strip their registered consumer path already. This means combiningplatform-httpwith thecamel-httpproducer'sbridgeEndpoint=trueto build a zero-code, path-based reverse proxy doesn't work: the full raw request path is forwarded instead of the path relative to the consumer, forcing route authors to manually strip the prefix withsetHeader+ a Simple/OGNLsubstring()expression.Change
Adds a new consumer option
stripUriPrefix(defaultfalse) onplatform-http:HttpHelper.stripUriPrefix(requestPath, consumerPath)incamel-http-base, usable by other HTTP consumers in the future.stripUriPrefixconsumer option onPlatformHttpEndpoint(engine-agnostic).camel-platform-http-vertx); the Spring Boot engine is not wired in this PR - it lives in the separateapache/camel-spring-bootrepo and gets its own follow-up PR (already in progress) once this PR provides the option to build against.CamelHttpUri/CamelHttpUrlare left untouched - onlyCamelHttpPathis rewritten.false), and the existingplatform-http:proxyforward-proxy mode (CAMEL-24455) is provably unaffected: its consumer path is"/", and the helper treats a consumer path of"/"as "nothing to strip".platform-http-component.adoc.JIRA: CAMEL-24625