Guard Undertow port() against internal NPE - #12373
Conversation
HttpServerExchange.getHostPort() can NPE inside Undertow itself when there's no Host header, the scheme isn't http/https, and the connection's local address isn't an InetSocketAddress (e.g. AJP or a Unix domain socket transport). Fall back to 0 (the existing "no port" sentinel used throughout URIUtils) rather than letting the NPE propagate into the instrumented application. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
🎯 Code Coverage (details) 🔗 Commit SHA: 37f9d28 | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| public int port() { | ||
| return httpServerExchange.getHostPort(); | ||
| try { | ||
| return httpServerExchange.getHostPort(); |
There was a problem hiding this comment.
I'm debating how best to add a test.
It could be done via a mock, but in my mind, that does prove much.
To create the problem for real, requires a rather specific undertow set-up, but maybe I (Claude) can figure out how to reconstruct that scenario.
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
dd-octo-sts
Bot
added
the
tag: ai generated
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new port catch does not stop the null pointer failure. The same request then calls peerPort(), which calls getPort() on the same null destination address outside the URL catch.
🤖 Datadog Autotest · Commit 37f9d28 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| } catch (final NullPointerException e) { | ||
| // Undertow's getHostPort() can NPE internally (e.g. no Host header and a connection whose | ||
| // local address isn't an InetSocketAddress, such as AJP or a Unix domain socket transport). | ||
| return 0; |
There was a problem hiding this comment.
The original request still causes a null pointer exception and stops later request decoration.
Assertion details
- Input: A request has no usable Host port, uses a scheme other than HTTP or HTTPS, and has a null destination address.
- Expected:
The decorator must omit the unavailable port and continue without an exception. - Actual:
port() returns 0. onRequest() then calls peerPort(), which calls getPort() on the null destination address outside the URL catch.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
There was a problem hiding this comment.
This makes sense.. we should probably instead pass some indication to skip the unavailable port or pass the error on elsewhere?
2 participants
What Does This Do
Guards
HttpServerExchangeURIDataAdapter.port()against aNullPointerExceptionthrown from inside Undertow's ownHttpServerExchange.getHostPort().getHostPort()'s internal fallback chain is: parse theHostheader → default port forhttp/httpsscheme →getDestinationAddress().getPort(). That last fallback NPEs whenServerConnection.getLocalAddress(InetSocketAddress.class)returnsnull— which happens when there's no (parseable)Hostheader, the scheme is neitherhttpnorhttps, and the connection isn't backed by a plainInetSocketAddress(e.g. AJP, a Unix domain socket transport, or a wrapped/detachedServerConnection).Since this is a bug in Undertow's own fallback logic rather than something we can validate in advance without re-implementing that logic ourselves, the fix wraps the call and falls back to
0— the same "no port" sentinelURIUtilsalready treats as "omit the port" (port <= 0).Motivation
Fixes a NullPointerException reported in Error Tracking (issue, first seen 2025-10-16, regressed 2025-11-19, still recurring):
Additional Notes
No new unit test:
HttpServerExchangeis afinalUndertow class, and this repo pins Mockito 4.4.0 (the inline mock-maker needed to mock final classes isn't a default/existing dependency here). Addingmockito-inlinejust to force an artificial NPE through a mock wouldn't meaningfully validate the real failure mode anyway (no Host header + non-http/https scheme + non-InetSocketAddressconnection) — reliably reproducing that combination would need an integration test against a non-standard connector (e.g. AJP), which is out of scope for this quick fix.🤖 Generated with Claude Code
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueJira ticket: [PROJ-IDENT]