Reproducer for request failures present in 2.2.25 and 2.3.7. #1495
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I attempted a simpler reproducer without servlets and jersey, however I was unable to reproduce the issue. Similarly, when I read the response without parsing JSON, I failed to reproduce the issue. Parsing response JSON may be adding some backpressure necessary to trigger the failure.
|
@carterkozak - is there a ticket for those failures? Also, could you please rebase it? |
fl4via
added
the
under verification
|
After a big void dealing with CVEs and releases, we are finally back to this one. I'll be away during next week, but then I'll pick up from here in early July. |
fl4via
added
the
bug reproducer
carterkozak
added a commit
to carterkozak/undertow
that referenced
this pull request
Aug 12, 2024
The failure was the result of disagreement over the owner of a connection, after an initial thread ends an exchange and releases the connection, but continues to operate upon it with a premature `flush()` from an illegal thread (per undertow thread model). The HttpServerExchange uses a `WriteDispatchChannel` in order to avoid interfering with the underlying connection after the exchange is completed, however the `WriteDispatchChannel` is an outer layer, around all response-wrappers. Response wrappers themselves may still interact with the underlying connections even after the `HttpReadListener` state has been reset to allow another request to be parsed. We must wrap the lower level streams with a similar function to prevent this spillover. In practice, the failure I've encountered and reproduced was the result of `DeflatingStreamSinkConduit` invoking `next.terminateWrites()` followed by `next.flush()`. The `terminateWrites()` invocation causes the delegate `ChunkedStreamSinkConduit` to flush within `terminateWrites` which triggers the end of the exchange. Then, the following `flush()` in `DeflatingStreamSinkConduit` applies to another request entirely. This can be reproduced quickly using the reproducer branch (undertow-io#1495) with a slight code modification, adding a `sleep(100)` thusly: ```diff if (performFlushIfRequired()) { state |= NEXT_SHUTDOWN; freeBuffer(); next.terminateWrites(); + try { + Thread.sleep(100); + } catch (InterruptedException e) {} return next.flush(); } else { ```
This was referenced Aug 12, 2024
fl4via
removed
the
under verification
|
I'm closing this as the issue is solved by PR #1648 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I attempted a simpler reproducer without servlets and jersey, however I was unable to reproduce the issue. Similarly, when I read the response without parsing JSON, I failed to reproduce the issue. Parsing response JSON may be adding some backpressure necessary to trigger the failure.
I've shared this reproducer as a draft PR into the undertow repo for ease of use in debugging, I don't expect it to be merged.