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

Exception when a Content-Length is set on a 304 response #12481

Closed
drivron opened this issue Nov 5, 2024 · 5 comments · Fixed by #12484
Closed

Exception when a Content-Length is set on a 304 response #12481

drivron opened this issue Nov 5, 2024 · 5 comments · Fixed by #12484
Assignees
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@drivron
Copy link

drivron commented Nov 5, 2024

Jetty version(s)
12

Jetty Environment
ee8

Java version/vendor (use: java -version)
openjdk version "17.0.12" 2024-07-16

OS type/version
Ubuntu 24.04

Description
When a Content-Length is set on a 304 response, the following exception occurs :

java.io.IOException: written 0 < 454 content-length
        at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.write(HttpChannelState.java:1271)
        at org.eclipse.jetty.server.Response$Wrapper.write(Response.java:768)
        at org.eclipse.jetty.server.handler.ContextResponse.write(ContextResponse.java:56)
        at org.eclipse.jetty.ee8.nested.HttpChannel.send(HttpChannel.java:837)
        at org.eclipse.jetty.ee8.nested.HttpChannel.sendResponse(HttpChannel.java:822)
        at org.eclipse.jetty.ee8.nested.HttpChannel.write(HttpChannel.java:891)
        at org.eclipse.jetty.ee8.nested.HttpOutput.channelWrite(HttpOutput.java:286)
        at org.eclipse.jetty.ee8.nested.HttpOutput.complete(HttpOutput.java:466)
        at org.eclipse.jetty.ee8.nested.Response.completeOutput(Response.java:852)
        at org.eclipse.jetty.ee8.nested.HttpChannel.handle(HttpChannel.java:553)
        at org.eclipse.jetty.ee8.nested.ContextHandler$CoreContextHandler$CoreToNestedHandler.handle(ContextHandler.java:2376)
        at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1060)
        at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:151)
        at org.eclipse.jetty.server.Server.handle(Server.java:182)
        at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:662)
        at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:414)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
        at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:478)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:441)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.run(AdaptiveExecutionStrategy.java:201)
        at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:311)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:979)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1209)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1164)
        at java.base/java.lang.Thread.run(Thread.java:1583)

RFC7230 allows a Content-Length on 304 responses, see #4208.
The pull request that introduced this control: #10948.

@drivron drivron added the Bug For general bugs on Jetty side label Nov 5, 2024
@sbordet
Copy link
Contributor

sbordet commented Nov 5, 2024

@joakime
Copy link
Contributor

joakime commented Nov 5, 2024

@sbordet the error says that the Content-Length: 454 was set on the status code 304 response.

RFC9110 says ...

   A server MAY send a Content-Length header field in a 304 (Not
   Modified) response to a conditional GET request (Section 15.4.5); a
   server MUST NOT send Content-Length in such a response unless its
   field value equals the decimal number of octets that would have been
   sent in the content of a 200 (OK) response to the same request.

My interpretation is that if that same request, without the use of Last-Modified request header, would result in a 200 status with Content-Length: 454, then that response is 100% valid, without sending that content.

@joakime joakime added the Specification For all industry Specifications (IETF / Servlet / etc) label Nov 5, 2024
@sbordet
Copy link
Contributor

sbordet commented Nov 5, 2024

@joakime I don't understand your comment.
We cannot send a 200 response with Content-Length: 454 and then not sending the content like your last paragraph says.

The OP is right that we should allow for the Content-Length header to be set by the application for 304 responses.

@gregw gregw self-assigned this Nov 5, 2024
@gregw
Copy link
Contributor

gregw commented Nov 5, 2024

Our HttpGenerator explicitly allows for the content-length in a 304 response, but our write length sanity check does not.

@gregw
Copy link
Contributor

gregw commented Nov 6, 2024

I also note that we should be sending content headers with our own 304 responses: https://www.rfc-editor.org/rfc/rfc9110#status.304

gregw added a commit that referenced this issue Nov 6, 2024
Allow Content-Length header to be set with a 304 response
Follow RFC9110 recommendations for headers to be sent with 304 response.
@gregw gregw linked a pull request Nov 6, 2024 that will close this issue
gregw added a commit that referenced this issue Nov 6, 2024
Allow Content-Length header to be set with a 304 response
Follow RFC9110 recommendations for headers to be sent with 304 response.
gregw added a commit that referenced this issue Nov 8, 2024
Allow Content-Length header to be set with a 304 response
Follow RFC9110 recommendations for headers to be sent with 304 response.
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 Specification For all industry Specifications (IETF / Servlet / etc)
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants