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 parsing of uneven stream bodies #1764

Merged
merged 5 commits into from Apr 4, 2018

Conversation

@jmcardon
Copy link
Member

@jmcardon jmcardon commented Apr 3, 2018

Closes #1755 .

Fixes a bug happening on uneven chunks + chunks that ended on a partial match with the boundary, but the boundary was contained in the middle of the chunk.

jmcardon added 2 commits Apr 3, 2018
//the chunk matches from the very beginning,
//since currstate can never be greater than
//(i + state).
val middleChunked = i + state - currState > 0

This comment has been minimized.

@jmcardon

jmcardon Apr 3, 2018
Author Member

Some background on this condition to help review:

state is essentially a counter to the last index of a particular byte sequence seen. This is necessary since boundaries may be spread across chunks. I also denote carry as a possible partial match on a boundary, which gets re-emitted if it happens that it wasn't, or ignored if it completed a boundary.

middleChunked is simply the condition of whether the particular match, for example a boundary match, starts somewhere after the start of this chunk (index 0). If that is the case, then we know that the previous possible partial match on the boundary was a false positive and we can reemit it.

i - (currState - state) > 0 is essentially the condition we are after: This means if the amount of the value to match is equal to i (which is the counter over the whole chunk until possibly completing the match), then clearly the match started at the beginning of the chunk, in which case we can ignore the previous carry. If this condition is greater than 0, this means that this boundary match began somewhere in the midde, thus it is middleChunked

@@ -274,7 +266,7 @@ object MultipartParser {
c: Chunk[Byte]
): (Int, Stream[F, Byte], Stream[F, Byte]) = {
val ixx = i - currState
if (middleChunked || state == 0) {

This comment has been minimized.

@jmcardon

jmcardon Apr 3, 2018
Author Member

state == 0 condition was removed because it is not longer necessary: this case will be picked up as a middleChunked case, now always.

@@ -318,7 +351,7 @@ object MultipartParser {
} else if (currState == len) {
splitCompleteMatch(middleChunked, currState, i, acc, carry, c)
} else {
splitPartialMatch(middleChunked, currState, i, acc, carry, c)
splitPartialMatch0(middleChunked, currState, i, acc, carry, c)

This comment has been minimized.

@jmcardon

jmcardon Apr 3, 2018
Author Member

Sux but I had to do this because MiMa :(.

I messed up not making these private in the first place.

@rossabaker
Copy link
Member

@rossabaker rossabaker commented Apr 3, 2018

We can release this as 0.18.6 tonight. I'll give @ChristopherDavenport a chance to peruse it.

Copy link
Member

@ChristopherDavenport ChristopherDavenport left a comment

👍 let's start cutting.

jmcardon and others added 2 commits Apr 4, 2018
@jmcardon
Copy link
Member Author

@jmcardon jmcardon commented Apr 4, 2018

[error] Error: Total 51, Failed 0, Errors 1, Passed 48, Skipped 2, Pending 1
[error] Error during tests:
[error] 	org.http4s.client.blaze.ClientTimeoutSpec

wat

@rossabaker rossabaker merged commit ecbb5ca into http4s:release-0.18.x Apr 4, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants