[
https://issues.apache.org/jira/browse/CAMEL-25841?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
shashank updated CAMEL-25841:
-----------------------------
Description:
{{XmppComponent.createEndpoint}} (main 20cdb309f7d0) keeps the endpoints it
creates in a map and returns a cached endpoint for any URI with the same key.
The key is built from the scheme, host, port and query only:
{code:java}
private String extractCacheKeyFromUri(String uri) throws URISyntaxException {
URI u = new URI(uri);
return u.getScheme() + "://" + u.getHost() + u.getPort() + u.getQuery();
}
{code}
The participant (the path) and the user (the user info) are not in the key. So
{{xmpp://alice@host:5222/bob@host?password=x}} and
{{xmpp://alice@host:5222/carol@host?password=x}} resolve to the same endpoint,
the one for bob, and every message sent to the second URI goes to bob. With two
users on the same server ({{xmpp://alice@host/...}} and
{{xmpp://dave@host/...}}, same query), the second URI gets the first user's
endpoint, so messages are sent from alice's account with alice's password. This
happens with plain {{to}} endpoints in two routes as well as with toD and a
participant taken from the message, and nothing in the logs shows it (the
endpoint is only logged at DEBUG as "Using cached endpoint"). The map is also a
plain HashMap that toD can use from several threads, and it keeps every
endpoint until the component stops.
*Fix*: remove the component's endpoint cache. The component creates a new
endpoint for each URI, like other components. The CamelContext already reuses
the endpoint of the same URI and stops the endpoints it holds, so the cache is
not needed for clean-up.
The endpoints of one account must still share one connection: with a connection
per endpoint, {{xmpp://bot@host/alice}} and {{xmpp://bot@host/carol}} log in
with the same JID and resource ({{Camel}} by default), the server closes the
older session with a {{conflict}} stream error, and Smack 4.4.8's
{{ReconnectionManager}} does not reconnect after {{conflict}}, so one endpoint
would stop working for good. (A distinct resource per endpoint is no fix: a
message to the bare JID goes to one resource only.) So the component keeps one
connection per account (host, port, service name, user, password, resource,
login, connection configuration), used by the consumers and producers of all
endpoints of that account and closed when the last of them stops; each endpoint
keeps its own participant, room and other settings. Consumers and producers
release the connection on stop instead of disconnecting it, and a consumer
removes its listeners from it.
*Tests*: new {{XmppComponentEndpointPerUriTest}} (3 tests, no XMPP server
needed). {{endpointPerParticipant}} fails on main: {{expected:
<carol@localhost> but was: <bob@localhost>}}. {{endpointPerUser}} fails on
main: {{expected: <dave> but was: <alice>}}. {{sameUriSameEndpoint}} passes on
main and with the fix. New {{XmppSharedConnectionTest}} (3 tests) with a small
in-process XMPP server ({{FakeXmppServer}}: plain TCP, SASL PLAIN and
ANONYMOUS, resource binding, and the {{conflict}} stream error for a second
session of the same JID and resource); the real Smack client connects to it.
{{endpointsOfOneAccountShareItsConnection}} (two endpoints of one account send
to alice and carol over one session) fails on main ({{expected: <[hello alice]>
but was: <[hello alice, hello carol]>}}) and with a connection per endpoint
({{expected: <[hello alice]> but was: <[]>}}, the first session was closed with
{{conflict}}); {{stoppingOneEndpointKeepsTheConnectionOfTheOther}} and
{{consumerRestartedOnASharedConnectionReceivesEachMessageOnce}} check that a
stopped endpoint or consumer leaves the shared session to the others and that
the session is closed with the last route. camel-xmpp unit tests with the fix:
15 run, 0 failures, 5 skipped (the tests that need an XMPP server container);
the integration tests were not run.
*How it was found*: a sweep for components that keep endpoints or clients in
their own maps (state that grows per endpoint and outlives it), then the JUnit
test above.
*Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x, camel-4.14.x and
camel-4.10.x (the same cache key; read, not run).
_Filed with Claude Code on behalf of allthingssecurity._
was:
{{XmppComponent.createEndpoint}} (main 20cdb309f7d0) keeps the endpoints it
creates in a map and returns a cached endpoint for any URI with the same key.
The key is built from the scheme, host, port and query only:
{code:java}
private String extractCacheKeyFromUri(String uri) throws URISyntaxException {
URI u = new URI(uri);
return u.getScheme() + "://" + u.getHost() + u.getPort() + u.getQuery();
}
{code}
The participant (the path) and the user (the user info) are not in the key. So
{{xmpp://alice@host:5222/bob@host?password=x}} and
{{xmpp://alice@host:5222/carol@host?password=x}} resolve to the same endpoint,
the one for bob, and every message sent to the second URI goes to bob. With two
users on the same server ({{xmpp://alice@host/...}} and
{{xmpp://dave@host/...}}, same query), the second URI gets the first user's
endpoint, so messages are sent from alice's account with alice's password. This
happens with plain {{to}} endpoints in two routes as well as with toD and a
participant taken from the message, and nothing in the logs shows it (the
endpoint is only logged at DEBUG as "Using cached endpoint"). The map is also a
plain HashMap that toD can use from several threads, and it keeps every
endpoint until the component stops.
*Fix*: remove the component's endpoint cache. The component creates a new
endpoint for each URI, like other components. The CamelContext already reuses
the endpoint of the same URI and stops the endpoints it holds, so the cache is
not needed for clean-up.
*Tests*: new {{XmppComponentEndpointPerUriTest}} (3 tests, no XMPP server
needed). {{endpointPerParticipant}} fails on main: {{expected:
<carol@localhost> but was: <bob@localhost>}}. {{endpointPerUser}} fails on
main: {{expected: <dave> but was: <alice>}}. {{sameUriSameEndpoint}} passes on
main and with the fix. camel-xmpp unit tests with the fix: 12 run, 0 failures,
5 skipped (the tests that need an XMPP server container); the integration tests
were not run.
*How it was found*: a sweep for components that keep endpoints or clients in
their own maps (state that grows per endpoint and outlives it), then the JUnit
test above.
*Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x, camel-4.14.x and
camel-4.10.x (the same cache key; read, not run).
_Filed with Claude Code on behalf of allthingssecurity._
> camel-xmpp - two endpoints that differ only in the participant or the user
> are the same endpoint, so messages go to the wrong participant or from the
> wrong account
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25841
> URL: https://issues.apache.org/jira/browse/CAMEL-25841
> Project: Camel
> Issue Type: Bug
> Components: camel-xmpp
> Reporter: shashank
> Assignee: shashank
> Priority: Major
>
> {{XmppComponent.createEndpoint}} (main 20cdb309f7d0) keeps the endpoints it
> creates in a map and returns a cached endpoint for any URI with the same key.
> The key is built from the scheme, host, port and query only:
> {code:java}
> private String extractCacheKeyFromUri(String uri) throws URISyntaxException {
> URI u = new URI(uri);
> return u.getScheme() + "://" + u.getHost() + u.getPort() + u.getQuery();
> }
> {code}
> The participant (the path) and the user (the user info) are not in the key.
> So {{xmpp://alice@host:5222/bob@host?password=x}} and
> {{xmpp://alice@host:5222/carol@host?password=x}} resolve to the same
> endpoint, the one for bob, and every message sent to the second URI goes to
> bob. With two users on the same server ({{xmpp://alice@host/...}} and
> {{xmpp://dave@host/...}}, same query), the second URI gets the first user's
> endpoint, so messages are sent from alice's account with alice's password.
> This happens with plain {{to}} endpoints in two routes as well as with toD
> and a participant taken from the message, and nothing in the logs shows it
> (the endpoint is only logged at DEBUG as "Using cached endpoint"). The map is
> also a plain HashMap that toD can use from several threads, and it keeps
> every endpoint until the component stops.
> *Fix*: remove the component's endpoint cache. The component creates a new
> endpoint for each URI, like other components. The CamelContext already reuses
> the endpoint of the same URI and stops the endpoints it holds, so the cache
> is not needed for clean-up.
> The endpoints of one account must still share one connection: with a
> connection per endpoint, {{xmpp://bot@host/alice}} and
> {{xmpp://bot@host/carol}} log in with the same JID and resource ({{Camel}} by
> default), the server closes the older session with a {{conflict}} stream
> error, and Smack 4.4.8's {{ReconnectionManager}} does not reconnect after
> {{conflict}}, so one endpoint would stop working for good. (A distinct
> resource per endpoint is no fix: a message to the bare JID goes to one
> resource only.) So the component keeps one connection per account (host,
> port, service name, user, password, resource, login, connection
> configuration), used by the consumers and producers of all endpoints of that
> account and closed when the last of them stops; each endpoint keeps its own
> participant, room and other settings. Consumers and producers release the
> connection on stop instead of disconnecting it, and a consumer removes its
> listeners from it.
> *Tests*: new {{XmppComponentEndpointPerUriTest}} (3 tests, no XMPP server
> needed). {{endpointPerParticipant}} fails on main: {{expected:
> <carol@localhost> but was: <bob@localhost>}}. {{endpointPerUser}} fails on
> main: {{expected: <dave> but was: <alice>}}. {{sameUriSameEndpoint}} passes
> on main and with the fix. New {{XmppSharedConnectionTest}} (3 tests) with a
> small in-process XMPP server ({{FakeXmppServer}}: plain TCP, SASL PLAIN and
> ANONYMOUS, resource binding, and the {{conflict}} stream error for a second
> session of the same JID and resource); the real Smack client connects to it.
> {{endpointsOfOneAccountShareItsConnection}} (two endpoints of one account
> send to alice and carol over one session) fails on main ({{expected: <[hello
> alice]> but was: <[hello alice, hello carol]>}}) and with a connection per
> endpoint ({{expected: <[hello alice]> but was: <[]>}}, the first session was
> closed with {{conflict}});
> {{stoppingOneEndpointKeepsTheConnectionOfTheOther}} and
> {{consumerRestartedOnASharedConnectionReceivesEachMessageOnce}} check that a
> stopped endpoint or consumer leaves the shared session to the others and that
> the session is closed with the last route. camel-xmpp unit tests with the
> fix: 15 run, 0 failures, 5 skipped (the tests that need an XMPP server
> container); the integration tests were not run.
> *How it was found*: a sweep for components that keep endpoints or clients in
> their own maps (state that grows per endpoint and outlives it), then the
> JUnit test above.
> *Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x, camel-4.14.x and
> camel-4.10.x (the same cache key; read, not run).
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
