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.

Sign up for GitHub

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

Jump to bottom

deflate-encoded content will fail md5 checksum #37

Closed
mfschwartz opened this issue Oct 20, 2017 · 1 comment · Fixed by #87
Closed

deflate-encoded content will fail md5 checksum #37

mfschwartz opened this issue Oct 20, 2017 · 1 comment · Fixed by #87
Assignees
@tseaver
Labels
download priority: p2 Moderately-important priority. Fix may not be included in next release. requests-transport 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mfschwartz
Copy link

mfschwartz commented Oct 20, 2017 •
edited
Loading

#36 fixes the problem of MD5 checksum failures when content was stored with content-encoding:gzip, but we will have a similar problem for content with content-encoding:deflate because that fix only monkey patches the decoder for the gzip case.

Two options:

  1. Change accept-encoding to only be "gzip", rather than "gzip, deflate"
  2. Broaden the monkey patch approach started in pull/36 to support deflate as well.

(Side note: I've read that deflate is somewhat more CPU efficient than gzip, which I think is more of a concern for browsers in general.)
@mfschwartz mfschwartz mentioned this issue Oct 20, 2017
Merged
@dhermes
Copy link
Contributor

dhermes commented Oct 20, 2017

I vote for option 2, it can be done in the same fashion by subclassing urllib3.response.DeflateDecoder.

@dhermes dhermes self-assigned this Oct 20, 2017
@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. 🚨 This issue needs some love. labels Dec 8, 2018
@tseaver tseaver added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Jan 16, 2019
@JustinBeckwith JustinBeckwith added the 🚨 This issue needs some love. label Jan 22, 2019
IlyaFaer pushed a commit to MaxxleLLC/google-resumable-media-python that referenced this issue Jul 8, 2019
deflate-encoded content will fail md5 checksum googleapis#37
5154431
This was referenced Jul 9, 2019
Closed
Closed
Closed
tseaver added a commit that referenced this issue Aug 14, 2019
@tseaver
Always use 'raw' response data.
613ea8a 
Allowing 'requests' to expand / inflate the file's body defeats downstream
excpectations.

Closes #37.
Closes #49.
Closes #50.
@tseaver tseaver mentioned this issue Aug 14, 2019
Merged
@crwilcox crwilcox assigned tseaver and unassigned dhermes Aug 15, 2019
tseaver added a commit that referenced this issue Aug 27, 2019
@tseaver
Always use raw response data. (#87)
2b9ffc8 
Don't expect 'Content-Length' header for chunked downloads.

Closes #37.
Closes #49.
Closes #50.
Closes #56.
Closes #76.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tseaver tseaver

Labels
download priority: p2 Moderately-important priority. Fix may not be included in next release. requests-transport 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Always use raw response data. googleapis/google-resumable-media-python
4 participants
@tseaver @mfschwartz @dhermes @JustinBeckwith