allthingssecurity opened a new pull request, #29125: URL: https://github.com/apache/camel/pull/29125
# Description [CAMEL-26463](https://issues.apache.org/jira/browse/CAMEL-26463) `UndertowConsumer.doStop` always unregistered its path from the host, and `BaseService.stop()` runs `doStop()` also for a consumer that never started (a route with `autoStartup=false` that is removed) and for a consumer whose start failed. The path handlers count the registrations per path and method (`CamelMethodHandler`), so such a consumer took the registration of a running consumer of the same path: - route `running` serves `undertow:http://0.0.0.0:8080/dup`, route `idle` consumes the same path with `autoStartup(false)` and is removed: `/dup` answers 404 although `running` still runs, and when `/dup` was the only path of the server, `DefaultUndertowHost` stops the server, so `running` gets "connection refused"; - a second consumer of the same path and method fails to start (the path already has a handler), and is then stopped and removed: the same. The same for a WebSocket consumer: a second consumer of a WebSocket path fails to start in `CamelWebSocketHandler.setConsumer` (the path already has a consumer). Its `doStop` (which can run twice for a failed consumer: once from the catch in `BaseService.start`, and again when the route is stopped) unregistered the shared handler, so the path went away and clients could no longer connect, and it called `setConsumer(null)` on the shared handler, which unset the running consumer (its clients' messages were then dropped). A WebSocket producer that never started (or whose start failed before it registered) unregistered the path of the consumer the same way when it was stopped. CAMEL-26083 fixed the same pairing for the rest-dsl consumers only (`RestRootHandler` adds and removes the rest path only when the consumer was added or removed). ## The fix `UndertowConsumer` remembers what it did in `doStart` and undoes only that in `doStop`, as `RestRootHandler` does for the rest-dsl consumers: - `registered`: set once `registerEndpoint` returned (for HTTP and WebSocket), so `unregisterEndpoint` runs only for a consumer that registered. A WebSocket consumer whose `setConsumer` failed did register (the shared handler counts it), so it still unregisters. - `webSocketConsumerSet`: set once `setConsumer(this, ...)` returned, so `setConsumer(null)` runs only for the consumer that the handler has. Both are reset in `doStop`, so a route that is stopped and started again registers and unregisters as before. Both are written and read in `doStart`/`doStop`, which `BaseService` runs under its lock, so they are plain fields. As a consumer whose registration fails no longer unregisters, a failed registration must leave nothing behind: - `CamelMethodHandler.add` is all or nothing: with `httpMethodRestrict=GET,POST` on a path whose `POST` is taken, `GET` was registered before `POST` failed, and stayed registered for a consumer that never runs (before, the unconditional unregister removed it, together with the `POST` of the running consumer). It now removes the methods it added before rethrowing. - `UndertowComponent.registerEndpoint` removes the registration info it added to its `handlers` set (used for the `Allow` header of `OPTIONS`) when the host refuses the registration (only if no other endpoint had added an equal one); without this, the `Allow` header of the running route listed the methods of the failed one (`POST,GET,POST,OPTIONS` instead of `POST,OPTIONS`). - `UndertowProducer` unregisters its WebSocket handler only when it registered it (a `registered` flag, as in the consumer; the handler field is kept, as an exchange sent while the producer stops still reads it). Not part of this change: the case of the JIRA with a path template next to a static path of the same base path (`/orders/{id}` and `/orders`) needs CAMEL-26460 for `/orders` to reach the static route at all; with both, a never-started second `/orders` route no longer takes the static path away, and its test goes into whichever of the two PRs is merged second. This branch does not depend on that one and merges cleanly with it. Also not changed (the same on main, a user error): `httpMethodRestrict=GET,GET` on a new path fails to start and leaves an empty method handler for the path. The other HTTP consumers already undo only what they registered (servlet and jetty `HttpConsumer` with `canConnected` and removal by identity, netty-http and platform-http by identity, vertx-websocket with its set of connected consumers). ## Behaviour changes Only the broken cases: a running consumer keeps its path (and its server) when another consumer of the same path that never started, or that failed to start, is removed. No upgrade guide entry (no working route behaves differently). Backport: `UndertowConsumer.doStart`/`doStop` are the same on camel-4.22.x and camel-4.18.x apart from lines around them, so the change ports with small context fixes. ## Tests New `UndertowConsumerNotStartedUnregisterTest` (camel-undertow, JUnit assertions as the module; the existing `UndertowConsumerUnregisterTest` covers unregistering a running consumer directly and is unchanged): - a removed route that never started, next to a running route of the same path and another path: both still answer; - the same with the running route's path as the only path of the server: the server keeps running; - a route of the same path and method whose start failed, stopped and removed: the running route still answers; - a WebSocket route of the same path whose start failed, stopped and removed: a client of the running WebSocket route connects and its message reaches the route; once the running route is removed too, the path is gone (the failed consumer had registered with the shared handler, and unregistered); - a WebSocket producer of the path that never started, stopped: the running WebSocket route still gets messages; - a route with `httpMethodRestrict=GET,POST` whose start failed on `POST` (taken by a running route), stopped and removed: `OPTIONS` lists only the running route's methods, a route for `GET` alone can then start, and the `POST` route still answers; - a route whose start failed: a second start while the path is still taken fails too and keeps the running route's path, and it starts once the path is free; - control: stopping and starting the running route twice unregisters and registers its path each time. Without the fix (main's `UndertowConsumer`, `UndertowProducer`, `UndertowComponent` and `CamelMethodHandler` compiled over the branch), 7 of the 8 tests fail (the restart control passes on main too). Mutants, each failing its test: without the WebSocket consumer guard (the running consumer is unset, its message is dropped); with main's `CamelMethodHandler` (`GET` stays taken: "Duplicate handler for GET method"); with main's `UndertowComponent` (the `Allow` header); with main's `UndertowProducer`. - The new class 20 times in one JVM: 160 tests, 0 failures. - camel-undertow, all tests (`org.apache.camel.component.undertow.**`, also `rest` and `ws`): 263 tests, 0 failures, 1 skipped. (An earlier full run had one `BindException: Address already in use` in `UndertowHttpStreamingTest` when it started its route; it passed in the other full runs and 5 times alone.) - Formatter and import-sort: no changes. # 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-26463) filed for the change (usually before you start working on it): [CAMEL-26463](https://issues.apache.org/jira/browse/CAMEL-26463). # 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 the core modules and camel-undertow and ran the tests above, including the formatter and import-sort plugins. No generated file changes. 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]
