HTTPCORE-799: Reset async entity producer state after interrupted writes - #698
Merged
Merged
Conversation
Discard buffered but unwritten bytes and partial digest state when async entity producers are released, so repeating an interrupted production starts cleanly from the beginning. Add regression tests for file, path, string, repeatable binary, and digesting producers.
Member
|
@mkurz That was quite sloppy on my part. Thank you for finding and reporting the defects. |
Member
|
Cherry-picked to |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This fixes HTTPCORE-799.
This was found while testing an HTTP/2 authentication retry in which a server returned a
401response and then reset the stream withRST_STREAM(NO_ERROR). The request producer was released mid-write and then reused for the retry.Repeatable async entity producers could retain state from an interrupted write when
releaseResources()was called. Although the source was reset for a replay, internal staging buffers could still contain the unwritten tail, and a digesting decorator could still contain the bytes accepted before the interruption.The affected producers now reset their per-production state when resources are released:
FileEntityProducerandPathEntityProducerclear their file buffers.AbstractCharAsyncEntityProducerclears its encoded-byte buffer, fixingStringAsyncEntityProducerand other repeatable character producers.AbstractBinAsyncEntityProducerclears its byte buffer, fixing repeatable binary subclasses.DigestingEntityProducerresets itsMessageDigestbefore releasing its wrapped producer.Regression tests cover file, path, string, repeatable binary, and digesting producers. Before the buffer fixes, replaying
abcdefafter onlyabchad been accepted produceddefabcdef; after the fixes it produces exactlyabcdef. Before the digest fix, the replay body was correct but its MD5 was calculated overabcabcdefinstead ofabcdef.The same affected code is present on the
5.4.xbranch, so this change is also a candidate for backporting.Tests:
./mvnw -pl httpcore5 -Dtest=TestStringAsyncEntityProducer,TestAbstractBinAsyncEntityProducer,TestFileAsyncEntityProducer,TestPathAsyncEntityProducer test— 13 tests passed./mvnw -pl httpcore5 -Dtest=TestDigestingEntityProducer test— 2 tests passed./mvnw -pl httpcore5 test— 1,415 tests passed./mvnw -pl httpcore5 verify -DskipTests— checkstyle, Animal Sniffer, and japicmp passed