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

Keep compatible with em-http-request > 1.1.3 #167

Closed

Conversation

damned
Copy link

@damned damned commented Jul 19, 2016

When puffing billy makes a proxy request, it appears to try to force accepting only raw (identity?) encoding and avoid e.g. gzip'd response.

It has been doing this by deleting the accept-encoding header.

em-http-request 1.1.4 has changed its default behaviour to default accept-encoding header if not already set. igrigorik/em-http-request@8c72228

this PR attempts to fix by forcing accept-encoding to empty value instead which effectively explicitly tells em-http-request to not request an encoding

damned added 2 commits July 19, 2016 18:15
Trying to reproduce original behaviour, not sure correect as there is
no documenting spec for the header delete...

But looks like trying to avoid e.g. gzip request, however em-http-request
default behaviour changed in 1.1.4 to default to use gzip if header is
missing.
@ronwsmith
Copy link
Collaborator

Thanks @damned for the contribution. Will let original reporter have a chance to try it out before I merge.

@ronwsmith
Copy link
Collaborator

@damned can you resolve the conflicts if you'd like to maintain your encoder heading change?

@ryansch
Copy link
Contributor

ryansch commented Apr 19, 2018

@damned @ronwsmith I just hit my head on this. I can confirm that the change in this PR fixes the problem in #204 which appears to be unrelated to https usage.

@ronwsmith
Copy link
Collaborator

@ryansch feel free to submit a new PR with the encoder header change. We haven't heard back from @damned in a while.

@ronwsmith
Copy link
Collaborator

Stale PR, closing. Feel free to reopen and continue if this is still an issue.

@ronwsmith ronwsmith closed this Jan 10, 2019
@zjohl zjohl mentioned this pull request Mar 15, 2019
@ronwsmith
Copy link
Collaborator

ronwsmith commented Mar 17, 2019

Fixed in v2.1.0

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