This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit ea99dd0edb07973ae677dd9ff90e2057aaa696e5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 5 15:14:30 2021 +0100

    Complete fix for BZ 65262. Encoders and decoders now use InstanceManager
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65262
---
 .../apache/tomcat/websocket/LocalStrings.properties  |  1 +
 .../tomcat/websocket/WsRemoteEndpointImplBase.java   | 20 ++++++++++++++++++--
 .../tomcat/websocket/server/WsServerContainer.java   | 20 +++++++++++++-------
 webapps/docs/changelog.xml                           |  6 +++---
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties 
b/java/org/apache/tomcat/websocket/LocalStrings.properties
index ae54da0..fbd7bfd 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -92,6 +92,7 @@ wsRemoteEndpoint.closed=Message will not be sent because the 
WebSocket session h
 wsRemoteEndpoint.closedDuringMessage=The remainder of the message will not be 
sent because the WebSocket session has been closed
 wsRemoteEndpoint.closedOutputStream=This method may not be called as the 
OutputStream has been closed
 wsRemoteEndpoint.closedWriter=This method may not be called as the Writer has 
been closed
+wsRemoteEndpoint.encoderDestoryFailed=Failed to destroy the encoder of type 
[{0}]
 wsRemoteEndpoint.flushOnCloseFailed=Batched messages still enabled after 
session has been closed. Unable to flush remaining batched message.
 wsRemoteEndpoint.invalidEncoder=The specified encoder of type [{0}] could not 
be instantiated
 wsRemoteEndpoint.noEncoder=No encoder specified for object of class [{0}]
diff --git a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java 
b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
index 18c723a..11f6fd4 100644
--- a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
+++ b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.websocket;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.Writer;
+import java.lang.reflect.InvocationTargetException;
 import java.net.SocketTimeoutException;
 import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
@@ -33,6 +34,7 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import javax.naming.NamingException;
 import javax.websocket.CloseReason;
 import javax.websocket.CloseReason.CloseCodes;
 import javax.websocket.DeploymentException;
@@ -45,6 +47,7 @@ import javax.websocket.SendResult;
 
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.InstanceManager;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.buf.Utf8Encoder;
 import org.apache.tomcat.util.res.StringManager;
@@ -704,10 +707,15 @@ public abstract class WsRemoteEndpointImplBase implements 
RemoteEndpoint {
         for (Class<? extends Encoder> encoderClazz :
                 endpointConfig.getEncoders()) {
             Encoder instance;
+            InstanceManager instanceManager = wsSession.getInstanceManager();
             try {
-                instance = encoderClazz.getConstructor().newInstance();
+                if (instanceManager == null) {
+                    instance = encoderClazz.getConstructor().newInstance();
+                } else {
+                    instance = (Encoder) 
instanceManager.newInstance(encoderClazz);
+                }
                 instance.init(endpointConfig);
-            } catch (ReflectiveOperationException e) {
+            } catch (ReflectiveOperationException | NamingException e) {
                 throw new DeploymentException(
                         sm.getString("wsRemoteEndpoint.invalidEncoder",
                                 encoderClazz.getName()), e);
@@ -730,8 +738,16 @@ public abstract class WsRemoteEndpointImplBase implements 
RemoteEndpoint {
 
 
     public final void close() {
+        InstanceManager instanceManager = wsSession.getInstanceManager();
         for (EncoderEntry entry : encoderEntries) {
             entry.getEncoder().destroy();
+            if (instanceManager != null) {
+                try {
+                    instanceManager.destroyInstance(entry);
+                } catch (IllegalAccessException | InvocationTargetException e) 
{
+                    
log.warn(sm.getString("wsRemoteEndpoint.encoderDestoryFailed", 
encoder.getClass()), e);
+                }
+            }
         }
         // The transformation handles both input and output. It only needs to 
be
         // closed once so it is closed here on the output side.
diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java 
b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
index c3d1415..e388157 100644
--- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
+++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
@@ -25,6 +25,7 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentSkipListMap;
 
+import javax.naming.NamingException;
 import javax.servlet.DispatcherType;
 import javax.servlet.FilterRegistration;
 import javax.servlet.ServletContext;
@@ -248,7 +249,7 @@ public class WsServerContainer extends WsWebSocketContainer
             String path = annotation.value();
 
             // Validate encoders
-            validateEncoders(annotation.encoders());
+            validateEncoders(annotation.encoders(), 
getInstanceManager(Thread.currentThread().getContextClassLoader()));
 
             // ServerEndpointConfig
             Class<? extends Configurator> configuratorClazz =
@@ -463,17 +464,22 @@ public class WsServerContainer extends 
WsWebSocketContainer
     }
 
 
-    private static void validateEncoders(Class<? extends Encoder>[] encoders)
+    private static void validateEncoders(Class<? extends Encoder>[] encoders, 
InstanceManager instanceManager)
             throws DeploymentException {
 
         for (Class<? extends Encoder> encoder : encoders) {
-            // Need to instantiate decoder to ensure it is valid and that
-            // deployment can be failed if it is not
-            @SuppressWarnings("unused")
+            // Need to instantiate encoder to ensure it is valid and that
+            // deployment can be failed if it is not. The encoder is then
+            // discarded immediately.
             Encoder instance;
             try {
-                encoder.getConstructor().newInstance();
-            } catch(ReflectiveOperationException e) {
+                if (instanceManager == null) {
+                    instance = encoder.getConstructor().newInstance();
+                } else {
+                    instance = (Encoder) instanceManager.newInstance(encoder);
+                    instanceManager.destroyInstance(instance);
+                }
+            } catch(ReflectiveOperationException | NamingException e) {
                 throw new DeploymentException(sm.getString(
                         "serverContainer.encoderFail", encoder.getName()), e);
             }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2f1aec0..781da7e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -184,9 +184,9 @@
         simplify the code. (markt)
       </scode>
       <fix>
-        <bug>65262</bug>: Refactor the creation of WebSocket end point 
instances
-        to be more IoC friendly. Instances are now created via the
-        <code>InstanceManager</code> where possible. (markt)
+        <bug>65262</bug>: Refactor the creation of WebSocket end point, decoder
+        and encoder instances to be more IoC friendly. Instances are now 
created
+        via the <code>InstanceManager</code> where possible. (markt)
       </fix>
     </changelog>
   </subsection>

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

Reply via email to