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

Attempt to reset Servlet response before calling ExceptionHandlers #31104

Closed
wants to merge 1 commit into from

Conversation

aooohan
Copy link
Contributor

@aooohan aooohan commented Aug 24, 2023

Fix #30702

If any other changes are needed, feel free to ask me. : )

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 24, 2023
@bclozel bclozel self-assigned this Aug 29, 2023
@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 29, 2023
@bclozel bclozel added this to the 6.1.0-RC1 milestone Aug 29, 2023
@bclozel bclozel closed this in 1d7cbfa Aug 29, 2023
bclozel added a commit that referenced this pull request Aug 29, 2023
@bclozel
Copy link
Member

bclozel commented Aug 29, 2023

Thanks for your contribution @aooohan !

@aooohan aooohan deleted the gh-30702 branch August 30, 2023 01:29
bclozel added a commit to bclozel/spring-framework that referenced this pull request Aug 30, 2023
Revert previous changes for spring-projectsgh-31104 and apply them on DispatcherServlet
@bclozel
Copy link
Member

bclozel commented Aug 30, 2023

After discussing this with @rstoyanchev , we agreed that dealing with this at the DispatcherServlet level would be a better fit as it would cover all types of exception resolvers, at the Servlet level.

I've adressed that in 0d7c9b7.

bclozel added a commit that referenced this pull request Aug 30, 2023
This is a follow-up change related to gh-31104.
This change reverts the changes previously made in
`ExceptionHandlerExceptionResolver` and instead attempts to reset the
response directly in `DispatcherServlet` in order to cover all types or
exception handling.

Unlike the previous change, we decided to continue even if the response
was already committed: exception handlers will have a chance to be
called, even if it means they'll have to operate on a garbled response.
This change will cause less disruption, in case existing exception
handlers were relying on this behavior.

See gh-31104
bclozel added a commit that referenced this pull request Sep 8, 2023
Prior to this commit, `DispatcherServlet` would completely reset the
response (status, headers and body) before handling errors within Spring
MVC. This can cause unintended consequences when Servlet Filters added
response headers before the error happened. Such response headers might
be still required in case of error handling.

This commit changes the complete reset of the response to only resetting
the response buffer, if possible.

Closes gh-31154
See gh-31104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attempt to reset Servlet response before calling ExceptionHandlers
3 participants