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

didn't respect origin server Sec-WebSocket-Extension header #41

Open
lawrenceching opened this issue Dec 31, 2022 · 2 comments
Open

didn't respect origin server Sec-WebSocket-Extension header #41

lawrenceching opened this issue Dec 31, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@lawrenceching
Copy link

Questions

Header Sec-WebSocket-Extension is used to negotiate the list of supported extensions between WebSocket client and server.

In Websocket response 101 Switch Protocols, even origin server returns without permessage-deflate extension, vertx-http-server proxies websocket message to orign server permessage-deflated.

Undoubly, it will not be functional as orgin server doesn't not support that extension.

Version

Which version(s) did you encounter this bug ?
4.3.5

Context

Do you have a reproducer?

Steps to reproduce

  1. create a new proxy project with below options:
val proxyClient: HttpClient = vertx.createHttpClient()
val proxyOptions = ProxyOptions()
proxyOptions.supportWebSocket = true
val proxy = HttpProxy.reverseProxy(proxyOptions, proxyClient)
proxy.origin(30000, "localhost")

val options = HttpServerOptions()
options.addWebSocketSubProtocol("tty")

vertx
      .createHttpServer(options)
      .requestHandler(proxy)
      .listen(8888) { http ->
        if (http.succeeded()) {
          startPromise.complete()
          println("HTTP server started on port 8888")
        } else {
          startPromise.fail(http.cause());
        }
      }
  1. downlown and execute ttyd
    ttyd is an open source project to serve terminal in Web interface.
    Download from https://github.com/tsl0922/ttyd/releases and run with command ./ttyd -p 30000 -b /ttyd bash

  2. Open http://localhost:8888/ttyd/

Extra

As a workaround, I turn off message compression:

options.setPerMessageWebSocketCompressionSupported(false)
  • Anything that can be relevant such as OS version, JVM version
@lawrenceching lawrenceching added the bug Something isn't working label Dec 31, 2022
@vietj vietj self-assigned this Feb 17, 2023
@vietj
Copy link
Member

vietj commented Feb 17, 2023

I tried your reproducer and it worked for me.

What you are saying is weird, because the Vert.x HTTP proxy does not use Vert.x WebSocket, instead it simply creates a tunnel between the origin and the user agent after the WebSocket handshake.

The line options.addWebSocketSubProtocol("tty") is not event needed (in fact it will not have any effect since we are not using Vert.x WebSockets)

Can you clarify what you are observing ?

@lawrenceching
Copy link
Author

vertx-http-proxy_issue41.zip

Hi, I uploaded two tcpdump captured by WireShark, which contains two tcpdump files.

For normal_case.pcapng, I add options.setPerMessageWebSocketCompressionSupported(false) for HttpServerOptions
As you can see at the area I highlighted in red arrow, the message was decoded by WireShark in plain text.

It's the first Websocket message vert.x send to ttyd(:30000).

image

While in abnormal_case.pcapng, I removed the options.setPerMessageWebSocketCompressionSupported(false)
The message was unable to decode by WireShark. I guessed it's because the message was compressed.
image


If I looked at the handshark message in the abnormal case,
vert.x talk to ttyd and require the permessage-deflate extension
image

But ttyd didn't claim that it support permessage-deflate.
image

However, the subsequent message sent from vert.x are compressed, ttyd didn't understsand.
So ttyd printed out a warning log:

[2023/02/20 23:23:11:1947] W: ignored unknown message type: ?

I don't know much about WebSocket. Base on my HTTP knowleage, I understand that the permessage-deflate extension is kind of a hop-by-hop header. When ttyd does not claim it support permessage-deflate, vert.x should respect to negotiation.

Am I right? or get something wrong?

Yes, options.addWebSocketSubProtocol("tty") doesn't matter. Let's ignore this line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants