Issue #4515 - ValidationExtension should not downcast CoreSession

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/AbstractExtension.java

@@ -26,11 +26,11 @@
 public class AbstractExtension implements Extension
 {
     private final Logger log;
+    private CoreSession coreSession;
     private ByteBufferPool bufferPool;
     private ExtensionConfig config;
     private OutgoingFrames nextOutgoing;
     private IncomingFrames nextIncoming;
-    private Configuration configuration;
     private DeflaterPool deflaterPool;
     private InflaterPool inflaterPool;
 
@@ -165,12 +165,17 @@ public void setNextOutgoingFrames(OutgoingFrames nextOutgoing)
     @Override
     public void setCoreSession(CoreSession coreSession)
     {
-        this.configuration = coreSession;
+        this.coreSession = coreSession;
+    }
+
+    public CoreSession getCoreSession()
+    {
+        return coreSession;
     }
 
     protected Configuration getConfiguration()
     {
-        return configuration;
+        return coreSession;
     }
 
     @Override

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java

@@ -77,8 +77,7 @@ public interface CoreSession extends OutgoingFrames, Configuration
     Behavior getBehavior();
 
     /**
-     * TODO
-     * @return
+     * @return the WebSocketComponents instance in use for this Connection.
      */
     WebSocketComponents getWebSocketComponents();
 
@@ -166,6 +165,21 @@ public interface CoreSession extends OutgoingFrames, Configuration
      */
     void demand(long n);
 
+    /**
+     * @return true if an extension has been negotiated which uses the RSV1 bit.
+     */
+    boolean isRsv1Used();
+
+    /**
+     * @return true if an extension has been negotiated which uses the RSV2 bit.
+     */
+    boolean isRsv2Used();
+
+    /**
+     * @return true if an extension has been negotiated which uses the RSV3 bit.
+     */
+    boolean isRsv3Used();
+
     class Empty extends ConfigurationCustomizer implements CoreSession
     {
         @Override
@@ -273,5 +287,23 @@ public void sendFrame(Frame frame, Callback callback, boolean batch)
         {
             callback.succeeded();
         }
+
+        @Override
+        public boolean isRsv1Used()
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isRsv2Used()
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isRsv3Used()
+        {
+            return false;
+        }
     }
 }

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/Extension.java

@@ -81,8 +81,7 @@ public interface Extension extends IncomingFrames, OutgoingFrames
     void setNextOutgoingFrames(OutgoingFrames nextOutgoing);
 
     /**
-     * Set the {@link CoreSession} for this Extension.
-     * @param coreSession
+     * @param coreSession the {@link CoreSession} for this Extension.
      */
     void setCoreSession(CoreSession coreSession);
 }

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java

@@ -20,7 +20,6 @@
 public class FrameSequence
 {
     private final AutoLock lock = new AutoLock();
-    // TODO should we be able to get a non fin frame then get a close frame without error
     private byte state = OpCode.UNDEFINED;
 
     public void check(byte opcode, boolean fin) throws ProtocolException
@@ -40,15 +39,15 @@ public void check(byte opcode, boolean fin) throws ProtocolException
                         throw new ProtocolException("CONTINUATION after fin==true");
                     if (fin)
                         state = OpCode.UNDEFINED;
-                    return;
+                    break;
 
                 case OpCode.CLOSE:
                     state = OpCode.CLOSE;
-                    return;
+                    break;
 
                 case OpCode.PING:
                 case OpCode.PONG:
-                    return;
+                    break;
 
                 case OpCode.TEXT:
                 case OpCode.BINARY:
@@ -57,7 +56,7 @@ public void check(byte opcode, boolean fin) throws ProtocolException
                         throw new ProtocolException("DataFrame before fin==true");
                     if (!fin)
                         state = opcode;
-                    return;
+                    break;
             }
         }
     }

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/ValidationExtension.java

@@ -17,11 +17,11 @@
 
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.websocket.core.AbstractExtension;
-import org.eclipse.jetty.websocket.core.CoreSession;
 import org.eclipse.jetty.websocket.core.ExtensionConfig;
 import org.eclipse.jetty.websocket.core.Frame;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.exception.ProtocolException;
+import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,7 +33,6 @@ public class ValidationExtension extends AbstractExtension
 {
     private static final Logger LOG = LoggerFactory.getLogger(ValidationExtension.class);
 
-    private WebSocketCoreSession coreSession;
     private FrameSequence incomingSequence = null;
     private FrameSequence outgoingSequence = null;
     private boolean incomingFrameValidation = false;
@@ -49,17 +48,6 @@ public String getName()
         return "@validation";
     }
 
-    @Override
-    public void setCoreSession(CoreSession coreSession)
-    {
-        super.setCoreSession(coreSession);
-
-        // TODO: change validation to use static methods instead of down casting CoreSession.
-        if (!(coreSession instanceof WebSocketCoreSession))
-            throw new IllegalArgumentException("ValidationExtension needs a CoreSession Configuration");
-        this.coreSession = (WebSocketCoreSession)coreSession;
-    }
-
     @Override
     public void onFrame(Frame frame, Callback callback)
     {
@@ -69,7 +57,7 @@ public void onFrame(Frame frame, Callback callback)
                 incomingSequence.check(frame.getOpCode(), frame.isFin());
 
             if (incomingFrameValidation)
-                coreSession.assertValidIncoming(frame);
+                FrameValidation.assertValidIncoming(frame, getCoreSession());
 
             if (incomingUtf8Validation != null)
                 validateUTF8(frame, incomingUtf8Validation, continuedInOpCode);
@@ -92,7 +80,7 @@ public void sendFrame(Frame frame, Callback callback, boolean batch)
                 outgoingSequence.check(frame.getOpCode(), frame.isFin());
 
             if (outgoingFrameValidation)
-                coreSession.assertValidOutgoing(frame);
+                FrameValidation.assertValidOutgoing(frame, getCoreSession());
 
             if (outgoingUtf8Validation != null)
                 validateUTF8(frame, outgoingUtf8Validation, continuedOutOpCode);

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java

@@ -43,11 +43,10 @@
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.WebSocketConstants;
 import org.eclipse.jetty.websocket.core.exception.CloseException;
-import org.eclipse.jetty.websocket.core.exception.MessageTooLargeException;
 import org.eclipse.jetty.websocket.core.exception.ProtocolException;
 import org.eclipse.jetty.websocket.core.exception.WebSocketTimeoutException;
 import org.eclipse.jetty.websocket.core.exception.WebSocketWriteTimeoutException;
-import org.eclipse.jetty.websocket.core.internal.Parser.ParsedFrame;
+import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -109,106 +108,6 @@ public boolean isDemanding()
         return demanding;
     }
 
