Repository: vert-x3/vertx-web
Issue: SockJS websocket connections initially fail when combined with clustered infinispan-backed sessions

### Version

4.0.0.Beta3 - 4.0.0

Version 3.X.Y, 4.0.0.Beta1 and 4.0.0.Beta2 are not effected.

### Context

Since vert.x stack version 4.0.0.Beta3 SockJS websocket connections are not established
for load-balanced verticles until the same verticle processes the initial sockjs "/info" request and the websocket upgrade.
After the first successful websocket upgrade the problem vanishes.
This only occurs in combination with clustered sessions handled by an infinispan cluster manager and forced websocket connections.

I have not found the root cause for this problem.

### Do you have a reproducer?

Yes, it also includes a more detailed analysis of the problem:
[vertx-4-infinispan-ws-sockjs-reproducer](https://github.com/trho/vertx-4-infinispan-ws-sockjs-reproducer)

### Steps to reproduce

See [vertx-4-infinispan-ws-sockjs-reproducer](https://github.com/trho/vertx-4-infinispan-ws-sockjs-reproducer)

### Extra

OS version, JVM version:
MacOS 10.15.7, AdoptOpenJDK 11 (build 11.0.9.1+1)


Changed files:
vertx-web/src/test/java/io/vertx/ext/web/sstore/ClusteredSessionHandlerTest.java
vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java
vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java

diff --git a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java
index f3be81d40..f42f2013b 100644
--- a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java
+++ b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java
@@ -72,17 +72,15 @@ public abstract class AuthenticationHandlerImpl<T extends AuthenticationProvider
     // pause parsing the request body. The reason is that
     // we don't want to loose the body or protocol upgrades
     // for async operations
-    final boolean parseEnded = ctx.request().isEnded();
+    HttpServerRequest request = ctx.request();
+    final boolean parseEnded = request.isEnded();
     if (!parseEnded) {
-      ctx.request().pause();
+      request.pause();
     }
     // parse the request in order to extract the credentials object
     parseCredentials(ctx, res -> {
       if (res.failed()) {
-        // resume as the error handler may allow this request to become valid again
-        if (!parseEnded) {
-          ctx.request().resume();
-        }
+        resume(request, parseEnded);
         processException(ctx, res.cause());
         return;
       }
@@ -98,9 +96,7 @@ public abstract class AuthenticationHandlerImpl<T extends AuthenticationProvider
             session.regenerateId();
           }
           // proceed with the router
-          if (!parseEnded) {
-            ctx.request().resume();
-          }
+          resume(request, parseEnded);
           postAuthentication(ctx);
         } else {
           String header = authenticateHeader(ctx);
@@ -109,24 +105,21 @@ public abstract class AuthenticationHandlerImpl<T extends AuthenticationProvider
               .putHeader("WWW-Authenticate", header);
           }
           // to allow further processing if needed
-          if (authN.cause() instanceof HttpStatusException) {
-            // resume as the error handler may allow this request to become valid again
-            if (!parseEnded) {
-              ctx.request().resume();
-            }
-            processException(ctx, authN.cause());
-          } else {
-            // resume as the error handler may allow this request to become valid again
-            if (!parseEnded) {
-              ctx.request().resume();
-            }
-            processException(ctx, new HttpStatusException(401, authN.cause()));
-          }
+          resume(request, parseEnded);
+          Throwable cause = authN.cause();
+          processException(ctx, cause instanceof HttpStatusException?cause:new HttpStatusException(401, cause));
         }
       });
     });
   }
 
+  private void resume(HttpServerRequest request, boolean parseEnded) {
+    // resume as the error handler may allow this request to become valid again
+    if (!parseEnded && !request.headers().contains(HttpHeaders.UPGRADE, HttpHeaders.WEBSOCKET, true)) {
+      request.resume();
+    }
+  }
+
   /**
    * This method is protected so custom auth handlers can override the default
    * error handling
diff --git a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java
index f6a453601..4f0736c0e 100644
--- a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java
+++ b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java
@@ -22,6 +22,8 @@ import io.vertx.core.Handler;
 import io.vertx.core.Vertx;
 import io.vertx.core.http.Cookie;
 import io.vertx.core.http.CookieSameSite;
+import io.vertx.core.http.HttpHeaders;
+import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.impl.logging.Logger;
 import io.vertx.core.impl.logging.LoggerFactory;
 import io.vertx.ext.auth.AuthProvider;
@@ -269,8 +271,9 @@ public class SessionHandlerImpl implements SessionHandler {
 
   @Override
   public void handle(RoutingContext context) {
+    HttpServerRequest request = context.request();
     if (nagHttps && log.isDebugEnabled()) {
-      String uri = context.request().absoluteURI();
+      String uri = request.absoluteURI();
       if (!uri.startsWith("https:")) {
         log.debug(
           "Using session cookies without https could make you susceptible to session hijacking: " + uri);
@@ -280,8 +283,19 @@ public class SessionHandlerImpl implements SessionHandler {
     // Look for existing session id
     String sessionID = getSessionId(context);
     if (sessionID != null && sessionID.length() > minLength) {
+      // before starting any potential async operation here
+      // pause parsing the request body. The reason is that
+      // we don't want to loose the body or protocol upgrades
+      // for async operations
+      final boolean parseEnded = request.isEnded();
+      if (!parseEnded) {
+        request.pause();
+      }
       // we passed the OWASP min length requirements
       getSession(context.vertx(), sessionID, res -> {
+        if (!parseEnded && !request.headers().contains(HttpHeaders.UPGRADE, HttpHeaders.WEBSOCKET, true)) {
+          request.resume();
+        }
         if (res.succeeded()) {
           Session session = res.result();
           if (session != null) {
diff --git a/vertx-web/src/test/java/io/vertx/ext/web/sstore/ClusteredSessionHandlerTest.java b/vertx-web/src/test/java/io/vertx/ext/web/sstore/ClusteredSessionHandlerTest.java
index 400414af8..2e430f731 100644
--- a/vertx-web/src/test/java/io/vertx/ext/web/sstore/ClusteredSessionHandlerTest.java
+++ b/vertx-web/src/test/java/io/vertx/ext/web/sstore/ClusteredSessionHandlerTest.java
@@ -175,4 +175,21 @@ public class ClusteredSessionHandlerTest extends SessionHandlerTestBase {
     long val = doTestSessionRetryTimeout();
     assertTrue(String.valueOf(val), val >= 3000 && val < 5000);
   }
+
+  @Test
+  public void testDelayedLookupWithRequestUpgrade() {
+    String sessionCookieName = "session";
+    router.route().handler(SessionHandler.create(store).setSessionCookieName(sessionCookieName).setMinLength(0));
+    router.route().handler(rc -> {
+      rc.request().toWebSocket(onSuccess(serverWebSocket -> serverWebSocket.textMessageHandler(msg -> {
+        assertEquals("foo", msg);
+        testComplete();
+      })));
+    });
+    WebSocketConnectOptions options = new WebSocketConnectOptions()
+      .setURI("/")
+      .addHeader("cookie", sessionCookieName + "=" + TestUtils.randomAlphaString(32));
+    client.webSocket(options, onSuccess(ws -> ws.writeTextMessage("foo")));
+    await();
+  }
 }