457017 - Reflective call to websocket methods that fail have ambiguous

exceptions 

+ Making JSR onOpen close and use onError properly, as well we
unwrapping the InvocationTargetException cause as to WHY the call to
onOpen failed.

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/JsrEvents.java

@@ -162,7 +162,7 @@ public class JsrEvents<T extends Annotation, C extends EndpointConfig>
         onOpen.call(websocket,config);
     }
 
-    public void callPong(RemoteEndpoint.Async endpoint, Object websocket, ByteBuffer pong) throws DecodeException, IOException
+    public void callPong(RemoteEndpoint.Async endpoint, Object websocket, ByteBuffer pong)
     {
         if (onPong == null)
         {

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessagePongCallable.java

@@ -21,8 +21,6 @@ package org.eclipse.jetty.websocket.jsr356.annotations;
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 
-import javax.websocket.DecodeException;
-
 import org.eclipse.jetty.websocket.jsr356.JsrPongMessage;
 import org.eclipse.jetty.websocket.jsr356.JsrSession;
 import org.eclipse.jetty.websocket.jsr356.annotations.Param.Role;
@@ -45,7 +43,7 @@ public class OnMessagePongCallable extends OnMessageCallable
         super(copy);
     }
 
-    public Object call(Object endpoint, ByteBuffer buf) throws DecodeException
+    public Object call(Object endpoint, ByteBuffer buf)
     {
         super.args[idxMessageObject] = new JsrPongMessage(buf);
         return super.call(endpoint,super.args);

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrAnnotatedEventDriver.java

@@ -23,6 +23,7 @@ import java.io.InputStream;
 import java.io.Reader;
 import java.nio.ByteBuffer;
 import java.util.Map;
+
 import javax.websocket.CloseReason;
 import javax.websocket.DecodeException;
 
@@ -123,7 +124,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
                         {
                             events.callBinaryStream(jsrsession.getAsyncRemote(),websocket,stream);
                         }
-                        catch (DecodeException | IOException e)
+                        catch (Throwable e)
                         {
                             onFatalError(e);
                         }
@@ -167,7 +168,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
             // FIN is always true here
             events.callBinary(jsrsession.getAsyncRemote(),websocket,buf,true);
         }
-        catch (DecodeException e)
+        catch (Throwable e)
         {
             onFatalError(e);
         }
@@ -187,9 +188,17 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
 
     @Override
     public void onError(Throwable cause)
+    {
+        try
         {
             events.callError(websocket,cause);
         }
+        catch (Throwable e)
+        {
+            LOG.warn("Unable to call onError with cause", cause);
+            LOG.warn("Call to onError resulted in exception", e);
+        }
+    }
 
     private void onFatalError(Throwable t)
     {
@@ -203,15 +212,15 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
     }
 
     @Override
-    public void onInputStream(InputStream stream)
+    public void onInputStream(InputStream stream) throws IOException
     {
         try
         {
             events.callBinaryStream(jsrsession.getAsyncRemote(),websocket,stream);
         }
-        catch (DecodeException | IOException e)
+        catch (DecodeException e)
         {
-            onFatalError(e);
+            throw new RuntimeException("Unable decode input stream", e);
         }
     }
 
@@ -223,7 +232,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
         }
         catch (DecodeException e)
         {
-            onFatalError(e);
+            throw new RuntimeException("Unable decode partial binary message", e);
         }
     }
 
@@ -235,46 +244,33 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
         }
         catch (DecodeException e)
         {
-            onFatalError(e);
+            throw new RuntimeException("Unable decode partial text message", e);
         }
     }
 
     @Override
     public void onPing(ByteBuffer buffer)
     {
-        try
+        // Call pong, as there is no "onPing" method in the JSR
-        {
         events.callPong(jsrsession.getAsyncRemote(),websocket,buffer);
     }
-        catch (DecodeException | IOException e)
-        {
-            onFatalError(e);
-        }
-    }
     
     @Override
     public void onPong(ByteBuffer buffer)
-    {
-        try
     {
         events.callPong(jsrsession.getAsyncRemote(),websocket,buffer);
     }
-        catch (DecodeException | IOException e)
-        {
-            onFatalError(e);
-        }
-    }
 
     @Override
-    public void onReader(Reader reader)
+    public void onReader(Reader reader) throws IOException
     {
         try
         {
             events.callTextStream(jsrsession.getAsyncRemote(),websocket,reader);
         }
-        catch (DecodeException | IOException e)
+        catch (DecodeException e)
         {
-            onFatalError(e);
+            throw new RuntimeException("Unable decode reader", e);
         }
     }
 
@@ -343,7 +339,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
                         {
                             events.callTextStream(jsrsession.getAsyncRemote(),websocket,stream);
                         }
-                        catch (DecodeException | IOException e)
+                        catch (Throwable e)
                         {
                             onFatalError(e);
                         }
@@ -380,7 +376,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver
             // FIN is always true here
             events.callText(jsrsession.getAsyncRemote(),websocket,message,true);
         }
-        catch (DecodeException e)
+        catch (Throwable e)
         {
             onFatalError(e);
         }

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrEndpointEventDriver.java

@@ -23,6 +23,7 @@ import java.io.InputStream;
 import java.io.Reader;
 import java.nio.ByteBuffer;
 import java.util.Map;
+
 import javax.websocket.CloseReason;
 import javax.websocket.Endpoint;
 import javax.websocket.MessageHandler;
@@ -134,21 +135,24 @@ public class JsrEndpointEventDriver extends AbstractJsrEventDriver
         {
             LOG.debug("onConnect({}, {})",jsrsession,config);
         }
-        try
+
-        {
+        // Let unhandled exceptions flow out
         endpoint.onOpen(jsrsession,config);
     }
-        catch (Throwable t)
-        {
-            LOG.warn("Uncaught exception",t);
-        }
-    }
 
     @Override
     public void onError(Throwable cause)
+    {
+        LOG.warn(cause);
+        try
         {
             endpoint.onError(jsrsession,cause);
         }
+        catch (Throwable t)
+        {
+            LOG.warn("Unable to report to onError due to exception",t);
+        }
+    }
 
     @Override
     public void onFrame(Frame frame)

jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/AnnotatedRuntimeOnOpen.java

@@ -0,0 +1,65 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2015 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket.jsr356.misbehaving;
+
+import java.util.LinkedList;
+import java.util.concurrent.CountDownLatch;
+
+import javax.websocket.ClientEndpoint;
+import javax.websocket.CloseReason;
+import javax.websocket.EndpointConfig;
+import javax.websocket.OnClose;
+import javax.websocket.OnError;
+import javax.websocket.OnOpen;
+import javax.websocket.Session;
+
+/**
+ * A JSR-356 Annotated that tosses a RuntimeException during its onOpen call
+ */
+@ClientEndpoint
+public class AnnotatedRuntimeOnOpen
+{
+    public CountDownLatch closeLatch = new CountDownLatch(1);
+    public CloseReason closeReason;
+    public LinkedList<Throwable> errors = new LinkedList<>();
+
+    @OnOpen
+    public void onOpen(Session session, EndpointConfig config)
+    {
+        // Intentional runtime exception.
+        int[] arr = new int[5];
+        for (int i = 0; i < 10; i++)
+        {
+            arr[i] = 222;
+        }
+    }
+
+    @OnClose
+    public void onClose(Session session, CloseReason closeReason)
+    {
+        this.closeReason = closeReason;
+        closeLatch.countDown();
+    }
+
+    @OnError
+    public void onError(Session session, Throwable thr)
+    {
+        errors.add(thr);
+    }
+}

jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/EndpointRuntimeOnOpen.java

@@ -0,0 +1,63 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2015 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket.jsr356.misbehaving;
+
+import java.util.LinkedList;
+import java.util.concurrent.CountDownLatch;
+
+import javax.websocket.CloseReason;
+import javax.websocket.Endpoint;
+import javax.websocket.EndpointConfig;
+import javax.websocket.Session;
+
+/**
+ * A JSR-356 Endpoint that tosses a RuntimeException during its onOpen call
+ */
+public class EndpointRuntimeOnOpen extends Endpoint
+{
+    public CountDownLatch closeLatch = new CountDownLatch(1);
+    public CloseReason closeReason;
+    public LinkedList<Throwable> errors = new LinkedList<>();
+
+    @Override
+    public void onOpen(Session session, EndpointConfig config)
+    {
+        // Intentional runtime exception.
+        int[] arr = new int[5];
+        for (int i = 0; i < 10; i++)
+        {
+            arr[i] = 222;
+        }
+    }
+
+    @Override
+    public void onClose(Session session, CloseReason closeReason)
+    {
+        super.onClose(session,closeReason);
+        this.closeReason = closeReason;
+        closeLatch.countDown();
+    }
+
+    @Override
+    public void onError(Session session, Throwable thr)
+    {
+        super.onError(session,thr);
+        errors.add(thr);
+    }
+}

jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/MisbehavingClassTest.java

