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

fetch: fix content-encoding order #3343

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Conversation

tsctx
Copy link
Member

@tsctx tsctx commented Jun 18, 2024

When decoding, it must be processed from the reverse. We didn't catch this because the test is wrong.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina requested a review from KhafraDev June 18, 2024 21:23
@KhafraDev
Copy link
Member

At some point c415fbb got reverted... why?

@climba03003
Copy link
Contributor

climba03003 commented Jun 19, 2024

@KhafraDev
FYI, see #2061 for the reason why it being reverted.
This PR is implementing the correct order.

@KhafraDev
Copy link
Member

It seems like I mentioned this in https://github.com/nodejs/undici/pull/2061/files#r1305403864 and then never followed up on.

@KhafraDev KhafraDev merged commit e5c242d into nodejs:main Jun 19, 2024
30 checks passed
@tsctx tsctx deleted the fix/encoding-order branch June 19, 2024 03:52
@tsctx tsctx mentioned this pull request Jul 3, 2024
7 tasks
github-actions bot pushed a commit that referenced this pull request Oct 23, 2024
tsctx added a commit that referenced this pull request Oct 23, 2024
(cherry picked from commit e5c242d)

Co-authored-by: tsctx <[email protected]>
@Uzlopak Uzlopak mentioned this pull request Nov 7, 2024
@github-actions github-actions bot mentioned this pull request Dec 3, 2024
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.

5 participants