Skip to content

Fix chunk and content-length encoding #759

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

Merged
merged 2 commits into from
May 15, 2017
Merged

Fix chunk and content-length encoding #759

merged 2 commits into from
May 15, 2017

Conversation

LoopinFool
Copy link

I used the 0.13-release branch because we're updating an existing application to Visual Studio 2017 and it would build, and build against the latest boost 1.64.

At first, things didn't work at all. After using the simple_wget sample to debug it, I found that a bunch of the content (starting with the headers) had been appended after the BODY. I narrowed it down and fixed the new EOF callback when the server has sent a content-length header.

Next, Visual Studio's iterator debugging found the problem with the "len + 2" statement where it could go beyond the end of the string if the buffer didn't yet contain the crlf terminator after the current chunk. After fixing that, things mostly worked but I encountered an occasional infinite loop. From that state I figured out that the existing logic couldn't handle having a partial chunk in the buffer when checking if the chunks have all been received.

The new chunk handling code isn't overly efficient, but my changes got it to work reliably without any major changes to the partial buffer handling.

LoopinFool and others added 2 commits May 11, 2017 12:29
Don't potentially access memory beyond the end of the string buffer.
In the case of a partial chunk, exit the loop. Continuing to search for crlf in the middle of a binary data chunk can result in an infinite loop, and wasn't the right logic.
@glynos
Copy link
Member

glynos commented May 14, 2017

LGTM.

Copy link
Member

@deanberris deanberris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@deanberris deanberris merged commit b930b8e into cpp-netlib:0.13-release May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants