You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/02/10 16:34:59 UTC
[tomcat] branch main updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 113be8f Fix BZ 65776. Reduce false positives detecting duplicate accept bug
113be8f is described below
commit 113be8f7e133af77a81d381769bacc0584e29eb8
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/Nio2Endpoint.java | 18 +++++++++++++-----
java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +++++++++++++-----
webapps/docs/changelog.xml | 4 ++++
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 687665b..11bd099 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.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,9 +86,10 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
private SynchronizedStack<Nio2Channel> nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+ private long previouspreviousAcceptedSocketNanoTime = 0;
- // --------------------------------------------------------- Public Methods
+ // --------------------------------------------------------- Public Methods
/**
* Number of keep-alive sockets.
@@ -362,11 +364,17 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
protected AsynchronousSocketChannel serverSocketAccept() throws Exception {
AsynchronousSocketChannel result = serverSock.accept().get();
- SocketAddress currentRemoteAddress = result.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 = result.getRemoteAddress();
+ long currentNanoTime = System.nanoTime();
+ if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+ currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
+ throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+ }
+ previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+ previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
- previousAcceptedSocketRemoteAddress = currentRemoteAddress;
return result;
}
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index d5a79cd..4baab31 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -58,6 +58,7 @@ import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedQueue;
import org.apache.tomcat.util.collections.SynchronizedStack;
import org.apache.tomcat.util.compat.JreCompat;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -109,11 +110,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
private SynchronizedStack<NioChannel> nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+ private long previouspreviousAcceptedSocketNanoTime = 0;
// ------------------------------------------------------------- Properties
-
/**
* Use System.inheritableChannel to obtain channel from stdin/stdout.
*/
@@ -516,11 +517,18 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
@Override
protected SocketChannel serverSocketAccept() throws Exception {
SocketChannel result = serverSock.accept();
- SocketAddress currentRemoteAddress = result.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 = result.getRemoteAddress();
+ long currentNanoTime = System.nanoTime();
+ if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+ currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
+ throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
+ }
+ previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+ previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
- previousAcceptedSocketRemoteAddress = currentRemoteAddress;
return result;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 00b3af8..280f03b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
Posted by Mark Thomas <ma...@apache.org>.
I know, I know. I missed the "!" in the OS test.
I'm just re-chcking the performance data before I fix it.
Mark
On 10/02/2022 16:34, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new 113be8f Fix BZ 65776. Reduce false positives detecting duplicate accept bug
> 113be8f is described below
>
> commit 113be8f7e133af77a81d381769bacc0584e29eb8
> 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/Nio2Endpoint.java | 18 +++++++++++++-----
> java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +++++++++++++-----
> webapps/docs/changelog.xml | 4 ++++
> 3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> index 687665b..11bd099 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.Acceptor.AcceptorState;
> import org.apache.tomcat.util.net.jsse.JSSESupport;
> @@ -85,9 +86,10 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
> private SynchronizedStack<Nio2Channel> nioChannels;
>
> private SocketAddress previousAcceptedSocketRemoteAddress = null;
> + private long previouspreviousAcceptedSocketNanoTime = 0;
>
> - // --------------------------------------------------------- Public Methods
>
> + // --------------------------------------------------------- Public Methods
>
> /**
> * Number of keep-alive sockets.
> @@ -362,11 +364,17 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
> protected AsynchronousSocketChannel serverSocketAccept() throws Exception {
> AsynchronousSocketChannel result = serverSock.accept().get();
>
> - SocketAddress currentRemoteAddress = result.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 = result.getRemoteAddress();
> + long currentNanoTime = System.nanoTime();
> + if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
> + currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
> + throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
> + }
> + previousAcceptedSocketRemoteAddress = currentRemoteAddress;
> + previouspreviousAcceptedSocketNanoTime = currentNanoTime;
> }
> - previousAcceptedSocketRemoteAddress = currentRemoteAddress;
>
> return result;
> }
> diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
> index d5a79cd..4baab31 100644
> --- a/java/org/apache/tomcat/util/net/NioEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
> @@ -58,6 +58,7 @@ import org.apache.tomcat.util.ExceptionUtils;
> import org.apache.tomcat.util.collections.SynchronizedQueue;
> import org.apache.tomcat.util.collections.SynchronizedStack;
> import org.apache.tomcat.util.compat.JreCompat;
> +import org.apache.tomcat.util.compat.JrePlatform;
> import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
> import org.apache.tomcat.util.net.Acceptor.AcceptorState;
> import org.apache.tomcat.util.net.jsse.JSSESupport;
> @@ -109,11 +110,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
> private SynchronizedStack<NioChannel> nioChannels;
>
> private SocketAddress previousAcceptedSocketRemoteAddress = null;
> + private long previouspreviousAcceptedSocketNanoTime = 0;
>
>
> // ------------------------------------------------------------- Properties
>
> -
> /**
> * Use System.inheritableChannel to obtain channel from stdin/stdout.
> */
> @@ -516,11 +517,18 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
> @Override
> protected SocketChannel serverSocketAccept() throws Exception {
> SocketChannel result = serverSock.accept();
> - SocketAddress currentRemoteAddress = result.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 = result.getRemoteAddress();
> + long currentNanoTime = System.nanoTime();
> + if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
> + currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) {
> + throw new IOException(sm.getString("endpoint.err.duplicateAccept"));
> + }
> + previousAcceptedSocketRemoteAddress = currentRemoteAddress;
> + previouspreviousAcceptedSocketNanoTime = currentNanoTime;
> }
> - previousAcceptedSocketRemoteAddress = currentRemoteAddress;
>
> return result;
> }
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 00b3af8..280f03b 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -143,6 +143,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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org