{"action":"created","issue":{"url":"https://api.github.com/repos/playframework/playframework/issues/6261","repository_url":"https://api.github.com/repos/playframework/playframework","labels_url":"https://api.github.com/repos/playframework/playframework/issues/6261/labels{/name}","comments_url":"https://api.github.com/repos/playframework/playframework/issues/6261/comments","events_url":"https://api.github.com/repos/playframework/playframework/issues/6261/events","html_url":"https://github.com/playframework/playframework/pull/6261","id":161710663,"number":6261,"title":"Don't allow users to set a Content-Length header field in a 204 response","user":{"login":"monkey-mas","id":5261677,"avatar_url":"https://avatars.githubusercontent.com/u/5261677?v=3","gravatar_id":"","url":"https://api.github.com/users/monkey-mas","html_url":"https://github.com/monkey-mas","followers_url":"https://api.github.com/users/monkey-mas/followers","following_url":"https://api.github.com/users/monkey-mas/following{/other_user}","gists_url":"https://api.github.com/users/monkey-mas/gists{/gist_id}","starred_url":"https://api.github.com/users/monkey-mas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/monkey-mas/subscriptions","organizations_url":"https://api.github.com/users/monkey-mas/orgs","repos_url":"https://api.github.com/users/monkey-mas/repos","events_url":"https://api.github.com/users/monkey-mas/events{/privacy}","received_events_url":"https://api.github.com/users/monkey-mas/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":16,"created_at":"2016-06-22T15:17:08Z","updated_at":"2016-07-27T14:10:39Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/playframework/playframework/pulls/6261","html_url":"https://github.com/playframework/playframework/pull/6261","diff_url":"https://github.com/playframework/playframework/pull/6261.diff","patch_url":"https://github.com/playframework/playframework/pull/6261.patch"},"body":"# Pull Request Checklist\r\n\r\n* [x] Have you read [How to write the perfect pull request](https://github.com/blog/1943-how-to-write-the-perfect-pull-request)?\r\n* [x] Have you read through the [contributor guidelines](https://www.playframework.com/contributing)?\r\n* [x] Have you signed the [Lightbend CLA](https://www.lightbend.com/contribute/cla)?\r\n* [ ] Have you [squashed your commits](https://www.playframework.com/documentation/2.4.x/WorkingWithGit#Squashing-commits)?\r\n* [x] Have you added copyright headers to new files?\r\n* [x] Have you checked that both Scala and Java APIs are updated?\r\n* [x] Have you updated the documentation for both Scala and Java sections?\r\n* [x] Have you added tests for any changed functionality?\r\n\r\n## Fixes\r\n\r\nThis fix makes sure that Play never sends Content-Length header field for a 204 response.\r\n\r\n## Purpose\r\n\r\nAs described in [RFC7230 section-3.3.2](https://tools.ietf.org/html/rfc7230#section-3.3.2), sending a Content-Length header field is not allowed in any response with a status code of 1xx (Informational) or 204 (No Content). This requirement was not forced in Play, which means a user can intentionally set a Content-Length header. To follow the RFC section, we need to make sure that we don't send the header field.\r\nWith regard to a Content-Length header field in a 304 response, a server May send it as mentioned in RFC7230 section-3.3.2. Thus it'd be better for us to allow users to set the field if they like.\r\n\r\n## Background Context\r\n\r\nAs I've mentioned in Purpose.\r\n\r\n## References\r\n\r\n- Related Issues/PRs\r\n  - [content-length header included in 304 response which is forbidden by the HTTP spec](https://github.com/playframework/playframework/issues/3899)\r\n  - [Don't include body and Content-Length in 204 and 304 responses. Fixes #3899](https://github.com/playframework/playframework/pull/3939)\r\n\r\n- The following is the partial citation from RFC7230 section-3.3.2.\r\n```\r\nA server MUST NOT send a Content-Length header field in any response with\r\na status code of 1xx (Informational) or 204 (No Content). A server MUST NOT send\r\na Content-Length header field in any 2xx(Successful) response to a CONNECT request\r\n(Section 4.3.6 of [RFC7231]).\r\n```\r\n```\r\nA server MAY send a Content-Length header field in a 304 (Not Modified) response to\r\na conditional GET request (Section 4.1 of [RFC7232]); a server MUST NOT send\r\nContent-Length in such a response unless its field-value equals the decimal number of\r\noctets that would have been sent in the payload body of a 200 (OK) response to\r\nthe same request.\r\n```\r\n\r\nP.S.\r\n`play.it.http.NettyScalaResultsHandlingSpec` appends a `Content-Length: 0` header field\r\nwhen we explicitly send it, but `AkkaHttpIntegrationSpecification` does not.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/playframework/playframework/issues/comments/235596925","html_url":"https://github.com/playframework/playframework/pull/6261#issuecomment-235596925","issue_url":"https://api.github.com/repos/playframework/playframework/issues/6261","id":235596925,"user":{"login":"monkey-mas","id":5261677,"avatar_url":"https://avatars.githubusercontent.com/u/5261677?v=3","gravatar_id":"","url":"https://api.github.com/users/monkey-mas","html_url":"https://github.com/monkey-mas","followers_url":"https://api.github.com/users/monkey-mas/followers","following_url":"https://api.github.com/users/monkey-mas/following{/other_user}","gists_url":"https://api.github.com/users/monkey-mas/gists{/gist_id}","starred_url":"https://api.github.com/users/monkey-mas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/monkey-mas/subscriptions","organizations_url":"https://api.github.com/users/monkey-mas/orgs","repos_url":"https://api.github.com/users/monkey-mas/repos","events_url":"https://api.github.com/users/monkey-mas/events{/privacy}","received_events_url":"https://api.github.com/users/monkey-mas/received_events","type":"User","site_admin":false},"created_at":"2016-07-27T14:10:39Z","updated_at":"2016-07-27T14:10:39Z","body":"@richdougherty  cc: @gmethvin  \r\nThanks for taking a look! Removed the duplicate function definitions and added test cases for 1xx responses (also modified a test for a 304 response) :) Can you please review again?\r\n\r\nP.S.\r\nI found that `BasicHttpClient` just returns an empty body[1] instead of sending an actual message-body by assuming that a 204 response doesn't send a body for example. (I think there's no tests to check if the returned body is acutally empty) I'll create a new PR and add some tests for `ServerResultUtils` to check if a message-body is canceld[2] so that we can confirm that an empty entity is returned for 1xx, 204 and 304. What do you think? :)\r\n\r\n[1] https://github.com/playframework/playframework/blob/2.5.4/framework/src/play-integration-test/src/test/scala/play/it/http/BasicHttpClient.scala#L214-L218\r\n[2] https://github.com/playframework/playframework/blob/2.5.4/framework/src/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala#L67-L68"}}
{"actor":{"display_login":"monkey-mas"}}