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

WebSocket compression extension should not depends on SimpleCompressionHandler #970

Closed
amizurov opened this issue Jan 24, 2020 · 3 comments
Labels
type/bug A general bug
Milestone

Comments

@amizurov
Copy link

Expected Behavior

If compression is enabled on the server side then WebSocket compression extensions should be applied according to https://tools.ietf.org/html/rfc7692#section-5 and not depend on SimpleCompressionHandler present in pipeline or not.

Actual Behavior

Configure HttpServer with enable compression and set maxResponseSize to value grate than zero, WebSocket compression never applied.

Steps to Reproduce

public class EchoServer {

    public static void main(String[] args) {
        HttpServer server =
            HttpServer.create()
                .port(8080)
                .compress(true)
                .compress(2048)
                .wiretap(true)
                .route(r -> r.ws("/ws", (in, out) ->  out.sendObject(in.receiveFrames().doOnNext(WebSocketFrame::retain))));

        server.bindNow()
            .onDispose()
            .block();
    }
}

Possible Solution

Change logic inside WebsocketServerOperations constructor.

Your Environment

  • Reactor version(s) used: 1.0.0.BUILD-SNAPSHOT
  • Other relevant libraries versions (eg. netty ): 4.1.45.Final
  • JVM version (javar -version): 1.8.0_232
  • OS and version (eg uname -a): Darwin Kernel Version 18.7.0: Sun Dec 1 18:59:03 PST 2019; root:xnu-4903.278.19~1/RELEASE_X86_64 x86_64
@amizurov amizurov added status/need-triage A new issue that still need to be evaluated as a whole type/bug A general bug labels Jan 24, 2020
@violetagg violetagg removed the status/need-triage A new issue that still need to be evaluated as a whole label Jan 24, 2020
@violetagg violetagg added this to the 0.8.16.RELEASE milestone Jan 24, 2020
violetagg added a commit that referenced this issue Jan 24, 2020
@violetagg
Copy link
Member

@amizurov PR #971

@violetagg
Copy link
Member

We will provide a WebSocket configuration for compression on/off

@violetagg
Copy link
Member

violetagg commented Jan 30, 2020

@amizurov @rstoyanchev PTAL PR #980

In 0.9.x versions we will keep both ways for specifying compression on/off for WebSocket (on HTTP level and the new way with the WebSocket configuration).
In 1.x I'm going to remove the one on HTTP level.

violetagg added a commit that referenced this issue Jan 30, 2020
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
Development

No branches or pull requests

2 participants