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

Delete 'Accept-Encoding' header on proxy requests #1668

Closed
diegossilveira opened this issue May 28, 2014 · 4 comments
Closed

Delete 'Accept-Encoding' header on proxy requests #1668

diegossilveira opened this issue May 28, 2014 · 4 comments
Assignees
Milestone

Comments

@diegossilveira
Copy link

@diegossilveira diegossilveira commented May 28, 2014

I was looking at proxy.js source code and found the following statement:

// lib/proxy.js file, line 69
if (settings.onResponse) {
   delete options.headers['accept-encoding'];
}

Removing the 'Accept-Encoding' from the headers that are forwarded to the upstream endpoint can be harmful, since response compression is quietly disabled. Why is this done?

I can imagine the short answer is: "If the developer wants to handle the response himself, Hapi can help him disabling the response compression". If so, I believe it's not a good idea to consider the presence of an onResponse handler to disable response compression. Wouldn't it be better creating a new option on proxy, like "disableResponseCompression: true|false"?

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented May 28, 2014

This is mostly due to historical reasons. It used to be that when you define an onResponse handler, hapi gave you back the payload, not the res stream. So this was needed to solve encoding while hapi didn't know how to decode the response for you.

It doesn't make much sense now, but at the same time, removing it isn't ideal. I think we need a new option that lets you set any other encoding you accept and the default will be to remove it. This way it is backwards compatible for now.

@hueniverse hueniverse self-assigned this May 28, 2014
@diegossilveira
Copy link
Author

@diegossilveira diegossilveira commented May 28, 2014

Great! I believe that creating a new option would solve the problem.
Thank you very much!

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Jun 10, 2014

Going the the flip version of a new acceptEncoding option that defaults to true.

@Marsup Marsup added feature and removed request labels Sep 20, 2019
@lock
Copy link

@lock lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants