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

Add requests/urllib3 work-around for intercepting gzipped bytes. #36

Merged
merged 2 commits into from
Oct 20, 2017

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 20, 2017

Fixes #34.

/cc @tseaver @mfschwartz

@jonparrott Do you think we should cc the requests / urllib3 maintainers? I'm also working on a PR to fix the failing system test, but it is not going to be part of this PR.

@theacodes
Copy link

Go for it.

@@ -18,6 +18,8 @@
import hashlib
import logging

import urllib3.response

This comment was marked as spam.

@mfschwartz
Copy link

I would suggest that rather than just Cc'ing the requests / urllib3 maintainers, we recommend adding a feature to their libraries so the caller can get ahold of the pre-decoded content to do things like computing checksums. I think that capability has more general value than just for this library.

since the caller will no longer need to hash to decoded bytes.
"""
encoding = response_raw.headers.get(u'content-encoding', u'').lower()
if encoding != u'gzip':

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 20, 2017

@Lukasa @shazow We're curious what you think of this hack.

For Google Cloud Storage (and likely other media APIs with checksumming) we "need" simultaneous access to the (gzip) compressed and uncompressed bytes so we can compute the checksum of the gzipped content (which is what is actually stored in "the cloud") while streaming the uncompressed bytes to a stream for the user.

@dhermes dhermes merged commit b6c62d8 into master Oct 20, 2017
@dhermes dhermes deleted the gzip-decoder branch October 20, 2017 21:18
@mfschwartz
Copy link

Opened #37 about the deflate encoding issue noted above.

@shazow
Copy link

shazow commented Oct 21, 2017

@dhermes Looks like you did what you could with the API we have. :) It really does feel like the decoder stuff should be a registry, doesn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MD5 validation broken?
5 participants