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

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


The following commit(s) were added to refs/heads/master by this push:
     new a9160a6  Fix BZ 64771. Support calls to SocketInputStream.isReady()
a9160a6 is described below

commit a9160a6cfc83adfdba18c78761ccb1b401f7b8b6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 22 21:56:44 2021 +0000

    Fix BZ 64771. Support calls to SocketInputStream.isReady()
    
    Prior to this fix, concurrent calls to isReady() could corrupt the input
    buffer. The fix is modelled on the similar changes made for
    SocketOutputStream.isReady()
---
 java/org/apache/coyote/Request.java | 60 ++++++++++++++++++++++++++++++++++---
 webapps/docs/changelog.xml          |  5 ++++
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/coyote/Request.java 
b/java/org/apache/coyote/Request.java
index 0fd62c3..9d609fd 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -167,7 +167,18 @@ public final class Request {
      */
     private Exception errorException = null;
 
+    /*
+     * State for non-blocking output is maintained here as it is the one point
+     * easily reachable from the CoyoteInputStream and the CoyoteAdapter which
+     * both need access to state.
+     */
     volatile ReadListener listener;
+    // Ensures listener is only fired after a call is isReady()
+    private boolean fireListener = false;
+    // Tracks read registration to prevent duplicate registrations
+    private boolean registeredForRead = false;
+    // Lock used to manage concurrent access to above flags
+    private final Object nonBlockingStateLock = new Object();
 
     public ReadListener getReadListener() {
         return listener;
@@ -202,6 +213,14 @@ public final class Request {
         // has been finished will register the socket for read interest and 
that
         // is not required.
         if (!isFinished() && isReady()) {
+            synchronized (nonBlockingStateLock) {
+                // Ensure we don't get multiple read registrations
+                registeredForRead = true;
+                // Need to set the fireListener flag otherwise when the
+                // container tries to trigger onDataAvailable, nothing will
+                // happen
+                fireListener = true;
+            }
             action(ActionCode.DISPATCH_READ, null);
             if (!ContainerThreadMarker.isContainerThread()) {
                 // Not on a container thread so need to execute the dispatch
@@ -211,13 +230,42 @@ public final class Request {
     }
 
     public boolean isReady() {
-        AtomicBoolean result = new AtomicBoolean();
-        action(ActionCode.NB_READ_INTEREST, result);
-        return result.get();
+        // Assume read is not possible
+        boolean ready = false;
+        synchronized (nonBlockingStateLock) {
+            if (registeredForRead) {
+                fireListener = true;
+                return false;
+            }
+            ready = checkRegisterForRead();
+            fireListener = !ready;
+        }
+        return ready;
+    }
+
+    private boolean checkRegisterForRead() {
+        AtomicBoolean ready = new AtomicBoolean(false);
+        synchronized (nonBlockingStateLock) {
+            if (!registeredForRead) {
+                action(ActionCode.NB_READ_INTEREST, ready);
+                registeredForRead = !ready.get();
+            }
+        }
+        return ready.get();
     }
 
     public void onDataAvailable() throws IOException {
-        listener.onDataAvailable();
+        boolean fire = false;
+        synchronized (nonBlockingStateLock) {
+            registeredForRead = false;
+            if (fireListener) {
+                fireListener = false;
+                fire = true;
+            }
+        }
+        if (fire) {
+            listener.onDataAvailable();
+        }
     }
 
 
@@ -728,6 +776,10 @@ public final class Request {
         attributes.clear();
 
         listener = null;
+        synchronized (nonBlockingStateLock) {
+            fireListener = false;
+            registeredForRead = false;
+        }
         allDataReadEventSent.set(false);
 
         startTimeNanos = -1;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b5b58d2..c6eaffe 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,11 @@
         present. (markt)
       </scode>
       <fix>
+        <bug>64771</bug>: Prevent concurrent calls to
+        <code>ServletInputStream.isReady()</code> corrupting the input buffer.
+        (markt)
+      </fix>
+      <fix>
         <bug>65179</bug>: Ensure that the connection level flow control window
         from the client to the server is updated when handling DATA frames
         received for completed streams else the flow control window  may become

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

Reply via email to