allthingssecurity opened a new pull request, #28263:
URL: https://github.com/apache/camel/pull/28263

   # Description
   
   [CAMEL-25842](https://issues.apache.org/jira/browse/CAMEL-25842)
   
   When the connection of `XmppConsumer` is lost, two things try to connect it 
again: the consumer's connection monitor (every `connectionPollDelay` seconds) 
and Smack's `ReconnectionManager`, which `XmppComponent.doStart` enables for 
every connection (its first attempt comes after a random delay of 2 to 14 
seconds). The `ReconnectionManager` logs in after it connected, but it gives up 
when the monitor connected first, and the monitor only called 
`connection.connect()`, which does not log in with Smack 4.4. An XMPP server 
delivers nothing to a stream that did not log in and bind a resource, so when 
the monitor won the race the consumer reported a working connection and 
consumed nothing again (private chats and rooms) until the route was restarted. 
And whoever logged in again, the consumer did not enter its room again, 
although the server removes the occupant from a room when its connection ends: 
a room consumer never received a room message again after a lost connection.
   
   Changes:
   - `XmppConsumer.checkConnection`: when the connection is connected but not 
authenticated, log it in again (not when `login=false`).
   - `XmppConsumer`: a `ConnectionListener` whose `authenticated(connection, 
resumed)` reacts to every fresh login of the connection (not to a resumed 
stream), by the monitor or by the `ReconnectionManager`: it logs "Logged in 
again" at INFO and enters the room again (same nickname and room password) in 
the monitor thread, not in the thread that logged in. If entering fails, the 
monitor tries again at its next check. Nothing is logged or done when no login 
happened.
   - `XmppConsumer.checkConnection`: the monitor and the `ReconnectionManager` 
can connect and log in the same connection at the same time. Smack 4.4.8 makes 
`connect()` and `login(...)` synchronized and throws 
`AlreadyConnectedException` / `AlreadyLoggedInException` to the second caller 
(the `ReconnectionManager` itself catches both). The monitor now takes either 
as done by the other one (DEBUG), and goes on with the login after 
`AlreadyConnectedException`; before, it logged a WARN with a stack trace and, 
after `AlreadyConnectedException`, skipped the login.
   - `XmppEndpoint.login`: the login part of `createConnection`, as the 
configured user or anonymously. `createAccount` still only applies to the first 
connection.
   
   The producers' reconnect is not changed (a send on such a connection fails 
loudly there). No option changed, so no upgrade guide entry.
   
   Tests:
   - New `XmppConsumerReconnectTest` (2 tests) with `FakeXmppServer`, a small 
in-process XMPP server (plain TCP, SASL PLAIN and ANONYMOUS, resource binding, 
room presence) that, like a real server, delivers stanzas only to sessions that 
logged in and to the room's occupants. The real Smack client connects to it; 
the tests drop every connection on the server. With `connectionPollDelay=1` the 
consumer's monitor connects first. Without the change both fail (`The consumers 
did not log in again ==> expected: <2> but was: <0>`, `The consumer did not 
enter the room again ==> expected: <1> but was: <0>`); with the change both 
pass.
   - New `XmppConsumerSmackReconnectTest`: the `ReconnectionManager` wins the 
race (fixed delay of 1 s, `connectionPollDelay=3600`), connects and logs in. 
Without the change, and with only the monitor re-entering the room, it fails 
(`The consumer did not enter the room again ==> expected: <1> but was: <0>`); 
with the change it passes.
   - New `XmppConsumerConnectionRaceTest` (2 tests, no server): a connection 
whose `connect()` throws `AlreadyConnectedException` is still logged in by the 
monitor, and a login that throws `AlreadyLoggedInException` is no failure of 
`checkConnection`. Without that part of the change both fail (`The monitor did 
not log the connection in ==> expected: <[[not-authenticated] (1)]> but was: 
<[]>`, `Unexpected exception thrown: ...AlreadyLoggedInException`).
   - camel-xmpp unit tests with the change: 14 run, 0 failures, 5 skipped.
   - Merged with the change for CAMEL-25841 (one connection per account), the 
camel-xmpp unit tests pass too (20 run, 0 failures, 5 skipped).
   
   Note: when Smack still thinks it is in the room, `MultiUserChat.join` first 
leaves it and waits for the server's answer; a server that does not answer the 
leave of a session that is gone delays the new entry by Smack's reply timeout 
(5 s by default).
   
   How it was found: variant analysis of CAMEL-25815 (server-side state, here 
the logged-in session and the room occupancy, lost on a reconnect and not 
created again), then the JUnit test. `XmppRobustConnectionTest` has been 
disabled since Smack 4.2 with the note that the reconnect does not seem to 
work; this is a likely cause, but I did not re-enable it (it needs the 
container).
   
   What was not run: a real XMPP server (the container tests); the full root 
build.
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (Camel 4 
uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked 
there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for 
the change (usually before you start working on it): 
[CAMEL-25842](https://issues.apache.org/jira/browse/CAMEL-25842).
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful 
subject line and body.
   - [ ] I have run `mvn clean install -DskipTests` locally from root folder 
and I have committed all auto-generated changes.
     (I built and tested camel-xmpp. No option and no component page changed. I 
did not run the full root build.)
   
   # AI-assisted contributions
   
   - [x] If this PR includes AI-generated code, commits have proper 
co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR 
description identifies the AI tool used.
     This PR was prepared with Claude Code (Claude Opus 5.5). The commit 
carries a `Co-Authored-By` trailer.
   
   _Claude Code on behalf of allthingssecurity_
   
   🤖 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to