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

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


The following commit(s) were added to refs/heads/8.5.x by this push:
     new bebb779  Fix BZ 65776. Reduce false positives detecting duplicate 
accept bug
bebb779 is described below

commit bebb779cd8a3c7402b07f5ff12f2a6c02bd2698b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 10 16:02:05 2022 +0000

    Fix BZ 65776. Reduce false positives detecting duplicate accept bug
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
 java/org/apache/tomcat/util/net/AprEndpoint.java  | 21 +++++++++++++++------
 java/org/apache/tomcat/util/net/Nio2Endpoint.java | 17 ++++++++++++-----
 java/org/apache/tomcat/util/net/NioEndpoint.java  | 17 ++++++++++++-----
 webapps/docs/changelog.xml                        |  4 ++++
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 001f8f6..0c47367 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -57,6 +57,7 @@ import org.apache.tomcat.jni.Status;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.buf.ByteBufferUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Acceptor.AcceptorState;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.openssl.OpenSSLContext;
@@ -109,6 +110,8 @@ public class AprEndpoint extends AbstractEndpoint<Long> 
implements SNICallBack {
 
     private int previousAcceptedPort = -1;
     private String previousAcceptedAddress = null;
+    private long previouspreviousAcceptedSocketNanoTime = 0;
+
 
 
     private final Map<Long,AprSocketWrapper> connections = new 
ConcurrentHashMap<>();
@@ -127,8 +130,8 @@ public class AprEndpoint extends AbstractEndpoint<Long> 
implements SNICallBack {
         setMaxConnections(8 * 1024);
     }
 
-    // ------------------------------------------------------------- Properties
 
+    // ------------------------------------------------------------- Properties
 
     /**
      * Defer accept.
@@ -772,13 +775,19 @@ public class AprEndpoint extends AbstractEndpoint<Long> 
implements SNICallBack {
                 // Do the duplicate accept check here rather than in 
Acceptor.run()
                 // so we can cache the results in the SocketWrapper
                 AprSocketWrapper wrapper = new 
AprSocketWrapper(Long.valueOf(socket), this);
-                if (wrapper.getRemotePort() == previousAcceptedPort) {
-                    if 
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-                        throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                // Bug does not affect Windows. Skip the check on that 
platform.
+                if (!JrePlatform.IS_WINDOWS) {
+                    long currentNanoTime = System.nanoTime();
+                    if (wrapper.getRemotePort() == previousAcceptedPort) {
+                        if 
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
+                            if (currentNanoTime - 
previouspreviousAcceptedSocketNanoTime < 1000) {
+                                throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                            }
+                        }
                     }
+                    previousAcceptedPort = wrapper.getRemotePort();
+                    previousAcceptedAddress = wrapper.getRemoteAddr();
                 }
-                previousAcceptedPort = wrapper.getRemotePort();
-                previousAcceptedAddress = wrapper.getRemoteAddr();
 
                 wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
                 wrapper.setReadTimeout(getConnectionTimeout());
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java 
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 6eddfd7..30c054d 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.jsse.JSSESupport;
 
@@ -84,6 +85,7 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel> {
     private SynchronizedStack<Nio2Channel> nioChannels;
 
     private SocketAddress previousAcceptedSocketRemoteAddress = null;
+    private long previousAcceptedSocketNanoTime = 0;
 
 
     public Nio2Endpoint() {
@@ -414,12 +416,17 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel> {
                         // socket
                         socket = serverSock.accept().get();
 
-                        SocketAddress currentRemoteAddress = 
socket.getRemoteAddress();
-                        if 
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-                            throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                        // Bug does not affect Windows. Skip the check on that 
platform.
+                        if (!JrePlatform.IS_WINDOWS) {
+                            SocketAddress currentRemoteAddress = 
socket.getRemoteAddress();
+                            long currentNanoTime = System.nanoTime();
+                            if 
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+                                    currentNanoTime - 
previousAcceptedSocketNanoTime < 1000) {
+                                throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                            }
+                            previousAcceptedSocketRemoteAddress = 
currentRemoteAddress;
+                            previousAcceptedSocketNanoTime = currentNanoTime;
                         }
-                        previousAcceptedSocketRemoteAddress = 
currentRemoteAddress;
-
                     } catch (Exception e) {
                         // We didn't get a socket
                         countDownConnection();
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java 
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 0665d89..e2466de 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -52,6 +52,7 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.IntrospectionUtils;
 import org.apache.tomcat.util.collections.SynchronizedQueue;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.jsse.JSSESupport;
 
@@ -104,11 +105,11 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel> {
     private SynchronizedStack<NioChannel> nioChannels;
 
     private SocketAddress previousAcceptedSocketRemoteAddress = null;
+    private long previousAcceptedSocketNanoTime = 0;
 
 
     // ------------------------------------------------------------- Properties
 
-
     /**
      * Generic properties, introspected
      */
@@ -514,11 +515,17 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel> {
                         // socket
                         socket = serverSock.accept();
 
-                        SocketAddress currentRemoteAddress = 
socket.getRemoteAddress();
-                        if 
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-                            throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                        // Bug does not affect Windows. Skip the check on that 
platform.
+                        if (!JrePlatform.IS_WINDOWS) {
+                            SocketAddress currentRemoteAddress = 
socket.getRemoteAddress();
+                            long currentNanoTime = System.nanoTime();
+                            if 
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+                                    currentNanoTime - 
previousAcceptedSocketNanoTime < 1000) {
+                                throw new 
IOException(sm.getString("endpoint.err.duplicateAccept"));
+                            }
+                            previousAcceptedSocketRemoteAddress = 
currentRemoteAddress;
+                            previousAcceptedSocketNanoTime = currentNanoTime;
                         }
-                        previousAcceptedSocketRemoteAddress = 
currentRemoteAddress;
                     } catch (IOException ioe) {
                         // We didn't get a socket
                         countDownConnection();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9d1e4e4..093ea78 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -138,6 +138,10 @@
         ignored when the Connector used an internal executor. (markt)
       </fix>
       <fix>
+        <bug>65776</bug>: Improve the detection of the Linux duplicate accept
+        bug and reduce (hopefully avoid) instances of false positives. (markt)
+      </fix>
+      <fix>
         <bug>65848</bug>: Revert the change that attempted to align the
         behaviour of client certificate authentication with NIO or NIO2 with
         OpenSSL for TLS between MacOS and Linux/Windows as the root cause was

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

Reply via email to