Keep the buffered response writer usable across responseReset() - #1033
Merged
tandraschko merged 1 commit intoJul 29, 2026
Merged
Conversation
Member
|
@BalusC checkstyle failed |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (length >= BUFFER_SIZE) | ||
| { | ||
| drain(); | ||
| response.getWriter().write(string, offset, length); |
| if (length >= BUFFER_SIZE) | ||
| { | ||
| drain(); | ||
| response.getWriter().write(string, offset, length); |
| if (length >= BUFFER_SIZE) | ||
| { | ||
| drain(); | ||
| response.getWriter().write(string, offset, length); |
ServletExternalContextImpl.getResponseOutputWriter() caches a BufferedWriter. responseReset() and responseSendError() dropped that field, but a ResponseWriter created earlier in the render phase keeps writing into the instance it obtained back then, so everything rendered after the reset — typically an error page — landed in a buffer nobody drains and the client got HTTP 200 with an empty body. AjaxExceptionHandlerImpl and ErrorPageWriter follow the same reset-then-reuse pattern and were affected as well. release() additionally flushed the BufferedWriter, which flushed the container writer and thereby committed the response, so a render exception on a non-ajax request could no longer be reset and forwarded to the error page. Buffer through a ResettableBufferedWriter instead: it keeps its identity across a reset, discards the buffered chars there, resolves the container writer per drain, and drains without flushing the container writer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BalusC
force-pushed
the
fix-response-reset-with-buffered-writer
branch
from
July 29, 2026 14:38
55c9a57 to
36c0b21
Compare
Contributor
Author
Fixed now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ServletExternalContextImpl.getResponseOutputWriter()caches aBufferedWritersince 0e84278.responseReset()andresponseSendError()drop that field, but aResponseWritercreated earlier in the render phase keeps writing into the instance it obtained back then. Everything rendered after the reset — typically an error page — therefore lands in a buffer nobody drains, and the client gets HTTP 200 with an empty body.This is not limited to third party exception handlers:
AjaxExceptionHandlerImpl(line 205) andErrorPageWriter(line 427) follow the same reset-then-reuse-the-cached-writer pattern.There is a second break from the same commit:
release()flushes theBufferedWriter, which flushes the container writer and thereby commits the response. A render exception on a non-ajax request can then no longer be reset and forwarded to the error page.Fix
ResettableBufferedWriterreplaces the plainBufferedWriter:responseReset()/responseSendError(), which now discard the buffered chars instead of dropping the writer;HttpServletResponse.reset();flush()drains into the container writer but does not flush it — committing stays the container's decision, or that of an explicitresponseFlushBuffer().The buffering and its performance benefit are unchanged: same 8 KB buffer, same coalescing of the many small
HtmlResponseWriterImplwrites.Verified
ServletExternalContextImplTest#testResponseResetDiscardsBufferedOutputButKeepsWriterUsable. On currentmainit fails withexpected: <error page> but was: <aborted responseerror page>, which also shows aborted output being re-committed.mvn clean verifyon the reactor is green.FullAjaxExceptionHandlerITon Tomcat 11 + MyFaces 5.0.0-SNAPSHOT goes from 7 errors to 14/14 passing.🤖 Generated with Claude Code (Opus 5)