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

Netty force encodes already encoded responses #6785

Closed
wants to merge 1 commit into from

Conversation

@NSProgrammer
Copy link

commented May 26, 2017

Motivation:

Fix the regression recently introduced that causes already encoded responses to be encoded again as gzip

Modification:

instead of just looking for IDENTITY, anything set for Content-Encoding should be respected and left as-is

added unit tests to capture this use case

Result:

Fixes #6784

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 26, 2017

Can you please sign the icla:

https://netty.io/s/icla

@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

signed

@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

all checks pass 💯

@mosesn

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

@NSProgrammer would you mind also updating one of your commit messages to the Motivation / Modification / Result style?

@normanmaurer thanks for taking a look! We really want to get this fix into the next release. This is blocking us from upgrading to 4.1.11.

@mosesn
mosesn approved these changes May 26, 2017
@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

@mosesn I don't know if that's possible to amend something already committed on github... does the comment not suffice?

@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

OK, added an empty commit with requested message

@CodingFabian

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

you can amend and force push. This is your branch (NSProgrammer:fix_forced_encode) and most likely nobody else is working on this. Github will automatically update the PR for you.

@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

I'm gonna wait on @normanmaurer to give direction. Would really like to unblock our use of latest Netty bits.

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 26, 2017

Fix 4.1.11 forced encoding bug
Motivation:

Fix the regression recently introduced that causes already encoded responses to be encoded again as gzip

Modification:

instead of just looking for IDENTITY, anything set for Content-Encoding should be respected and left as-is

added unit tests to capture this use case

Result:

Fixes #6784

@NSProgrammer NSProgrammer force-pushed the NSProgrammer:fix_forced_encode branch from da5fffd to f2ff647 May 26, 2017

@NSProgrammer

This comment has been minimized.

Copy link
Author

commented May 26, 2017

done

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 27, 2017

Cherry-picked into 4.1 (d56a756) and 4.0 (6655a23)

@NSProgrammer thanks!

@normanmaurer normanmaurer self-assigned this May 27, 2017

@normanmaurer normanmaurer added the defect label May 27, 2017

@normanmaurer normanmaurer added this to the 4.0.48.Final milestone May 27, 2017

@mosesn

This comment has been minimized.

Copy link
Contributor

commented May 27, 2017

@normanmaurer thanks so much for the quick turnaround!

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 27, 2017

@normanmaurer normanmaurer changed the title Fix forced encode Netty force encodes already encoded responses Jun 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.