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

HTTP: Don't attempt to needlessly decompress redirect body #2798

Closed
wants to merge 1 commit into from
Closed

HTTP: Don't attempt to needlessly decompress redirect body #2798

wants to merge 1 commit into from

Conversation

piru
Copy link

@piru piru commented Jul 26, 2018

This change fixes a regression where redirect body would needlessly be decompressed even though it was to be ignored anyway. As it happens this causes secondary issues since there appears to be a bug in apache2 that it in certain conditions generates a corrupt zlib response. The regression was created by commit: dbcced8

To reproduce the issue use a affectred curl version and attempt to:

curl --referer something --compressed -L http://morph.zone

With this change the problem disappears (read: the server bug is happily ignored).

This change fixes a regression where redirect body would needlessly be
decompressed even though it was to be ignored anyway. As it happens
this causes secondary issues since there appears to be a bug in apache2
that it in certain conditions generates a corrupt zlib response. The
regression was created by commit:
dbcced8

Discovered-by: Harry Sintonen
@@ -797,7 +797,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,
nread);
}
}
else
else if(!k->ignorebody)
result = Curl_unencode_write(conn, k->writer_stack, k->str, nread);
Copy link
Member

Choose a reason for hiding this comment

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

result is checked right after this in all cases so i'd add this

else
  result = CURLE_OK;

because what if it's set to CURLE_AGAIN and or something at that point, otherwise it's just going to return that and it shouldn't, (unless I'm reading it wrong)

Copy link
Member

Choose a reason for hiding this comment

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

It is set to CURLE_OK at the top of the function and then it gets checked every time it gets assigned so I don't think there's a risk for that...

@bagder
Copy link
Member

bagder commented Jul 31, 2018

Thanks!

@bagder bagder closed this in 1836d59 Jul 31, 2018
xquery pushed a commit to xquery/curl that referenced this pull request Aug 9, 2018
This change fixes a regression where redirect body would needlessly be
decompressed even though it was to be ignored anyway. As it happens this
causes secondary issues since there appears to be a bug in apache2 that
it in certain conditions generates a corrupt zlib response. The
regression was created by commit:
dbcced8

Discovered-by: Harry Sintonen
Closes curl#2798
falconindy pushed a commit to falconindy/curl that referenced this pull request Sep 10, 2018
This change fixes a regression where redirect body would needlessly be
decompressed even though it was to be ignored anyway. As it happens this
causes secondary issues since there appears to be a bug in apache2 that
it in certain conditions generates a corrupt zlib response. The
regression was created by commit:
dbcced8

Discovered-by: Harry Sintonen
Closes curl#2798
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants