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

Added handling of a "Content-Encoding" response of gzip or x-gzip #109

Closed
wants to merge 2 commits into from

Conversation

cgorshing
Copy link

I'm writing up an ueberauth strategy for StackOverflow and all their responses are gzip'd. For now, hackney doesn't handle this situation (see below for other issues pointing to it). So I need oauth2 to gunzip the body so it can read the StackOverflow response correctly. I've added tests for this case as well.

Assuming hackney removes the header when it handles the encoding, then this code should still be fine.

edgurgel/httpoison#81
benoitc/hackney#155
benoitc/hackney#456

@cgorshing
Copy link
Author

hmmm - That build failure doesn't make sense. I really don't think that failure was caused by my latest commit.

@scrogson
Copy link
Member

@cgorshing hey there! Thanks for putting this PR together. I imagine that you created this PR because the automatic body parsing is failing (raising an error)?

While looking into content-encoding I found that the value can actually include a comma-separated list of values (i.e. gzip, chunked). In cases like that, it looks like this solution would also run into some issues.

I wonder if it might be better to allow the response body to pass through untouched in order to allow these custom encodings to be handled by the user instead of this library?

Let me know what you think.

@cgorshing
Copy link
Author

"handled by the user instead of this library" -- That is reasonable. If that is your stance, then I have work to do inside hackney.

Yes I'm having issues with the decode_response_body function. For this specific scenario, the "user" is really another library (i.e. a new ueberauth strategy I'm working on). If inside that strategy I can get at the body before ueberauth does, then we are fine. But I don't think that is the way it happens now.

If you don't feel like this belongs here, then please close and I'll look elsewhere.

@cgorshing
Copy link
Author

cgorshing commented May 21, 2018

Bringing this back up again:
"allow the response body to pass through untouched" -- Whatcha thinking? Possibly pass in a key/value in opts so in Request's case statement it will let the body pass on through without doing a process_body?

I might be able to use https://github.com/scrogson/oauth2#configure-a-serializer as well if the Content-Type and Content-Encoding was passed in. Using #92 is also an option if it were merged in.

@yordis
Copy link
Member

yordis commented Sep 1, 2019

@cgorshing there are some merge conflicts.

Is this PR still relevant?

@cgorshing cgorshing closed this Sep 2, 2019
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