Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for some invalid server behaviors when a client is aborting a request #11637

Merged
merged 11 commits into from
Apr 23, 2024

Conversation

lorban
Copy link
Contributor

@lorban lorban commented Apr 9, 2024

Fix three bugs that were discovered at the same time:

  1. Fix java.lang.IllegalArgumentException: demand pending warning log. When a ReadListener is registered on a EE10 ServletInputStream after startAsync() has been called, and the client abruptly closes the connection, the server is left with a pending demand and trying to complete the HttpChannelState's callback. The latter expects no pending demand otherwise IllegalArgumentException: demand pending is thrown. We should fail the callback with the IllegalArgumentException instead of throwing it.

  2. Add the necessary wiring so that QUIC can fail its session when a HTTP 3 client aborts. A QUIC reset packet is immediately sent to the server which is supposed to react to it; when the QUIC endpoint has fill interest, this reset is passed to the HTTP 3 layer which does the right thing; but when the endpoint has no fill interest, the reset is incorrectly ignored.

  3. Abort instead of throwing when the exception handler in EE10 ServletChannel.handle() throws an exception to avoid infinitely looping and adding an infinite amount of suppressed stack traces when the SEND_ERROR action itself throws.

See #11631

@lorban lorban added the Bug For general bugs on Jetty side label Apr 9, 2024
@lorban lorban self-assigned this Apr 9, 2024
@lorban lorban requested review from gregw and sbordet April 12, 2024 13:37
@lorban
Copy link
Contributor Author

lorban commented Apr 12, 2024

I could not find the cause of the HttpFields corruption that makes HttpFields.Mutable.remove() throw NPE, but in the process of trying to reproduce the issue I found two other bugs:

  • making the client abort while the servlet has started async, has registered an input listener and isn't processing makes the EE10 servlet state machine fail with ISE.
  • making the client abort a HTTP3 stream is ignored by the server.

I've committed fixes for those.

@lorban lorban requested a review from sbordet April 17, 2024 14:28
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban requested a review from gregw April 18, 2024 09:32
Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban marked this pull request as ready for review April 19, 2024 09:06
@lorban lorban added the Sponsored This issue affects a user with a commercial support agreement label Apr 23, 2024
@lorban lorban changed the title Mitigate infinite loop when ServletContextResponse.resetContent() throws Fix for some invalid server behaviors when a client is aborting a request Apr 23, 2024
@joakime joakime merged commit a1450f5 into jetty-12.0.x Apr 23, 2024
8 of 10 checks passed
@joakime joakime deleted the fix/jetty-12.0.x/11631-fix-npe-infinite-loop branch April 23, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants