Issue #272 - Read/Parse exceptions should flow out, so Session can handle it via new Session.close()

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/Parser.java

@@ -180,7 +180,7 @@ public class Parser
         return (flagsInUse & 0x10) != 0;
     }
 
-    protected void notifyFrame(final Frame f)
+    protected void notifyFrame(final Frame f) throws WebSocketException
     {
         if (LOG.isDebugEnabled())
             LOG.debug("{} Notify {}",policy.getBehavior(),getIncomingFramesHandler());
@@ -221,25 +221,14 @@ public class Parser
         }
         catch (WebSocketException e)
         {
-            notifyWebSocketException(e);
+            throw e;
         }
         catch (Throwable t)
         {
-            LOG.warn(t);
+            throw new WebSocketException(t);
-            notifyWebSocketException(new WebSocketException(t));
         }
     }
 
-    protected void notifyWebSocketException(WebSocketException e)
-    {
-        LOG.warn(e);
-        if (incomingFramesHandler == null)
-        {
-            return;
-        }
-        incomingFramesHandler.incomingError(e);
-    }
-
     public void parse(ByteBuffer buffer) throws WebSocketException
     {
         if (buffer.remaining() <= 0)
@@ -265,8 +254,6 @@ public class Parser
         {
             buffer.position(buffer.limit()); // consume remaining
             reset();
-            // let session know
-            notifyWebSocketException(e);
             // need to throw for proper close behavior in connection
             throw e;
         }
@@ -274,11 +261,8 @@ public class Parser
         {
             buffer.position(buffer.limit()); // consume remaining
             reset();
-            // let session know
-            WebSocketException e = new WebSocketException(t);
-            notifyWebSocketException(e);
             // need to throw for proper close behavior in connection
-            throw e;
+            throw new WebSocketException(t);
         }
     }
 

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/ParserTest.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.websocket.common;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
 import java.nio.ByteBuffer;
@@ -26,6 +27,7 @@ import java.util.Arrays;
 import java.util.List;
 
 import org.eclipse.jetty.util.StringUtil;
+import org.eclipse.jetty.websocket.api.ProtocolException;
 import org.eclipse.jetty.websocket.api.StatusCode;
 import org.eclipse.jetty.websocket.api.WebSocketBehavior;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