-    public void assertValidIncoming(Frame frame)
-    {
-        assertValidFrame(frame);
-
-        // Validate frame size.
-        if (maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
-            throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
-
-        // Assert Incoming Frame Behavior Required by RFC-6455 / Section 5.1
-        switch (behavior)
-        {
-            case SERVER:
-                if (!frame.isMasked())
-                    throw new ProtocolException("Client MUST mask all frames (RFC-6455: Section 5.1)");
-                break;
-
-            case CLIENT:
-                if (frame.isMasked())
-                    throw new ProtocolException("Server MUST NOT mask any frames (RFC-6455: Section 5.1)");
-                break;
-
-            default:
-                throw new IllegalStateException(behavior.toString());
-        }
-
-        /*
-         * RFC 6455 Section 5.5.1
-         * close frame payload is specially formatted which is checked in CloseStatus
-         */
-        if (frame.getOpCode() == OpCode.CLOSE)
-        {
-            if (!(frame instanceof ParsedFrame)) // already check in parser
-                CloseStatus.getCloseStatus(frame); // return ignored as get used to validate there is a closeStatus
-        }
-    }
-
-    public void assertValidOutgoing(Frame frame) throws CloseException
-    {
-        assertValidFrame(frame);
-
-        // Validate frame size (allowed to be over max frame size if autoFragment is true).
-        if (!autoFragment && maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
-            throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
-
-        /*
-         * RFC 6455 Section 5.5.1
-         * close frame payload is specially formatted which is checked in CloseStatus
-         */
-        if (frame.getOpCode() == OpCode.CLOSE)
-        {
-            if (!(frame instanceof ParsedFrame)) // already check in parser
-            {
-                CloseStatus closeStatus = CloseStatus.getCloseStatus(frame);
-                if (!CloseStatus.isTransmittableStatusCode(closeStatus.getCode()) && (closeStatus.getCode() != CloseStatus.NO_CODE))
-                {
-                    throw new ProtocolException("Frame has non-transmittable status code");
-                }
-            }
-        }
-    }
-
-    public void assertValidFrame(Frame frame)
-    {
-        if (!OpCode.isKnown(frame.getOpCode()))
-            throw new ProtocolException("Unknown opcode: " + frame.getOpCode());
-
-        int payloadLength = frame.getPayloadLength();
-        if (frame.isControlFrame())
-        {
-            if (!frame.isFin())
-                throw new ProtocolException("Fragmented Control Frame [" + OpCode.name(frame.getOpCode()) + "]");
-
-            if (payloadLength > Frame.MAX_CONTROL_PAYLOAD)
-                throw new ProtocolException("Invalid control frame payload length, [" + payloadLength + "] cannot exceed [" + Frame.MAX_CONTROL_PAYLOAD + "]");
-
-            if (frame.isRsv1())
-                throw new ProtocolException("Cannot have RSV1==true on Control frames");
-            if (frame.isRsv2())
-                throw new ProtocolException("Cannot have RSV2==true on Control frames");
-            if (frame.isRsv3())
-                throw new ProtocolException("Cannot have RSV3==true on Control frames");
-        }
-        else
-        {
-            /*
-             * RFC 6455 Section 5.2
-             *
-             * MUST be 0 unless an extension is negotiated that defines meanings for non-zero values. If a nonzero value is received and none of the negotiated
-             * extensions defines the meaning of such a nonzero value, the receiving endpoint MUST _Fail the WebSocket Connection_.
-             */
-            ExtensionStack extensionStack = negotiated.getExtensions();
-            if (frame.isRsv1() && !extensionStack.isRsv1Used())
-                throw new ProtocolException("RSV1 not allowed to be set");
-            if (frame.isRsv2() && !extensionStack.isRsv2Used())
-                throw new ProtocolException("RSV2 not allowed to be set");
-            if (frame.isRsv3() && !extensionStack.isRsv3Used())
-                throw new ProtocolException("RSV3 not allowed to be set");
-        }
-    }
-
     public ExtensionStack getExtensionStack()
     {
         return negotiated.getExtensions();
@@ -501,6 +400,24 @@ public void demand(long n)
         connection.demand(n);
     }
 
+    @Override
+    public boolean isRsv1Used()
+    {
+        return getExtensionStack().isRsv1Used();
+    }
+
+    @Override
+    public boolean isRsv2Used()
+    {
+        return getExtensionStack().isRsv2Used();
+    }
+
+    @Override
+    public boolean isRsv3Used()
+    {
+        return getExtensionStack().isRsv3Used();
+    }
+
     public WebSocketConnection getConnection()
     {
         return connection;
@@ -519,7 +436,7 @@ public void onFrame(Frame frame, Callback callback)
 
         try
         {
-            assertValidIncoming(frame);
+            FrameValidation.assertValidIncoming(frame, this);
         }
         catch (Throwable t)
         {
@@ -546,7 +463,7 @@ public void sendFrame(Frame frame, Callback callback, boolean batch)
 
         try
         {
-            assertValidOutgoing(frame);
+            FrameValidation.assertValidOutgoing(frame, this);
         }
         catch (Throwable t)
         {