@@ -0,0 +1,127 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2015 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket.jsr356.misbehaving;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.net.URI;
+import java.util.concurrent.TimeUnit;
+
+import javax.websocket.ContainerProvider;
+import javax.websocket.Session;
+import javax.websocket.WebSocketContainer;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.util.log.StacklessLogging;
+import org.eclipse.jetty.websocket.jsr356.EchoHandler;
+import org.eclipse.jetty.websocket.jsr356.endpoints.JsrEndpointEventDriver;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class MisbehavingClassTest
+{
+    private static Server server;
+    private static EchoHandler handler;
+    private static URI serverUri;
+
+    @BeforeClass
+    public static void startServer() throws Exception
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        handler = new EchoHandler();
+
+        ContextHandler context = new ContextHandler();
+        context.setContextPath("/");
+        context.setHandler(handler);
+        server.setHandler(context);
+
+        // Start Server
+        server.start();
+
+        String host = connector.getHost();
+        if (host == null)
+        {
+            host = "localhost";
+        }
+        int port = connector.getLocalPort();
+        serverUri = new URI(String.format("ws://%s:%d/",host,port));
+    }
+
+    @AfterClass
+    public static void stopServer()
+    {
+        try
+        {
+            server.stop();
+        }
+        catch (Exception e)
+        {
+            e.printStackTrace(System.err);
+        }
+    }
+
+    @Test
+    public void testEndpointRuntimeOnOpen() throws Exception
+    {
+        WebSocketContainer container = ContainerProvider.getWebSocketContainer();
+        EndpointRuntimeOnOpen socket = new EndpointRuntimeOnOpen();
+
+        try (StacklessLogging logging = new StacklessLogging(EndpointRuntimeOnOpen.class,JsrEndpointEventDriver.class))
+        {
+            // expecting ArrayIndexOutOfBoundsException during onOpen
+            Session session = container.connectToServer(socket,serverUri);
+            assertThat("Close should have occurred",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true));
+
+            // technically, the session object isn't invalid here.
+            assertThat("Session.isOpen",session.isOpen(),is(false));
+            assertThat("Should have only had 1 error",socket.errors.size(),is(1));
+
+            Throwable cause = socket.errors.pop();
+            assertThat("Error",cause,instanceOf(ArrayIndexOutOfBoundsException.class));
+        }
+    }
+
+    @Test
+    public void testAnnotatedRuntimeOnOpen() throws Exception
+    {
+        WebSocketContainer container = ContainerProvider.getWebSocketContainer();
+        AnnotatedRuntimeOnOpen socket = new AnnotatedRuntimeOnOpen();
+
+        try (StacklessLogging logging = new StacklessLogging(AnnotatedRuntimeOnOpen.class))
+        {
+            // expecting ArrayIndexOutOfBoundsException during onOpen
+            Session session = container.connectToServer(socket,serverUri);
+            assertThat("Close should have occurred",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true));
+
+            // technically, the session object isn't invalid here.
+            assertThat("Session.isOpen",session.isOpen(),is(false));
+            assertThat("Should have only had 1 error",socket.errors.size(),is(1));
+
+            Throwable cause = socket.errors.pop();
+            assertThat("Error",cause,instanceOf(ArrayIndexOutOfBoundsException.class));
+        }
+    }
+}

jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/MemoryUsageTest.java

@@ -26,6 +26,7 @@ import java.lang.management.MemoryUsage;
 import java.net.URI;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+
 import javax.websocket.ContainerProvider;
 import javax.websocket.Endpoint;
 import javax.websocket.EndpointConfig;
@@ -74,6 +75,7 @@ public class MemoryUsageTest
         server.stop();
     }
 
+    @SuppressWarnings("unused")
     @Test
     public void testMemoryUsage() throws Exception
     {

jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/CloseStatus.java

@@ -32,6 +32,11 @@ public class CloseStatus
      */
     public static String trimMaxReasonLength(String reason)
     {
+        if (reason == null)
+        {
+            return null;
+        }
+        
         if (reason.length() > MAX_REASON_PHRASE)
         {
             return reason.substring(0,MAX_REASON_PHRASE);

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

@@ -105,7 +105,7 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc
     @Override
     public void close(int statusCode, String reason)
     {
-        connection.close(statusCode,reason);
+        connection.close(statusCode,CloseStatus.trimMaxReasonLength(reason));
     }
 
     /**

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java

@@ -49,13 +49,13 @@ public interface EventDriver extends IncomingFrames
 
     public void onFrame(Frame frame);
 
-    public void onInputStream(InputStream stream);
+    public void onInputStream(InputStream stream) throws IOException;
 
     public void onPing(ByteBuffer buffer);
     
     public void onPong(ByteBuffer buffer);
 
-    public void onReader(Reader reader);
+    public void onReader(Reader reader) throws IOException;
 
     public void onTextFrame(ByteBuffer buffer, boolean fin) throws IOException;
 

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java

@@ -85,9 +85,17 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver
                 {
                     @Override
                     public void run()
+                    {
+                        try
                         {
                             events.onBinary.call(websocket,session,msg);
                         }
+                        catch (Throwable t)
+                        {
+                            // dispatched calls need to be reported
+                            onError(t);
+                        }
+                    }
                 });
             }
             else
@@ -187,9 +195,17 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver
                 {
                     @Override
                     public void run()
+                    {
+                        try
                         {
                             events.onText.call(websocket,session,msg);
                         }
+                        catch (Throwable t)
+                        {
+                            // dispatched calls need to be reported
+                            onError(t);
+                        }
+                    }
                 });
             }
             else

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/AbstractMethodAnnotationScanner.java

@@ -22,7 +22,6 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 
-import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.websocket.api.InvalidWebSocketException;
 import org.eclipse.jetty.websocket.common.events.ParamList;
 
@@ -39,7 +38,7 @@ public abstract class AbstractMethodAnnotationScanner<T>
             StringBuilder err = new StringBuilder();
             err.append("Invalid declaration of ");
             err.append(method);
-            err.append(StringUtil.__LINE_SEPARATOR);
+            err.append(System.lineSeparator());
 
             err.append("Method modifier must be public");
 
@@ -51,7 +50,7 @@ public abstract class AbstractMethodAnnotationScanner<T>
             StringBuilder err = new StringBuilder();
             err.append("Invalid declaration of ");
             err.append(method);
-            err.append(StringUtil.__LINE_SEPARATOR);
+            err.append(System.lineSeparator());
 
             err.append("Method modifier may not be static");
 
@@ -66,7 +65,7 @@ public abstract class AbstractMethodAnnotationScanner<T>
             StringBuilder err = new StringBuilder();
             err.append("Invalid declaration of ");
             err.append(method);
-            err.append(StringUtil.__LINE_SEPARATOR);
+            err.append(System.lineSeparator());
 
             err.append("Return type must be ").append(type);
 
@@ -87,7 +86,7 @@ public abstract class AbstractMethodAnnotationScanner<T>
             StringBuilder err = new StringBuilder();
             err.append("Duplicate @").append(annoClass.getSimpleName()).append(" declaration on ");
             err.append(method);
-            err.append(StringUtil.__LINE_SEPARATOR);
+            err.append(System.lineSeparator());
 
             err.append("@").append(annoClass.getSimpleName()).append(" previously declared at ");
             err.append(callable.getMethod());

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/CallableMethod.java

@@ -24,7 +24,6 @@ import java.util.Objects;
 
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.common.util.ReflectUtils;
 
 /**
@@ -70,7 +69,31 @@ public class CallableMethod
         {
             return this.method.invoke(obj,args);
         }
-        catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e)
+        catch (Throwable t)
+        {
+            String err = formatMethodCallError(args);
+            throw unwrapRuntimeException(err,t);
+        }
+    }
+
+    private RuntimeException unwrapRuntimeException(String err, final Throwable t)
+    {
+        Throwable ret = t;
+
+        while (ret instanceof InvocationTargetException)
+        {
+            ret = ((InvocationTargetException)ret).getCause();
+        }
+
+        if (ret instanceof RuntimeException)
+        {
+            return (RuntimeException)ret;
+        }
+
+        return new RuntimeException(err,ret);
+    }
+
+    public String formatMethodCallError(Object... args)
     {
         StringBuilder err = new StringBuilder();
         err.append("Cannot call method ");
@@ -95,9 +118,7 @@ public class CallableMethod
             delim = true;
         }
         err.append("]");
-
+        return err.toString();
-            throw new WebSocketException(err.toString(),e);
-        }
     }
 
     public Method getMethod()

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/InvalidSignatureException.java

@@ -21,7 +21,6 @@ package org.eclipse.jetty.websocket.common.events.annotated;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 
-import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.websocket.api.InvalidWebSocketException;
 import org.eclipse.jetty.websocket.common.events.ParamList;
 
@@ -34,7 +33,7 @@ public class InvalidSignatureException extends InvalidWebSocketException
         StringBuilder err = new StringBuilder();
         err.append("Invalid declaration of ");
         err.append(method);
-        err.append(StringUtil.__LINE_SEPARATOR);
+        err.append(System.lineSeparator());
 
         err.append("Acceptable method declarations for @");
         err.append(annoClass.getSimpleName());
@@ -43,7 +42,7 @@ public class InvalidSignatureException extends InvalidWebSocketException
         {
             for (Class<?>[] params : validParams)
             {
-                err.append(StringUtil.__LINE_SEPARATOR);
+                err.append(System.lineSeparator());
                 err.append("public void ").append(method.getName());
                 err.append('(');
                 boolean delim = false;