@@ -39,10 +41,15 @@ import org.eclipse.jetty.websocket.common.test.UnitGenerator;
 import org.eclipse.jetty.websocket.common.test.UnitParser;
 import org.eclipse.jetty.websocket.common.util.Hex;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class ParserTest
 {
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     /**
      * Similar to the server side 5.15 testcase. A normal 2 fragment text text message, followed by another continuation.
      */
@@ -61,11 +68,9 @@ public class ParserTest
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
 
+        expectedException.expect(ProtocolException.class);
+        expectedException.expectMessage(containsString("CONTINUATION frame without prior !FIN"));
         parser.parseQuietly(completeBuf);
-
-        capture.assertErrorCount(1);
-        capture.assertHasFrame(OpCode.TEXT,1);
-        capture.assertHasFrame(OpCode.CONTINUATION,1);
     }
 
     /**
@@ -83,10 +88,10 @@ public class ParserTest
         UnitParser parser = new UnitParser();
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
-        parser.parseQuietly(completeBuf);
 
-        capture.assertErrorCount(1);
+        expectedException.expect(ProtocolException.class);
-        capture.assertHasFrame(OpCode.TEXT,1); // fragment 1
+        expectedException.expectMessage(containsString("Unexpected TEXT frame"));
+        parser.parseQuietly(completeBuf);
     }
 
     /**

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/TextPayloadParserTest.java

@@ -28,17 +28,21 @@ import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 
 import org.eclipse.jetty.websocket.api.MessageTooLargeException;
-import org.eclipse.jetty.websocket.api.StatusCode;
 import org.eclipse.jetty.websocket.api.WebSocketBehavior;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.common.test.IncomingFramesCapture;
 import org.eclipse.jetty.websocket.common.test.UnitParser;
 import org.eclipse.jetty.websocket.common.util.MaskedByteBuffer;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class TextPayloadParserTest
 {
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     @Test
     public void testFrameTooLargeDueToPolicy() throws Exception
     {
@@ -63,19 +67,15 @@ public class TextPayloadParserTest
         UnitParser parser = new UnitParser(policy);
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
+
+        expectedException.expect(MessageTooLargeException.class);
         parser.parseQuietly(buf);
-
-        capture.assertHasErrors(MessageTooLargeException.class,1);
-        capture.assertHasNoFrames();
-
-        MessageTooLargeException err = (MessageTooLargeException)capture.getErrors().poll();
-        Assert.assertThat("Error.closeCode",err.getStatusCode(),is(StatusCode.MESSAGE_TOO_LARGE));
     }
 
     @Test
     public void testLongMaskedText() throws Exception
     {
-        StringBuffer sb = new StringBuffer(); ;
+        StringBuffer sb = new StringBuffer();
         for (int i = 0; i < 3500; i++)
         {
             sb.append("Hell\uFF4f Big W\uFF4Frld ");

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/ab/TestABCase2.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.websocket.common.ab;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
 import java.nio.ByteBuffer;
@@ -39,11 +40,16 @@ import org.eclipse.jetty.websocket.common.test.IncomingFramesCapture;
 import org.eclipse.jetty.websocket.common.test.UnitGenerator;
 import org.eclipse.jetty.websocket.common.test.UnitParser;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class TestABCase2
 {
-    WebSocketPolicy policy = new WebSocketPolicy(WebSocketBehavior.CLIENT);
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    private WebSocketPolicy policy = new WebSocketPolicy(WebSocketBehavior.CLIENT);
 
     @Test
     public void testGenerate125OctetPingCase2_4()
@@ -315,9 +321,10 @@ public class TestABCase2
         UnitParser parser = new UnitParser(policy);
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
-        parser.parseQuietly(expected);
 
-        Assert.assertEquals("error should be returned for too large of ping payload",1,capture.getErrorCount(ProtocolException.class));
+        expectedException.expect(ProtocolException.class);
+        expectedException.expectMessage(containsString("Invalid control frame payload length"));
+        parser.parseQuietly(expected);
     }
 
 }

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/ab/TestABCase4.java

@@ -23,16 +23,19 @@ import java.nio.ByteBuffer;
 import org.eclipse.jetty.util.log.StacklessLogging;
 import org.eclipse.jetty.websocket.api.ProtocolException;
 import org.eclipse.jetty.websocket.api.WebSocketBehavior;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.common.Parser;
 import org.eclipse.jetty.websocket.common.test.IncomingFramesCapture;
 import org.eclipse.jetty.websocket.common.test.UnitParser;
-import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class TestABCase4
 {
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     private WebSocketPolicy policy = new WebSocketPolicy(WebSocketBehavior.CLIENT);
 
     @Test
@@ -46,25 +49,13 @@ public class TestABCase4
 
         IncomingFramesCapture capture = new IncomingFramesCapture();
 
-        try (StacklessLogging logging = new StacklessLogging(Parser.class))
+        try (StacklessLogging ignore = new StacklessLogging(Parser.class))
         {
             Parser parser = new UnitParser(policy);
             parser.setIncomingFramesHandler(capture);
-            try
+            expectedException.expect(ProtocolException.class);
-            {
             parser.parse(expected);
         }
-            catch (ProtocolException ignore)
-            {
-                // ignore
-            }
-        }
-
-        Assert.assertEquals("error on undefined opcode",1,capture.getErrorCount(WebSocketException.class));
-
-        Throwable known = capture.getErrors().poll();
-
-        Assert.assertTrue("undefined option should be in message",known.getMessage().contains("Unknown opcode: 11"));
     }
 
     @Test
@@ -78,25 +69,13 @@ public class TestABCase4
 
         IncomingFramesCapture capture = new IncomingFramesCapture();
 
-        try (StacklessLogging logging = new StacklessLogging(Parser.class))
+        try (StacklessLogging ignore = new StacklessLogging(Parser.class))
         {
             Parser parser = new UnitParser(policy);
             parser.setIncomingFramesHandler(capture);
-            try
+            expectedException.expect(ProtocolException.class);
-            {
             parser.parse(expected);
         }
-            catch (ProtocolException ignore)
-            {
-                // ignore
-            }
-        }
-
-        Assert.assertEquals("error on undefined opcode",1,capture.getErrorCount(WebSocketException.class));
-
-        Throwable known = capture.getErrors().poll();
-
-        Assert.assertTrue("undefined option should be in message",known.getMessage().contains("Unknown opcode: 12"));
     }
 
     @Test
@@ -110,25 +89,13 @@ public class TestABCase4
 
         IncomingFramesCapture capture = new IncomingFramesCapture();
 
-        try (StacklessLogging logging = new StacklessLogging(Parser.class))
+        try (StacklessLogging ignore = new StacklessLogging(Parser.class))
         {
             Parser parser = new UnitParser(policy);
             parser.setIncomingFramesHandler(capture);
-            try
+            expectedException.expect(ProtocolException.class);
-            {
             parser.parse(expected);
         }
-            catch (ProtocolException ignore)
-            {
-                // ignore
-            }
-        }
-
-        Assert.assertEquals("error on undefined opcode",1,capture.getErrorCount(WebSocketException.class));
-
-        Throwable known = capture.getErrors().poll();
-
-        Assert.assertTrue("undefined option should be in message",known.getMessage().contains("Unknown opcode: 3"));
     }
 
     @Test
@@ -142,24 +109,12 @@ public class TestABCase4
 
         IncomingFramesCapture capture = new IncomingFramesCapture();
 
-        try (StacklessLogging logging = new StacklessLogging(Parser.class))
+        try (StacklessLogging ignore = new StacklessLogging(Parser.class))
         {
             Parser parser = new UnitParser(policy);
             parser.setIncomingFramesHandler(capture);
-            try
+            expectedException.expect(ProtocolException.class);
-            {
             parser.parse(expected);
         }
-            catch (ProtocolException ignore)
-            {
-                // ignore
-            }
-        }
-
-        Assert.assertEquals("error on undefined opcode",1,capture.getErrorCount(WebSocketException.class));
-
-        Throwable known = capture.getErrors().poll();
-
-        Assert.assertTrue("undefined option should be in message",known.getMessage().contains("Unknown opcode: 4"));
     }
 }

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/ab/TestABCase7_3.java

@@ -18,7 +18,6 @@
 
 package org.eclipse.jetty.websocket.common.ab;
 
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
 import java.nio.ByteBuffer;
@@ -40,11 +39,16 @@ import org.eclipse.jetty.websocket.common.test.UnitGenerator;
 import org.eclipse.jetty.websocket.common.test.UnitParser;
 import org.eclipse.jetty.websocket.common.util.Hex;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class TestABCase7_3
 {
-    WebSocketPolicy policy = new WebSocketPolicy(WebSocketBehavior.CLIENT);
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    private WebSocketPolicy policy = new WebSocketPolicy(WebSocketBehavior.CLIENT);
 
     @Test
     public void testCase7_3_1GenerateEmptyClose()
@@ -83,7 +87,6 @@ public class TestABCase7_3
 
         Frame pActual = capture.getFrames().poll();
         Assert.assertThat("CloseFrame.payloadLength",pActual.getPayloadLength(),is(0));
-
     }
 
 
@@ -104,13 +107,8 @@ public class TestABCase7_3
         UnitParser parser = new UnitParser(policy);
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
+        expectedException.expect(ProtocolException.class);
         parser.parseQuietly(expected);
-
-        Assert.assertEquals("error on invalid close payload",1,capture.getErrorCount(ProtocolException.class));
-
-        ProtocolException known = (ProtocolException)capture.getErrors().poll();
-
-        Assert.assertThat("Payload.message",known.getMessage(),containsString("Invalid close frame payload length"));
     }
 
     @Test
@@ -338,12 +336,7 @@ public class TestABCase7_3
         UnitParser parser = new UnitParser(policy);
         IncomingFramesCapture capture = new IncomingFramesCapture();
         parser.setIncomingFramesHandler(capture);
+        expectedException.expect(ProtocolException.class);
         parser.parseQuietly(expected);
-
-        Assert.assertEquals("error on invalid close payload",1,capture.getErrorCount(ProtocolException.class));
-
-        ProtocolException known = (ProtocolException)capture.getErrors().poll();
-
-        Assert.assertThat("Payload.message",known.getMessage(),containsString("Invalid control frame payload length"));
     }
 }

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/UnitParser.java

@@ -58,10 +58,6 @@ public class UnitParser extends Parser
         {
             parse(buf);
         }
-        catch (Exception ignore)
-        {
-            /* ignore */
-        }
     }
 
     public void parseSlowly(ByteBuffer buf, int segmentSize)