[
https://issues.apache.org/jira/browse/CAMEL-24446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107253#comment-18107253
]
Andrea Cosentino commented on CAMEL-24446:
------------------------------------------
Stopping short of implementing this, because it would reverse a documented
decision rather than fill a gap. Handing the call back.
*The prior decision.* {{camel-4x-upgrade-guide-4_18.adoc}} (and the 4_14 copy)
states, under the CAMEL-23588 entry:
{quote}
The constants in {{UndertowConstants}} ({{CONNECTION_KEY}},
{{CONNECTION_KEY_LIST}}, {{SEND_TO_ALL}}, {{EVENT_TYPE}}, {{EVENT_TYPE_ENUM}},
{{CHANNEL}}, {{EXCHANGE}}) keep their existing string values
({{websocket.connectionKey}}, {{websocket.connectionKey.list}},
{{websocket.sendToAll}}, etc.) because they are part of the undertow
component's externally-visible API contract; routes referencing them
(symbolically or by literal value) continue to work unchanged within an
undertow route.
{quote}
The same entry then documents precisely the cross-component case this issue
describes, and names the mitigation:
{quote}
Note that the {{HeaderFilterStrategy}} only governs the transport boundary; it
does not prevent cross-component header injection (for example, an {{http ->
undertow}} route where the HTTP consumer maps an attacker-supplied
{{websocket.connectionKey}} header into the exchange and the undertow producer
then reads it via {{in.getHeader(...)}}). For defence in depth at the trust
boundary, route authors should explicitly strip these headers from untrusted
inbound traffic
{quote}
{{UndertowHeaderFilterStrategy}}'s javadoc carries the same note. So this is a
known, accepted residual with a published mitigation, already shipped in
4.14.x, 4.18.x and 4.22.x - not an oversight.
*Why it is still worth a decision.* The argument for renaming is consistency,
not novelty: the sibling {{camel-atmosphere-websocket}} component *was* renamed
in the same wave, to {{CamelAtmosphereWebsocketConnectionKey}} /
{{...ConnectionKeyList}} / {{...SendToAll}} (see the tables at the top of the
4_18 and 4_14 guides). Both components expose the same three header names for
the same purpose, so it is not obvious why undertow's are an externally-visible
contract and atmosphere's were not. That asymmetry reads more like an accident
of sequencing than a principled distinction.
*What renaming would cost.* A second breaking rename of the same headers within
a few releases, for users who were explicitly told in 4.14/4.18 that these
values are stable. A dual-read deprecation period would soften that, but it
would not close the injection path - the producer falling back to the legacy
name is exactly what an injected header would exercise - so it buys migration
comfort and no security benefit.
*My recommendation:* leave the values as they are unless the PMC wants the
atmosphere/undertow inconsistency resolved. If it does, the rename should be
one deliberate change covering both the naming and the guide entry, made by a
maintainer who can weigh the second break - not folded in as a security fix.
I have not made any code change on this issue.
_Claude Code on behalf of oscerd_
> camel-undertow - websocket.* producer header constants sit outside the Camel
> namespace
> --------------------------------------------------------------------------------------
>
> Key: CAMEL-24446
> URL: https://issues.apache.org/jira/browse/CAMEL-24446
> Project: Camel
> Issue Type: Improvement
> Components: camel-undertow
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.23.0
>
>
> {{UndertowConstants}} defines the websocket dispatch headers outside the
> {{Camel}} prefix:
> {code:java}
> public static final String CONNECTION_KEY = "websocket.connectionKey";
> public static final String CONNECTION_KEY_LIST =
> "websocket.connectionKey.list";
> public static final String SEND_TO_ALL = "websocket.sendToAll";
> {code}
> {{UndertowProducer}} reads them straight off the message with
> {{in.getHeader()}}. Every HTTP consumer's default {{HeaderFilterStrategy}}
> filters names starting with {{Camel}}/{{camel}} only, so these names pass
> through untouched from any other consumer in the route.
> CAMEL-23588 addressed the intra-component case -
> {{UndertowHeaderFilterStrategy}} strips {{websocket.*}} at undertow's own
> consumers - and its javadoc explicitly records the remaining cross-component
> case: a message entering through jetty, servlet, netty-http or platform-http
> and routed to an undertow websocket producer still carries whatever
> {{websocket.*}} headers it arrived with.
> This is the same naming issue the CAMEL-23577 sweep addressed across other
> components; undertow was not included at the time because {{websocket.*}} is
> part of the component's documented external contract.
> Proposal: introduce {{CamelUndertowWebSocket*}} spellings, read both for one
> minor release with the old ones deprecated, then drop the unprefixed forms.
> Needs an upgrade-guide entry.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
