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

Fixes #7818 - Regression: allow HttpChannel.Listener.onResponseBegin to modify response headers #7850

Merged

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Apr 6, 2022

Fixes #7850 - A regression in HttpChannel.Listener.onResponseBegin with the ability to modify response headers immediately before commit.

See PR #7851 for Jetty 9.4.x test demonstrating behavior. (Jetty 9.4.x does not need a code change)

Signed-off-by: Joakim Erdfelt joakim.erdfelt@gmail.com

…sponse headers

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime added the Bug For general bugs on Jetty side label Apr 6, 2022
@joakime joakime requested a review from gregw April 6, 2022 16:47
@joakime joakime added this to In progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 via automation Apr 6, 2022
@joakime joakime self-assigned this Apr 6, 2022
@joakime joakime moved this from In progress to Review in progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 Apr 6, 2022
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
uschindler
uschindler previously approved these changes Apr 6, 2022
Copy link

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

Fix looks fine to me and test also replicates what I did in my code.

Did you test if the test succeeded in 9.4 to confirm it was a regression?

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor Author

joakime commented Apr 6, 2022

Fix looks fine to me and test also replicates what I did in my code.

Did you test if the test succeeded in 9.4 to confirm it was a regression?

The 9.4.x test case which shows that this works is in PR #7851

@joakime joakime changed the title Fixes #7818 - allow HttpChannel.Listener.onResponseBegin to modify response headers Fixes #7818 - Regression: allow HttpChannel.Listener.onResponseBegin to modify response headers Apr 6, 2022
@joakime joakime added this to the 10.0.x milestone Apr 6, 2022
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

Not building? But looks good other than that.

Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Review in progress to Reviewer approved Jun 1, 2022
@joakime joakime merged commit 2850db1 into jetty-10.0.x Jun 1, 2022
Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Reviewer approved to Done Jun 1, 2022
@joakime joakime deleted the fix/jetty-10.0.x-7818-onresponsebegin-modify-headers branch June 1, 2022 18:12
olamy added a commit that referenced this pull request Oct 9, 2022
Signed-off-by: Olivier Lamy <olamy@apache.org>
olamy added a commit that referenced this pull request Oct 10, 2022
* simplify poms, add back missing changes from #7850
* remove non used plugin, missing from #7687


Signed-off-by: Olivier Lamy <olamy@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants