[tomcat] branch master updated: Fix bz 65137 Don't corrupt response on early termination

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated: Fix bz 65137 Don't corrupt response on early termination

markt
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 b9b3fa7  Fix bz 65137 Don't corrupt response on early termination
b9b3fa7 is described below

commit b9b3fa78a00e76b6344ababb1fd28cf20561cb82
Author: Mark Thomas <[hidden email]>
AuthorDate: Fri Feb 19 15:33:56 2021 +0000

    Fix bz 65137 Don't corrupt response on early termination
   
    Ensure that a response is not corrupted as well as incomplete if the
    connection is closed before the response is fully written due to a write
    timeout.
   
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65137
---
 java/org/apache/tomcat/util/net/AprEndpoint.java   | 22 +++++++++++++++++++++-
 java/org/apache/tomcat/util/net/NioEndpoint.java   | 20 +++++++++++++++++++-
 .../apache/tomcat/util/net/SocketWrapperBase.java  |  2 ++
 webapps/docs/changelog.xml                         |  5 +++++
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index cf62eab..b388d04 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2364,6 +2364,25 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
 
 
         private void doWriteInternal(ByteBuffer from) throws IOException {
+
+            if (previousIOException != null) {
+                /*
+                 * Socket has previously seen an IOException on write.
+                 *
+                 * Blocking writes assume that buffer is always fully written so
+                 * there is no code checking for incomplete writes, retaining
+                 * the unwritten data and attempting to write it as part of a
+                 * subsequent write call.
+                 *
+                 * Because of the above, when an IOException is triggered we
+                 * need so skip subsequent attempts to write as otherwise it
+                 * will appear to the client as if some data was dropped just
+                 * before the connection is lost. It is better if the client
+                 * just sees the dropped connection.
+                 */
+                throw new IOException(previousIOException);
+            }
+
             int thisTime;
 
             do {
@@ -2400,8 +2419,9 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
                     // 10053 on Windows is connection aborted
                     throw new EOFException(sm.getString("socket.apr.clientAbort"));
                 } else if (thisTime < 0) {
-                    throw new IOException(sm.getString("socket.apr.write.error",
+                    previousIOException = new IOException(sm.getString("socket.apr.write.error",
                             Integer.valueOf(-thisTime), getSocket(), this));
+                    throw previousIOException;
                 }
             } while ((thisTime > 0 || getBlockingStatus()) && from.hasRemaining());
 
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 9cbf422..5d8db75 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1305,6 +1305,23 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                 throw new ClosedChannelException();
             }
             if (block) {
+                if (previousIOException != null) {
+                    /*
+                     * Socket has previously timed out.
+                     *
+                     * Blocking writes assume that buffer is always fully
+                     * written so there is no code checking for incomplete
+                     * writes, retaining the unwritten data and attempting to
+                     * write it as part of a subsequent write call.
+                     *
+                     * Because of the above, when a timeout is triggered we need
+                     * so skip subsequent attempts to write as otherwise it will
+                     * appear to the client as if some data was dropped just
+                     * before the connection is lost. It is better if the client
+                     * just sees the dropped connection.
+                     */
+                    throw new IOException(previousIOException);
+                }
                 long timeout = getWriteTimeout();
                 long startNanos = 0;
                 do {
@@ -1315,7 +1332,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                         }
                         timeout -= elapsedMillis;
                         if (timeout <= 0) {
-                            throw new SocketTimeoutException();
+                            previousIOException = new SocketTimeoutException();
+                            throw previousIOException;
                         }
                     }
                     n = getSocket().write(buffer);
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 06fea5f..0a988aa 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -51,6 +51,8 @@ public abstract class SocketWrapperBase<E> {
     private volatile long readTimeout = -1;
     private volatile long writeTimeout = -1;
 
+    protected volatile IOException previousIOException = null;
+
     private volatile int keepAliveLeft = 100;
     private String negotiatedProtocol = null;
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 653c66e..42ffdbd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -157,6 +157,11 @@
         entire request body and the server is ready the request body using
         non-blocking I/O. (markt)
       </fix>
+      <fix>
+        <bug>65137</bug>: Ensure that a response is not corrupted as well as
+        incomplete if the connection is closed before the response is fully
+        written due to a write timeout. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]