search

jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

jetty-12 ee10 Closure of Response Object #9651

Open janbartel opened 1 year ago

janbartel commented 1 year ago

The Servlet Specification 6.0 says:

5.7. Closure of Response Object When a response is closed, the container must immediately flush all remaining content in the response buffer to the client. The following events indicate that the servlet has satisfied the request and that the response object is to be closed: • The termination of the service method of the servlet. • The amount of content specified in the setContentLength or setContentLengthLong method of the response has been greater than zero and has been written to the response. • The sendError method is called. • The sendRedirect method is called. • The complete method on AsyncContext is called.

The tck has a test which sets the contentLength then writes more bytes than that, and also tries to set a header:

    int size = 40;
    response.setContentLength(size);

    PrintWriter pw = response.getWriter();
    pw.println("Test PASSED");
    StringBuffer tmp = new StringBuffer(2 * size);
    int i = 0;

    while (i < 8) {
      tmp = tmp.append("111111111x");
      i = i + 1;
    }
    pw.println(tmp);
    response.addIntHeader("header1", 12345);

Tck test is com/sun/ts/tests/servlet/spec/httpservletresponse/URLClient.java.flushBufferTest

Jetty fails the tck test because we respond with an 500 error instead of ignoring the extra content:

<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 500 java.io.IOException: written 92 &gt; 40 content-length</title>
</head>
<body>
<h2>HTTP ERROR 500 java.io.IOException: written 92 &gt; 40 content-length</h2>
<table>
<tr><th>URI:</th><td>http://localhost:38225/servlet_spec_httpservletresponse_web/HttpTestServlet?testname=flushBufferTest</td></tr>
<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>java.io.IOException: written 92 &gt; 40 content-length</td></tr>
<tr><th>CAUSED BY:</th><td>java.io.IOException: written 92 &gt; 40 content-length</td></tr>
</table>
<h3>Caused by:</h3><pre>java.io.IOException: written 92 &gt; 40 content-length
    at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.write(HttpChannelState.java:1093)
    at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:215)
    at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:200)
    at org.eclipse.jetty.ee10.servlet.HttpOutput.write(HttpOutput.java:804)
    at java.base/java.io.ByteArrayOutputStream.writeTo(ByteArrayOutputStream.java:161)
    at org.eclipse.jetty.ee10.servlet.writer.Iso88591HttpWriter.write(Iso88591HttpWriter.java:62)
    at org.eclipse.jetty.ee10.servlet.writer.HttpWriter.write(HttpWriter.java:69)
    at org.eclipse.jetty.ee10.servlet.writer.ResponseWriter.println(ResponseWriter.java:426)
    at org.eclipse.jetty.ee10.servlet.writer.ResponseWriter.println(ResponseWriter.java:445)
    at com.sun.ts.tests.servlet.spec.httpservletresponse.HttpTestServlet.flushBufferTest(HttpTestServlet.java:59)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at com.sun.ts.tests.servlet.common.servlets.HttpTCKServlet.invokeTest(HttpTCKServlet.java:91)
    at com.sun.ts.tests.servlet.common.servlets.HttpTCKServlet.doGet(HttpTCKServlet.java:116)
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:527)
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
    at org.eclipse.jetty.ee10.servlet.ServletHolder$NotAsync.service(ServletHolder.java:1383)
    at org.eclipse.jetty.ee10.servlet.ServletHolder.handle(ServletHolder.java:738)
    at org.eclipse.jetty.ee10.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1601)
    at org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1534)
    at org.eclipse.jetty.ee10.servlet.ServletChannel.lambda$0(ServletChannel.java:425)
    at org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:659)
    at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:420)
    at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:458)
    at org.eclipse.jetty.ee10.servlet.security.SecurityHandler.handle(SecurityHandler.java:564)
    at org.eclipse.jetty.ee10.servlet.SessionHandler.handle(SessionHandler.java:664)
    at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:819)
    at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:149)
    at org.eclipse.jetty.server.Handler$Sequence.handle(Handler.java:665)
    at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:611)
    at org.eclipse.jetty.server.Server.handle(Server.java:175)
    at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:553)
    at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:480)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
    at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
    at java.base/java.lang.Thread.run(Thread.java:833)
</pre>
<hr/><a href="https://eclipse.org/jetty">Powered by Jetty:// 12.0.0-SNAPSHOT</a><hr/>

</body>
</html>
gregw commented 1 year ago

I think this test can be challenged. It is OK that we should close the response if exactly the right amount of data is written, but it is a bit strange to truncate data in excess of the set length. The string written could be multi-byte characters and the truncation could split a character in half.

I think it is reasonable that we throw an exception from such a write rather than truncate.

janbartel commented 4 months ago

Still waiting on some action from the servlet spec group, note that this also affects the 6.1 tck and jetty as documented here: https://github.com/jetty/jetty.project/issues/11931