Author: markt
Date: Tue Mar 19 14:22:33 2013
New Revision: 1458304

URL: http://svn.apache.org/r1458304
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54716
Make Calling on POJO method more robust if those methods throw an exception.

Added:
    
tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java   
(with props)
Modified:
    tomcat/trunk/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
    tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java

Modified: 
tomcat/trunk/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties?rev=1458304&r1=1458303&r2=1458304&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties 
Tue Mar 19 14:22:33 2013
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+pojoEndpointBase.closeSessionFail=Failed to close WebSocket session during 
error handling
 pojoEndpointBase.onCloseFail=Failed to call onClose method of POJO end point 
for POJO of type [{0}]
 pojoEndpointBase.onErrorFail=Failed to call onError method of POJO end point 
for POJO of type [{0}]
 pojoEndpointBase.onOpenFail=Failed to call onOpen method of POJO end point for 
POJO of type [{0}]

Modified: 
tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java?rev=1458304&r1=1458303&r2=1458304&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
Tue Mar 19 14:22:33 2013
@@ -16,6 +16,7 @@
  */
 package org.apache.tomcat.websocket.pojo;
 
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Map;
 import java.util.Set;
@@ -28,6 +29,7 @@ import javax.websocket.Session;
 
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -55,19 +57,40 @@ public abstract class PojoEndpointBase e
             try {
                 methodMapping.getOnOpen().invoke(pojo,
                         methodMapping.getOnOpenArgs(pathParameters, session));
-            } catch (IllegalAccessException | InvocationTargetException e) {
-                throw new IllegalArgumentException(sm.getString(
+
+                for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
+                        pathParameters, session, config)) {
+                    session.addMessageHandler(mh);
+                }
+            } catch (IllegalAccessException e) {
+                // Reflection related problems
+                log.error(sm.getString(
                         "pojoEndpointBase.onOpenFail",
                         pojo.getClass().getName()), e);
+                handleOnOpenError(session, e);
+            } catch (InvocationTargetException e) {
+                Throwable cause = e.getCause();
+                handleOnOpenError(session, cause);
+            } catch (Throwable t) {
+                handleOnOpenError(session, t);
             }
         }
-        for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
-                pathParameters, session, config)) {
-            session.addMessageHandler(mh);
-        }
     }
 
 
+    private void handleOnOpenError(Session session, Throwable t) {
+        // If really fatal - re-throw
+        ExceptionUtils.handleThrowable(t);
+
+        // Trigger the error handler and close the session
+        onError(session, t);
+        try {
+            session.close();
+        } catch (IOException ioe) {
+            log.warn(sm.getString("pojoEndpointBase.closeSessionFail"), ioe);
+        }
+    }
+
     @Override
     public final void onClose(Session session, CloseReason closeReason) {
 
@@ -75,10 +98,10 @@ public abstract class PojoEndpointBase e
             try {
                 methodMapping.getOnClose().invoke(pojo,
                         methodMapping.getOnCloseArgs(pathParameters, session, 
closeReason));
-            } catch (IllegalAccessException | IllegalArgumentException
-                    | InvocationTargetException e) {
+            } catch (Throwable t) {
+                ExceptionUtils.handleThrowable(t);
                 log.error(sm.getString("pojoEndpointBase.onCloseFail",
-                        pojo.getClass().getName()), e);
+                        pojo.getClass().getName()), t);
             }
         }
 
@@ -101,10 +124,10 @@ public abstract class PojoEndpointBase e
                         pojo,
                         methodMapping.getOnErrorArgs(pathParameters, session,
                                 throwable));
-            } catch (IllegalAccessException | IllegalArgumentException
-                    | InvocationTargetException e) {
+            } catch (Throwable t) {
+                ExceptionUtils.handleThrowable(t);
                 log.error(sm.getString("pojoEndpointBase.onErrorFail",
-                        pojo.getClass().getName()), e);
+                        pojo.getClass().getName()), t);
             }
         }
     }

Added: 
tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java?rev=1458304&view=auto
==============================================================================
--- 
tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java 
(added)
+++ 
tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java 
Tue Mar 19 14:22:33 2013
@@ -0,0 +1,96 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.tomcat.websocket.pojo;
+
+import java.net.URI;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import javax.websocket.ClientEndpoint;
+import javax.websocket.ContainerProvider;
+import javax.websocket.OnClose;
+import javax.websocket.OnOpen;
+import javax.websocket.WebSocketContainer;
+import javax.websocket.server.ServerEndpoint;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.websocket.pojo.Util.ServerConfigListener;
+import org.apache.tomcat.websocket.pojo.Util.SingletonConfigurator;
+
+
+public class TestPojoEndpointBase extends TomcatBaseTest {
+
+    @Test
+    public void testBug54716() throws Exception {
+        // Set up utility classes
+        Bug54716 server = new Bug54716();
+        SingletonConfigurator.setInstance(server);
+        ServerConfigListener.setPojoClazz(Bug54716.class);
+
+        Tomcat tomcat = getTomcatInstance();
+        // Must have a real docBase - just use temp
+        Context ctx =
+            tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+        ctx.addApplicationListener(ServerConfigListener.class.getName());
+
+        WebSocketContainer wsContainer =
+                ContainerProvider.getWebSocketContainer();
+
+
+        tomcat.start();
+
+        Client client = new Client();
+        URI uri = new URI("ws://localhost:" + getPort() + "/");
+
+        wsContainer.connectToServer(client, uri);
+
+        // Server should close the connection after the exception on open.
+        boolean closed = client.waitForClose(5);
+        Assert.assertTrue("Server failed to close connection", closed);
+    }
+
+
+    @ServerEndpoint("/")
+    public static class Bug54716 {
+
+        @OnOpen
+        public void onOpen() {
+            throw new RuntimeException();
+        }
+    }
+
+
+    @ClientEndpoint
+    public static final class Client {
+
+        private final CountDownLatch closeLatch = new CountDownLatch(1);
+
+        @OnClose
+        public void onClose() {
+            closeLatch.countDown();
+        }
+
+        public boolean waitForClose(int seconds) throws InterruptedException {
+            return closeLatch.await(seconds, TimeUnit.SECONDS);
+        }
+    }
+}

Propchange: 
tomcat/trunk/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to