JSP not rendered on Jetty

[arey]

I'm contributing to the Spring Petclinic sample: https://github.com/spring-projects/spring-petclinic
The web application is working fine on Tomcat 7 and 8. But on Jetty 9, JSP pages are not rendered.
The application uses Spring MVC 4.2.7 and Dandelion 1.2.1.

A org.eclipse.jetty.io.EofException is thrown in the DandelionFilter because the HttpOuput is closed:

This problem appears to be similar to issue #55.
I've tested the Dandelion 1.2.2-SNAPSHOT version. But nothing changes.
This problem can be tested with the Spring Petclinic project. All pages fall in error.
If you remove the DandelionFilter from PetclinicInitializer, pages are rendered.

[gregw]

Hi, just a note to say that the Jetty team are also looking a dandelion and are happy to help resolve these issues, either by updating jetty and/or improving the Dandelion Filter.

Our inspection of the Dandelion filter indicate that it is wrapping the response and overriding the getWriter method to intercept output. This approach is extremely fragile and really depend entirely on how the content is generated within the chain.doFilter call.

I think the wrapper needs to be extended to override more methods that can cause a commit. You could look at implementations of GzipFilters that are available, as these have dealt with similar problems. At the very least you should probably override:

• getOutputStream at least to throw a unsupported exception if not to buffer
• flushBuffers to prevent a commit of an empty response
• sendError to prevent a commit of some other response
• setContentLength if you are going to change the content size in any way and avoid autoflushing

Also note that in general, such response wrappers are getting harder and harder to implement correctly - specially with asynchronous IO. IT may be better to modularize the output transformations that you want to do and them provide that in various adaptors:

• A filter as you currently have for non-blocking applications - would throw a nice exception for any attempt to set a read/write listener
• A Jetty handler which can use our interceptor mechanism to use a single API to deal with transforming async, outputStream and writers
  • Some kind of tomcat valve that would work for their async mechanism

[joakime]

@gregw I just filed those bugs a few moments ago at #114 and #115

[gregw]

I think @janbartel and I have worked out a bit more of this issue.

The wrapper only wraps getWriter, and the request is forwarded by spring to the JSP which calls it and fills up the ByteArrayOutputStream with content. But importantly neither getOutputStream nor getWriter have been called on the jetty response.

The dispatch forward to the JSP returns and Jetty has to commit the response (by the servlet spec), but it has seen neither a getWriter nor a getOutputStream, so it just picks one (in this case the getOutputStream) and closes it. So the close flushes the response and all the content that was written to the writer is lost.

We have a fix for jetty that helps it make a better choice of what to close and that will be in a release soon (just missed 9.3.11). But I can also see a way for you to improve your filter so that it does not suffer this problem.

You need to override getOutputStream so that when it is called it get's the jetty outputstream and first writes all the buffered content to it before return the stream. This should then work with the current jetty releases. Do you want us to make a pull request?

[gregw]

An alternate solution, that is probably more in line with the contract of Response, is that the wrapped getOutputStream should throw an ISE if getWriter has been called. Jetty already has code to try one and then the other if ISE is correctly thrown. The problem is that Jetty is trying to close the OutputStream first and this succeeds even though it should not, so we don't then close the writer.

[gregw]

We have looked again at the "fix" in jetty code and also looked at tomcat. It was just that tomcat tries to close the writer first and if the ISE, they close the output stream. Jetty does it the otherway around. So our fix is not really a fix it is just an arbitrary change to make up for a problem in your filter.

You need to override both getOutputStream and getWriter and correctly throw an ISE. See our PR #116 for a proposed implementation of this, which also respects any character encoding set.

If this looks OK, and ETA of when it could make it into a release would be good.