InvalidArgumentExceptions due to invalid status codes are not handled properly #11749
Comments
|
In this example, From the point of view of Consider a valid response (e.g. with a valid status code), and listener implementation such as: public void onHeaders(Response response) {
System.err.println("HEADERS");
throw new NullPointerException();
}From the point of view of There is an explicit API to abort a request or a response, so applications should use that. What we can do is to be more aggressive at parsing a response with a status code < 100 or >= 600, as per RFC 9110, and generate a parser error. @gregw WDYT? Should we be more strict at parsing status codes? However, in general, if an event listener implementation can throw, it should catch and decide what to do: sometimes the error is recoverable, other times it is fatal and the abort APIs should be used. |
|
Probably a good idea to be extra strict on the response line. After all, what would happen if the HttpClient decided to connect to something that isn't HTTP? (eg: an SMTP server - |
|
@sbordet I think HttpParser could check for a 3 digit status code, as the RFC says they are 3 digit: https://www.rfc-editor.org/rfc/rfc9110.html#section-15 |
Fix #11749 by detecting more bad status codes
Jetty version(s)
Jetty
11.0.20Jetty ReactiveStream HttpClient
1.1.13Java version/vendor
(use: java -version)openjdk
17.0.10OS type/version
Windows
Description
If an http response contains an invalid status code <100, an
IllegalArgumentExceptionis thrown by Spring'sHttpStatusCodeinterface, when theResponseNotifiertries to execute theonHeadersmethod. TheonHeadersmethod of Jetty ReactiveStream Client'sResponseListenerProcessorclass, which is one of the registered listeners that are notified, applies the content function, whichcalls the constructor of
JettyClientHttpResponse. The problem only surfaced after this commit was introduce to Spring, which introduced aHttpStatusCode.valueOfmethod call in theJettyClientHttpResponseconstructor to create a status code object, which then throws the exception mentioned above.The problem is, that the
ResponseNotifiercatches all exceptions, logs them and doesn't terminate the exchange nor propagates the exception to the caller. Therefore http calls with status codes <100 run into a timeout resulting in aTimeoutException, with no indication of the real cause.The following Exception is logged (see log excerpt for further details incl. timeout exception):
How to reproduce?
Log excerpt
The text was updated successfully, but these errors were encountered: