Merge pull request #4750 from eclipse/jetty-10.0.x-4747-WebSocketTCK

Issue #4747 - fix some test failures for jetty-10 with websocket tck
jetty · Apr 23, 2020 · 53246f9 · 53246f9
2 parents 1d96dda + c6e58e6
commit 53246f9
Show file tree

Hide file tree

Showing 24 changed files with 436 additions and 304 deletions.
diff --git a/...cket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java b/...cket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java
@@ -409,8 +409,6 @@ else if (values.length == 1)
 
         // Verify the negotiated subprotocol
         List<String> offeredSubProtocols = getSubProtocols();
-        if (negotiatedSubProtocol == null && !offeredSubProtocols.isEmpty())
-            throw new WebSocketException("Upgrade failed: no subprotocol selected from offered subprotocols ");
         if (negotiatedSubProtocol != null && !offeredSubProtocols.contains(negotiatedSubProtocol))
             throw new WebSocketException("Upgrade failed: subprotocol [" + negotiatedSubProtocol + "] not found in offered subprotocols " + offeredSubProtocols);
 

diff --git a/...et-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java b/...et-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java
@@ -679,6 +679,7 @@ private class IncomingAdaptor implements IncomingFrames
         @Override
         public void onFrame(Frame frame, final Callback callback)
         {
+            Callback closeCallback = null;
             try
             {
                 if (LOG.isDebugEnabled())
@@ -695,11 +696,13 @@ public void onFrame(Frame frame, final Callback callback)
 
                 // Handle inbound CLOSE
                 connection.cancelDemand();
-                Callback closeCallback;
-
                 if (closeConnection)
                 {
-                    closeCallback = Callback.from(() -> closeConnection(sessionState.getCloseStatus(), callback));
+                    closeCallback = Callback.from(() -> closeConnection(sessionState.getCloseStatus(), callback), t ->
+                    {
+                        sessionState.onError(t);
+                        closeConnection(sessionState.getCloseStatus(), callback);
+                    });
                 }
                 else
                 {
@@ -725,7 +728,10 @@ public void onFrame(Frame frame, final Callback callback)
             }
             catch (Throwable t)
             {
-                callback.failed(t);
+                if (closeCallback != null)
+                    closeCallback.failed(t);
+                else
+                    callback.failed(t);
             }
         }
     }

diff --git a/...t-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java b/...t-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java
@@ -20,6 +20,7 @@
 
 import java.nio.channels.ClosedChannelException;
 
+import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.websocket.core.CloseStatus;
 import org.eclipse.jetty.websocket.core.Frame;
 import org.eclipse.jetty.websocket.core.OpCode;
@@ -123,6 +124,39 @@ public boolean onClosed(CloseStatus closeStatus)
         }
     }
 
+    /**
+     * <p>
+     * If no error is set in the CloseStatus this will either, replace the current close status with
+     * a {@link CloseStatus#SERVER_ERROR} status if we had a NORMAL close code, or, it will set the cause
+     * of the CloseStatus if the previous cause was null, this allows onError to be notified after the connection is closed.
+     * </p>
+     * <p>
+     * This should only be called if there is an error directly before the call to
+     * {@link WebSocketCoreSession#closeConnection(CloseStatus, Callback)}.
+     * </p>
+     * <p>
+     * This could occur if the FrameHandler throws an exception in onFrame after receiving a close frame reply, in this
+     * case to notify onError we must set the cause in the closeStatus.
+     * </p>
+     * @param t the error which occurred.
+     */
+    public void onError(Throwable t)
+    {
+        synchronized (this)
+        {
+            if (_sessionState != State.CLOSED || _closeStatus == null)
+                throw new IllegalArgumentException();
+
+            // Override any normal close status.
+            if (!_closeStatus.isAbnormal())
+                _closeStatus = new CloseStatus(CloseStatus.SERVER_ERROR, t);
+
+            // Otherwise set the error if it wasn't already set to notify onError as well as onClose.
+            if (_closeStatus.getCause() == null)
+                _closeStatus = new CloseStatus(_closeStatus.getCode(), _closeStatus.getReason(), t);
+        }
+    }
+
     public boolean onEof()
     {
         synchronized (this)

diff --git a/...re/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java b/...re/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java
@@ -96,19 +96,14 @@ public boolean upgradeRequest(WebSocketNegotiator negotiator, HttpServletRequest
             return false;
         }
 
-        // Validate negotiated protocol
+        // Validate negotiated protocol.
         String protocol = negotiation.getSubprotocol();
         List<String> offeredProtocols = negotiation.getOfferedSubprotocols();
         if (protocol != null)
         {
             if (!offeredProtocols.contains(protocol))
                 throw new WebSocketException("not upgraded: selected a protocol not present in offered protocols");
         }
-        else
-        {
-            if (!offeredProtocols.isEmpty())
-                throw new WebSocketException("not upgraded: no protocol selected from offered protocols");
-        }
 
         // validate negotiated extensions
         for (ExtensionConfig config : negotiation.getNegotiatedExtensions())