[tomcat] 04/04: Fix BZ 64771. Support calls to SocketInputStream.isReady()

[markt]

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 5f85526a093190881b939486320b7e5b1d6849ab
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 be557a6..36f91c2 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -166,7 +166,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;
@@ -201,6 +212,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
@@ -210,13 +229,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();
+        }
     }
 
 
@@ -714,6 +762,10 @@ public final class Request {
         attributes.clear();
 
         listener = null;
+        synchronized (nonBlockingStateLock) {
+            fireListener = false;
+            registeredForRead = false;
+        }
         allDataReadEventSent.set(false);
 
         startTime = -1;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a5e2793..7385761 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