WebSocket fixes for NPE and clean up

Red5 · Sep 1, 2022 · 4917658 · 4917658
1 parent 47d62bf
commit 4917658
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 45 deletions.
diff --git a/client/pom.xml b/client/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.0</version>
+        <version>1.3.1</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-client</artifactId>

diff --git a/client/src/main/java/org/red5/client/Red5Client.java b/client/src/main/java/org/red5/client/Red5Client.java
@@ -18,7 +18,7 @@ public final class Red5Client {
     /**
      * Current server version with revision
      */
-    public static final String VERSION = "Red5 Client 1.3.0";
+    public static final String VERSION = "Red5 Client 1.3.1";
 
     /**
      * Create a new Red5Client object using the connection local to the current thread A bit of magic that lets you access the red5 scope

diff --git a/common/pom.xml b/common/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.0</version>
+        <version>1.3.1</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-server-common</artifactId>

diff --git a/common/src/main/java/org/red5/server/api/Red5.java b/common/src/main/java/org/red5/server/api/Red5.java
@@ -55,12 +55,12 @@ public final class Red5 {
     /**
      * Server version with revision
      */
-    public static final String VERSION = "Red5 Server 1.3.0";
+    public static final String VERSION = "Red5 Server 1.3.1";
 
     /**
      * Server version for fmsVer requests
      */
-    public static final String FMS_VERSION = "RED5/1,3,0,0";
+    public static final String FMS_VERSION = "RED5/1,3,1,0";
 
     /**
      * Server capabilities

diff --git a/common/src/main/java/org/red5/server/scheduling/JDKSchedulingServiceJob.java b/common/src/main/java/org/red5/server/scheduling/JDKSchedulingServiceJob.java
@@ -54,11 +54,11 @@ public void run() {
         IScheduledJob job = null;
         try {
             job = (IScheduledJob) jobDataMap.get(ISchedulingService.SCHEDULED_JOB);
-            job.execute(service);
+            if (job != null) {
+                job.execute(service);
+            }
         } catch (Throwable e) {
-            if (job == null) {
-                log.warn("Job not found");
-            } else {
+            if (job != null) {
                 log.warn("Job {} execution failed", job.toString(), e);
             }
         } finally {

diff --git a/io/pom.xml b/io/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.0</version>
+        <version>1.3.1</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-io</artifactId>

diff --git a/pom.xml b/pom.xml
@@ -23,7 +23,7 @@
     <name>Red5</name>
     <description>The Red5 server</description>
     <groupId>org.red5</groupId>
-    <version>1.3.0</version>
+    <version>1.3.1</version>
     <url>https://github.com/Red5/red5-server</url>
     <inceptionYear>2005</inceptionYear>
     <organization>

diff --git a/server/pom.xml b/server/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.0</version>
+        <version>1.3.1</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-server</artifactId>

diff --git a/server/src/main/java/org/red5/net/websocket/WebSocketConnection.java b/server/src/main/java/org/red5/net/websocket/WebSocketConnection.java
@@ -329,15 +329,21 @@ public void close() {
                 CloseReason reason = new CloseReason(CloseCodes.GOING_AWAY, "");
                 // close the socket, don't wait for the browser to respond or we could hang
                 session.onClose(reason);
-                // clean up our props
-                attributes.clear();
+            }
+            // clean up our props
+            attributes.clear();
+            if (querystringParameters != null) {
                 querystringParameters.clear();
                 querystringParameters = null;
+            }
+            if (extensions != null) {
                 extensions.clear();
                 extensions = null;
-                if (headers != null) {
-                    headers = null;
-                }
+            }
+            if (headers != null) {
+                headers = null;
+            }
+            if (scope.get() != null) {
                 // disconnect from scope
                 scope.get().removeConnection(this);
                 // clear weak refs
@@ -419,7 +425,11 @@ public void setOrigin(String origin) {
      * @return true if secure and false if unsecure or unconnected
      */
     public boolean isSecure() {
-        return (wsSession != null && wsSession.get().isOpen()) ? wsSession.get().isSecure() : false;
+        Optional<WsSession> opt = Optional.ofNullable(wsSession.get());
+        if (opt.isPresent()) {
+            return (opt.get().isOpen() ? opt.get().isSecure() : false);
+        }
+        return false;
     }
 
     public String getPath() {

diff --git a/server/src/main/java/org/red5/net/websocket/WebSocketScopeManager.java b/server/src/main/java/org/red5/net/websocket/WebSocketScopeManager.java
@@ -159,28 +159,34 @@ public boolean addWebSocketScope(WebSocketScope webSocketScope) {
                         scopes.forEach((sName, wsScope) -> {
                             log.trace("start pinging scope: {}", sName);
                             wsScope.getConns().forEach(wsConn -> {
-                                // ping connected websocket
-                                if (wsConn.isConnected()) {
-                                    log.debug("pinging ws: {} on scope: {}", wsConn.getWsSessionId(), sName);
-                                    try {
-                                        wsConn.sendPing(PING_BYTES);
-                                    } catch (Exception e) {
-                                        log.debug("Exception pinging connection: {} connection will be closed", wsConn.getSessionId(), e);
-                                        // if the ping fails, consider them gone
-                                        wsConn.close();
+                                try {
+                                    // ping connected websocket
+                                    if (wsConn.isConnected()) {
+                                        log.debug("pinging ws: {} on scope: {}", wsConn.getWsSessionId(), sName);
+                                        try {
+                                            wsConn.sendPing(PING_BYTES);
+                                        } catch (Exception e) {
+                                            log.debug("Exception pinging connection: {} connection will be closed", wsConn.getSessionId(), e);
+                                            // if the connection isn't connected, remove them
+                                            wsScope.removeConnection(wsConn);
+                                            // if the ping fails, consider them gone
+                                            wsConn.close();
+                                        }
+                                    } else {
+                                        log.debug("Removing unconnected connection: {} during ping loop", wsConn.getSessionId());
+                                        // if the connection isn't connected, remove them
+                                        wsScope.removeConnection(wsConn);
                                     }
-                                } else {
-                                    log.debug("Removing unconnected connection: {} during ping loop", wsConn.getSessionId());
-                                    // if the connection isn't connected, remove them
-                                    wsScope.removeConnection(wsConn);
+                                } catch (Exception e) {
+                                    log.warn("Exception in WS pinger", e);
                                 }
                             });
                             log.trace("finished pinging scope: {}", sName);
                         });
                         // sleep for interval
                         try {
                             Thread.sleep(websocketPingInterval);
-                        } catch (InterruptedException e1) {
+                        } catch (InterruptedException e) {
                         }
                     } while (!scopes.isEmpty());
                     // reset ping future

diff --git a/server/src/main/java/org/red5/net/websocket/server/DefaultWebSocketEndpoint.java b/server/src/main/java/org/red5/net/websocket/server/DefaultWebSocketEndpoint.java
@@ -73,20 +73,31 @@ public void onOpen(Session session, EndpointConfig config) {
     public void onClose(Session session, CloseReason closeReason) {
         final String sessionId = session.getId();
         log.debug("Session closed: {}", sessionId);
-        // get ws connection from session user props
-        WebSocketConnection conn = (WebSocketConnection) session.getUserProperties().get(WSConstants.WS_CONNECTION);
-        // if we don't get it from the session, try the scope lookup
-        if (conn == null) {
-            log.warn("Connection for id: {} was not found in the session onClose", sessionId);
-            conn = scope.getConnectionBySessionId(sessionId);
-        }
-        if (conn != null) {
-            // close the ws conn which removes it from the scope
-            if (conn.isConnected()) {
+        WebSocketConnection conn = null;
+        // getting the sessions user properties on a closed connection will throw an exception when it checks state
+        try {
+            // get ws connection from session user props
+            conn = (WebSocketConnection) session.getUserProperties().get(WSConstants.WS_CONNECTION);
+            // if we don't get it from the session, try the scope lookup
+            if (conn == null) {
+                log.warn("Connection for id: {} was not found in the session onClose", sessionId);
+                conn = scope.getConnectionBySessionId(sessionId);
+            }
+            if (conn != null) {
+                // close the ws conn which removes it from the scope
+                if (conn.isConnected()) {
+                    conn.close();
+                }
+            } else {
+                log.debug("Connection for id: {} was not found in the scope or session: {}", sessionId, scope.getPath());
+            }
+        } catch (Exception e) {
+            if (conn != null) {
+                // force remove on exception
+                scope.removeConnection(conn);
+                // fire close, to be sure
                 conn.close();
             }
-        } else {
-            log.debug("Connection for id: {} was not found in the scope or session: {}", sessionId, scope.getPath());
         }
     }
 

diff --git a/service/pom.xml b/service/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <groupId>org.red5</groupId>
         <artifactId>red5-parent</artifactId>
-        <version>1.3.0</version>
+        <version>1.3.1</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
     <artifactId>red5-service</artifactId>