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

lawrenceching commented 1 year ago

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

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()); } }

2. 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`

4. 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

vietj commented 1 year ago

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 commented 1 year ago

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).

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.

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

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

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.

eclipse-vertx / vertx-http-proxy