Review content-length check in ServletChannel

[sbordet]

Jetty version(s)
12+

Jetty Environment
ee10, possibly others too

Description
ServletChannel.handle() in case COMPLETE performs a check on the content-length, to see if it the actual bytes written match the value of the header.

Unfortunately, this check is performed too early in case of buffered content if looking at the bytes written to the ServletContextResponse (they are written to the HttpOutput, but not yet to the ServletContextResponse).

However, we cannot look at the bytes written to HttpOutput, because currently the welcome files are written bypassing it, so content-length the check would fail.
This could be solved by always wrapping the Servlet response in DefaultServlet, so it never bypasses HttpOutput so the count is always correct.

Lastly, the solution needs to take into account the possibility of gzip being performed before or after the ContextHandler.

[sbordet]

@gregw is this check necessary? I vaguely remember it is required by the Servlet spec?

We either need a way to count the bytes precisely before the check is performed, or we need to do the check after the aggregation buffer is flushed. Thoughts?

[gregw]

The servlet spec requires that we commit the response as soon as precisely the number of bytes written matches any content-length set.
So we do need to track bytes written, but only for responses that written via the response API.

I don't recall that we need to actively police too few bytes written and it should be impossible to write too many. Both of these will be caught by core.

[gregw]

@sbordet Is this an actual bug/issue? Can you provide and example of what you mean by "too early"

The requirement to commit as soon as a precise number of bytes have been written is only from the servlet spec and only relates to code that does response.setContentLength and then writes to the outputstream or writer. The DefaultServlet is free to write its content as it wants, and bypassing HttpOutput will result in the commit after bytes written behaviour as expected.

Note also that lots of thinks might change the actual content size (eg GzipHandler). From the servlet spec point of view, these do not exist. This if the app as said the content length is 1000, we need to allow it to write 1000 bytes and then close the response for it, regardless of how many bytes are actually in the real response. So there are two kinds of checks: one for the servlet API, which needs to be done high in the stack (around HttpOutput) to trigger closes; another low in the stack to ensure we are sending valid HTTP responses.

[gregw]

Currently we are counting bytes written in 3 places:

1. HttpOutput - for all EEx we count uncompressed bytes written by the servlet API so that org.eclipse.jetty.ee10.servlet.ServletContextResponse#isAllContentWritten can be called for each write
2. ServletContextResponse - for EE10 and later so that org.eclipse.jetty.ee10.servlet.ServletChannel#handle can check if sufficient bytes have been written.
3. HttpChannelState.ChannelResponse - to count bytes actually written so that a Content-Length header might be generated and for logging. This value is also checked against the commitedContentLength in the ChannelCallback

We do need to rationalize this and count only before/after compression