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

content-encoding header should be removed #28

Closed
snytkine opened this issue Dec 14, 2020 · 6 comments · Fixed by #30
Closed

content-encoding header should be removed #28

snytkine opened this issue Dec 14, 2020 · 6 comments · Fixed by #30

Comments

@snytkine
Copy link

Return of this function is instance of http.IncomingMessage
The problem is that this function does not delete content-encoding header, so the result is that you have an IncomingMessage object with content-encoding header with original value like 'gzip' but the stream is no longer compressed.

I think the the function should also delete content-encoding header before returning the result stream

@sindresorhus
Copy link
Owner

That's a good find 👍🏻

@cardosob
Copy link

I think this change introduced a different issue.

The header "content-encoding" is removed from the original response object but the content is only decompressed in the finalStream object.

So you end up with a response object without the header and with the content still compressed.

In a context where this response object goes through multiple handlers, this is a problem.

@sindresorhus @snytkine

sindresorhus added a commit that referenced this issue May 28, 2024
And correctly remove `content-length` header.

Fixes #28 (comment)

Fixes #37
@sindresorhus
Copy link
Owner

@cardosob
Copy link

Thanks for looking into this @sindresorhus.

Is there any chance to release this as a fix for v7.0 ? i.e. 7.0.1 or simply 7.1.0

@sindresorhus
Copy link
Owner

While it is a bug fix, changing this in v7 (or v8) is simply too risky. Some users may depend on the current behavior.

@cardosob
Copy link

I understand. Thanks @sindresorhus ! 🙇

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

Successfully merging a pull request may close this issue.

3 participants