Skip to content

Conversation

@TheAntimist
Copy link
Contributor

Had to create a new PR, because the commits got detached on the previous fork.

Related to: playframework/playframework#11169

Purpose

Adding tests for with and without compression enabled. Hopefully it covers all possible scenarios regarding compression.

The actual implementation unfortunately could not be placed in this repository due to the chain of Netty's codecs where WebSocketServerCompressionHandler needs to be added is right after the HTTP Codecs.

This commit adds test parameters to be able to enable and disable
compression along with WebSocketExtension settings for the handshaker.
assertTrue(headers.contains("sec-websocket-extensions"));
assertEquals(headers.get("sec-websocket-extensions"), "permessage-deflate");
} else {
assertTrue(!headers.contains("sec-websocket-extensions") ||
Copy link
Member

@mkurz mkurz Feb 22, 2022

Choose a reason for hiding this comment

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

Shouldn't we use and (&&) instead of or (||)?

Copy link
Contributor Author

@TheAntimist TheAntimist Feb 22, 2022

Choose a reason for hiding this comment

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

It's more future proofing, intuitively, either the server does not have extensions enabled or it does have extensions but compression is not.

EDIT: Because of this it is possible that it does have extensions enabled (with no compression) in that case the first condition would fail and second would succeed. That is why it is || so either of the conditions hold true.

@mkurz mkurz merged commit 1879c8e into playframework:main Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants