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

Unable to configure Websocket compression when both HttpProtocol.H2C and HttpProtocol.HTTP1.1 are configured #3036

Closed
joebyneil opened this issue Jan 23, 2024 · 2 comments · Fixed by #3037
Assignees
Labels
type/bug A general bug
Milestone

Comments

@joebyneil
Copy link

joebyneil commented Jan 23, 2024

Expected Behavior

Websocket compression should be negotiated when both HTTP11 and H2C protocols are enabled, in the same way as when only HTTP11 protocol is enabled, and both the server and client support it.

Both the request and response contain a sec-websocket-extensions header indicating that compression is in use. (ie. permessage-deflate)

Actual Behavior

Websocket compression is not negotiated when HTTP11 and H2C are both enabled. The HttpClient correctly sends the extension, but the HttpServer does not apply it. The response does not contain the extensions header, and debug logs show that the compression handler was removed.

Using only HTTP11 or using H2 instead of H2C works as expected.

Steps to Reproduce

@Test
public void testCompression() {
    var websocketServerSpec = WebsocketServerSpec.builder().compress(true).build();
    var compressionExpected = HttpServer.create()
        .protocol(HttpProtocol.HTTP11, HttpProtocol.H2C)
        .handle((req, res) ->
            res.sendWebsocket((in, out) -> out.sendString(Mono.just("test")), websocketServerSpec))
        .bindNow();

    var responseHeaders = new HashMap<String, String>();
    var webSocketClientSpec = WebsocketClientSpec.builder().compress(true).build();
    var connectionClient = HttpClient.create()
        .remoteAddress(compressionExpected::address)
        .websocket(webSocketClientSpec)
        .handle((in, out) -> {
            var headers = in.headers();
            headers.forEach(header -> responseHeaders.put(header.getKey(), header.getValue()));
            return out.sendClose();
        }).then().block();

    assertThat(responseHeaders.keySet()).contains("sec-websocket-extensions");
    assertThat(responseHeaders.get("sec-websocket-extensions")).contains("permessage-deflate");
}

Your Environment

  • Reactor version(s) used:
    reactor-netty 1.1.12
  • JVM version (java -version):
    openjdk 17.0.6 2023-01-17 LTS
    OpenJDK Runtime Environment Zulu17.40+19-CA (build 17.0.6+10-LTS)
    OpenJDK 64-Bit Server VM Zulu17.40+19-CA (build 17.0.6+10-LTS, mixed mode, sharing)
  • OS and version (eg. uname -a):
    XXX.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64
@joebyneil joebyneil added status/need-triage A new issue that still need to be evaluated as a whole type/bug A general bug labels Jan 23, 2024
@violetagg violetagg removed the status/need-triage A new issue that still need to be evaluated as a whole label Jan 23, 2024
@violetagg violetagg self-assigned this Jan 23, 2024
@violetagg violetagg added this to the 1.0.42 milestone Jan 23, 2024
violetagg added a commit that referenced this issue Jan 23, 2024
…h HttpProtocol.H2C and HttpProtocol.HTTP1.1

Websocket compression handler has to be located after the HTTP codec.

Fixes #3036
@violetagg
Copy link
Member

@joebyneil Thanks for the reproducible example! PR #3037 should fix the issue.

@joebyneil
Copy link
Author

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
2 participants