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

EE10 Servlet include after HttpServletResponse.getWriter().println() omits Content-Length from the response #10155

Closed
lorban opened this issue Jul 26, 2023 · 3 comments · Fixed by #10157
Labels
Bug For general bugs on Jetty side

Comments

@lorban
Copy link
Contributor

lorban commented Jul 26, 2023

Jetty version(s)
12

Description
Any EE10 servlet that does the following:

response.getWriter().println("Include:");
getServletContext().getRequestDispatcher("/test.txt").include(request, response);

will not add the Content-Length header to the response.

@lorban lorban added the Bug For general bugs on Jetty side label Jul 26, 2023
@gregw
Copy link
Contributor

gregw commented Jul 26, 2023

@lorban content length is not expected when include is used, instead chunking or EOF will be used.
However, we do need to write some code to covert writers to outputstreams and visa versa

@lorban
Copy link
Contributor Author

lorban commented Jul 26, 2023

We at least need to assert that the EE10 behavior is sane as using the OutputStream instead of the PrintWriter results in a content length header in the response, and EE9 adds content length no matter if you use the output stream or the writer.

gregw added a commit that referenced this issue Jul 26, 2023
Fix #10155 Include mixed writers and input streams
@gregw
Copy link
Contributor

gregw commented Jul 26, 2023

@lorban See #10157 for a potential fix with new tests

@gregw gregw linked a pull request Jul 26, 2023 that will close this issue
gregw added a commit that referenced this issue Jul 26, 2023
If the included servlet uses the writer, we must flush it and thus commit the response.  Previously we were sometimes able to avoid the commit, but that is too complex now and not required by the spec.
gregw added a commit that referenced this issue Jul 26, 2023
gregw added a commit that referenced this issue Jul 27, 2023
Avoid flushing writer if at all possible to avoid early commit
gregw added a commit that referenced this issue Jul 27, 2023
* Fix #10155 Include mixed writers and input streams

---------

Signed-off-by: Ludovic Orban <[email protected]>
Co-authored-by: Ludovic Orban <[email protected]>
@gregw gregw closed this as completed Jul 27, 2023
@gregw gregw moved this to ✅ Done in Jetty 12.0.1 - FROZEN Jul 27, 2023
@gregw gregw moved this to ✅ Done in Jetty 12.0.0 Jul 27, 2023
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
None yet
Development

Successfully merging a pull request may close this issue.

2 participants