Don't include body and Content-Length in 204 and 304 responses. Fixes #3899 #3939

Merged
merged 1 commit into from Feb 17, 2015

Conversation

Projects
None yet
2 participants
@dotta
Contributor

dotta commented Feb 16, 2015

As described in http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4,
both 204 and 304 responses must not include a message-body. This requirement
was not enforced in Play, but the built-in instances for both 204 and 304
responses (see play.api.mvc.Results.NoContent and
play.api.mvc.Results.NotModified) are constructed with an empty body. Hence,
the above mentioned requirement was already respected (though not enforced)
when using these values.

This commit enforces the requirement, returning an empty body for both 204 and
304 responses (whether a message body is provided or not).

Ticket #3899 requested the Content-Length header to never be included in a
304 result. The specification for 304 (see
http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.5) only states
that no message body should be returned, but it does not say that a
Content-Length must not be passed.

Hence, the solution adopted in this commit is to be lenient with the
Content-Length for both 204 and 304 responses. Meaning that we won't
automatically inject a Content-Length header for such responses, but we also
won't disallow users to provide it.

Don't include body and Content-Length in 204 and 304 responses. Fixes #…
…3899

As described in http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4,
both 204 and 304 responses must not include a message-body. This requirement
was not enforced in Play, but the built-in instances for both 204 and 304
responses (see `play.api.mvc.Results.NoContent` and
`play.api.mvc.Results.NotModified`) are constructed with an empty body. Hence,
the above mentioned requirement was already respected (though not enforced)
when using these values.

This commit enforces the requirement, returning an empty body for both 204 and
304 responses (whether a message body is provided or not).

Ticket #3899 requested the `Content-Length` header to never be included in a
304 result. The specification for 304 (see
http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.5) only states
that no message body should be returned, but it does not say that a
`Content-Length` must not be passed.

Hence, the solution adopted in this commit is to be lenient with the
`Content-Length` for both 204 and 304 responses. Meaning that we won't
automatically inject a `Content-Length` header for such responses, but we also
won't disallow users to provide it.
+ val nettyResponse = createNettyResponse(result.header, closeConnection, httpVersion)
+ val future = sendDownstream(startSequence, !closeConnection, nettyResponse).toScala
+ val channelStatus = new ChannelStatus(closeConnection, startSequence)
+ future.map(_ => channelStatus).recover { case _ => channelStatus }

This comment has been minimized.

@dotta

dotta Feb 16, 2015

Contributor

There is definitely an opportunity for extracting a method (and avoid code duplication). The new method will be used by both StreamWithNoBody and StreamWithStrictBody. (If you have a suggestion for what the method's name should be, speak up!)

@dotta

dotta Feb 16, 2015

Contributor

There is definitely an opportunity for extracting a method (and avoid code duplication). The new method will be used by both StreamWithNoBody and StreamWithStrictBody. (If you have a suggestion for what the method's name should be, speak up!)

jroper added a commit that referenced this pull request Feb 17, 2015

Merge pull request #3939 from dotta/content-length-header-in-304-resp…
…onse-3899

Don't include body and Content-Length in 204 and 304 responses.  Fixes #3899

@jroper jroper merged commit b264a77 into playframework:master Feb 17, 2015

2 checks passed

jenkins-pr-validator Merged build finished.
Details
typesafe-cla-validator All users have signed the CLA
Details

@dotta dotta deleted the dotta:content-length-header-in-304-response-3899 branch Feb 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment