andreacavalli
/
netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Improve `PerMessageDeflateClientExtensionHandler` (#11413) 

Motivation:

The `PerMessageDeflateClientExtensionHandler` has the following strange behaviors currently:

* The `requestedServerNoContext` parameter doesn't actually add the `server_no_context_takeover` parameter to the client offer; instead it depends on the requested server window size.
* The handshake will fail if the server responds with a `server_no_context_takeover` parameter and `requestedServerNoContext` is false. According to RFC 7692 (7.1.1.1) the server may do this, and this means that to cover both cases one needs to use two handshakers in the channel pipeline: one with `requestedServerNoContext = true` and one with `requestedServerNoContext = false`.
* The value of the `server_max_window_bits` parameter in the server response is never checked (should be between 8 and 15). And the value of `client_max_window_bits` is checked only in the branch handling the server window parameter.

Modification:

* Add the `server_no_context_takeover` parameter if `requestedServerNoContext` is true.
* Accept a server handshake response which includes the server no context takeover parameter even if we did not request it.
* Check the values of the client and server window size in their respective branches and fail the handshake if they are out of bounds.

Result:

There will be no need to use two handshakers in the pipeline to be lenient in what handshakes are accepted.
This commit is contained in:
Kasimir Torri 2021-07-02 13:47:59 +01:00 committed by GitHub
parent 842e73f8d3
commit c97981403d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 63 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshaker.java
View File

@ -117,7 +117,7 @@ public final class PerMessageDeflateClientExtensionHandshaker implements WebSock
@Override
public WebSocketExtensionData newRequestData() {
HashMap<String, String> parameters = new HashMap<String, String>(4);
if (requestedServerWindowSize != MAX_WINDOW_SIZE) {
if (requestedServerNoContext) {
parameters.put(SERVER_NO_CONTEXT, null);
}
if (allowClientNoContext) {
@ -153,13 +153,16 @@ public final class PerMessageDeflateClientExtensionHandshaker implements WebSock
// allowed client_window_size_bits
if (allowClientWindowSize) {
clientWindowSize = Integer.parseInt(parameter.getValue());
if (clientWindowSize > MAX_WINDOW_SIZE || clientWindowSize < MIN_WINDOW_SIZE) {
succeed = false;
}
} else {
succeed = false;
}
} else if (SERVER_MAX_WINDOW.equalsIgnoreCase(parameter.getKey())) {
// acknowledged server_window_size_bits
serverWindowSize = Integer.parseInt(parameter.getValue());
if (clientWindowSize > MAX_WINDOW_SIZE || clientWindowSize < MIN_WINDOW_SIZE) {
if (serverWindowSize > MAX_WINDOW_SIZE || serverWindowSize < MIN_WINDOW_SIZE) {
succeed = false;
}
} else if (CLIENT_NO_CONTEXT.equalsIgnoreCase(parameter.getKey())) {
@ -171,11 +174,7 @@ public final class PerMessageDeflateClientExtensionHandshaker implements WebSock
}
} else if (SERVER_NO_CONTEXT.equalsIgnoreCase(parameter.getKey())) {
// acknowledged server_no_context_takeover
if (requestedServerNoContext) {
serverNoContext = true;
} else {
succeed = false;
}
serverNoContext = true;
} else {
// unknown parameter
succeed = false;

57
codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshakerTest.java
View File

@ -130,6 +130,63 @@ public class PerMessageDeflateClientExtensionHandshakerTest {
assertNull(extension);
}
@Test
public void testParameterValidation() {
WebSocketClientExtension extension;
Map<String, String> parameters;
PerMessageDeflateClientExtensionHandshaker handshaker =
new PerMessageDeflateClientExtensionHandshaker(6, true, 15, true, false);
parameters = new HashMap<String, String>();
parameters.put(CLIENT_MAX_WINDOW, "15");
parameters.put(SERVER_MAX_WINDOW, "8");
extension = handshaker.handshakeExtension(new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, parameters));
// Test that handshake succeeds when parameters are valid
assertNotNull(extension);
assertEquals(RSV1, extension.rsv());
assertTrue(extension.newExtensionDecoder() instanceof PerMessageDeflateDecoder);
assertTrue(extension.newExtensionEncoder() instanceof PerMessageDeflateEncoder);
parameters = new HashMap<String, String>();
parameters.put(CLIENT_MAX_WINDOW, "15");
parameters.put(SERVER_MAX_WINDOW, "7");
extension = handshaker.handshakeExtension(new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, parameters));
// Test that handshake fails when parameters are invalid
assertNull(extension);
}
@Test
public void testServerNoContextTakeover() {
WebSocketClientExtension extension;
Map<String, String> parameters;
PerMessageDeflateClientExtensionHandshaker handshaker =
new PerMessageDeflateClientExtensionHandshaker(6, true, 15, true, false);
parameters = new HashMap<String, String>();
parameters.put(SERVER_NO_CONTEXT, null);
extension = handshaker.handshakeExtension(new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, parameters));
// Test that handshake succeeds when server responds with `server_no_context_takeover` that we didn't offer
assertNotNull(extension);
assertEquals(RSV1, extension.rsv());
assertTrue(extension.newExtensionDecoder() instanceof PerMessageDeflateDecoder);
assertTrue(extension.newExtensionEncoder() instanceof PerMessageDeflateEncoder);
// initialize
handshaker = new PerMessageDeflateClientExtensionHandshaker(6, true, 15, true, true);
parameters = new HashMap<String, String>();
extension = handshaker.handshakeExtension(new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, parameters));
// Test that handshake fails when client offers `server_no_context_takeover` but server doesn't support it
assertNull(extension);
}
@Test
public void testDecoderNoClientContext() {
PerMessageDeflateClientExtensionHandshaker handshaker =