Websockets serving cached/wrong responses and potentially have incorrect headers in websocket

[agapic]

Version

Vertx core 3.6.3 AND 3.7.0
NGINX 1.19.9

Context

Vertx 3.6.3

We use NGINX 1.19.9 to proxy requests to our services in Kubernetes. One such service is a client manager that creates web sockets. For paths where the user in unauthorized, we call client.reject(403).

Let's say client 1 connects to the service using wss://myuri.com/api and gets a valid 403. Great.
Now when client 2 connects to the service, the request handler appears to be re-using the request headers from client 1. The same is true for client 3, 4, 5, etc. Now all of our clients are in a bad state.

We've determined that it seems to be an interplay between nginx and vertx/netty. When we change upstream-keepalive-timeout to 0, the issue goes away. Only client 1 gets the 403. When we get rid of nginx entirely, the issue is also gone.

https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#upstream-keepalive-timeout

Do you have a reproducer?

Vertx 3.7.0

Reproducer code: https://github.com/NguyenVoDev/pure-vertx-js/blob/main/src/main/java/org/example/Server.java

Note this won't work if you try and repro locally, only if using nginx with keepalive set to > 0 (we have 60s). It could actually just be the headers nginx is adding to the request though

Instead of clients receiving headers from the existing web socket connection (like in 3.6.3), now the connections just hang and the websocket handler isn't even called.

Extra

Are we supposed to be closing websockets after rejecting them?

• Java 11, Adopt openJDK

[agapic]

@vietj any ideas what might cause this? This is causing a super critical bug for our product (911 software), and upgrading Vertx with our current configuration introduces other issues related to connectivity (but that’s an aside)

[agapic]

Added reproducer code for Vertx 3.7.0

[vietj]

can you upgrade to 3.9 ? that's the only version of Vert.x 3 we are currently supporting

[agapic]

@vietj I just tried with Vertx 4.2.3 - same issue as 3.7.
We don't appear the get the issue with NodeJS websocket server "ws" library.
One theory around that is the nodejs code explicitly sends the Connection: close header which might force nginx or the client to close the connection in its upstream cache.

[agapic]

I think fundamentally the root cause here is that Vertx doesn't expect different clients to re-use connections. The interplay between Nginx caching connections and Vertx causes issues. When a socket is rejected and a new client connects, the response from the previous client is left hanging because it never ended. Vertx tries to re-use the existing connection and incoming requests "hang" because the previous request is not finished (but really.. it is.. it's been rejected)

For 3.6.3, I think it's because Vertx reuses the websocket in the Http1xServerConnection class (line 302), and it reuses the headers too unfortunately, even if the incoming request has different headers.

However, in Vertx 3.9.12 AND 4.2.3l, instead of reusing the websocket, the connection hangs. :(

I think it hangs because in the Http1xServerConnection class in 3.9.12, it thinks there is a response in progress and pauses the request probably until the old conneciton is killed. My theory is that Vertx doesn't handle connection reuse correctly, which is what Nginx does with upstream caching. Our proposed solution is to close websockets after calling reject.

      synchronized (this) {
        requestInProgress = req;
        if (responseInProgress != null) {
          // Deferred until the current response completion
          req.pause();
          responseInProgress.enqueue(req);
          return;
        }
        responseInProgress = requestInProgress;
        req.handleBegin();
      }

[vietj]

I'll have a look at the reproducer, thanks