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

Keep headers when Content-Encoding set #7930

Merged

Conversation

richdougherty
Copy link
Member

Fixes #7911.

The code now filters out the Content-Encoding header instead of all the other headers when decoding the request content.

@@ -13,7 +13,7 @@ private[server] object HttpRequestDecoder {

private def decodeRequestWith(decoderFlow: Flow[ByteString, ByteString, NotUsed], request: HttpRequest): HttpRequest = {
request.withEntity(request.entity.transformDataBytes(decoderFlow))
.withHeaders(request.headers.filter(_.isInstanceOf[`Content-Encoding`]))
.withHeaders(request.headers.filter(!_.isInstanceOf[`Content-Encoding`]))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the problem! The condition needs to negated, otherwise we strip out the wrong headers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filterNot?

Fixes playframework#7911.

The code now filters out the Content-Encoding header
instead of all the other headers when decoding the
request content.
@richdougherty
Copy link
Member Author

Updated - filterNot plus a few scaladocs. Should be ready now!

Copy link
Member

@gmethvin gmethvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks for adding the comments.

@gmethvin gmethvin merged commit a2350ce into playframework:master Oct 18, 2017
gmethvin pushed a commit that referenced this pull request Oct 18, 2017
Fixes #7911.

The code now filters out the Content-Encoding header
instead of all the other headers when decoding the
request content.
@marcospereira marcospereira added this to the Play 2.6.7 milestone Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants