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
The following commit(s) were added to refs/heads/9.0.x by this push:
new e6efb7f Fix bz 65137 Don't corrupt response on early termination
e6efb7f is described below
commit e6efb7ffe662c6cca4f3e6024d8065976e605f64
Author: Mark Thomas <[email protected]>
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 +++++++++++++++++++++-
.../tomcat/util/net/NioBlockingSelector.java | 20 +++++++++++++++++++-
.../apache/tomcat/util/net/NioSelectorPool.java | 20 +++++++++++++++++++-
.../apache/tomcat/util/net/SocketWrapperBase.java | 2 ++
webapps/docs/changelog.xml | 5 +++++
5 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index cb67e07..887a1d5 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2290,6 +2290,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 {
@@ -2326,8 +2345,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/NioBlockingSelector.java
b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
index eb8d511..67d6a53 100644
--- a/java/org/apache/tomcat/util/net/NioBlockingSelector.java
+++ b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
@@ -91,6 +91,23 @@ public class NioBlockingSelector {
reference = new KeyReference();
}
NioSocketWrapper att = (NioSocketWrapper) key.attachment();
+ if (att.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(att.previousIOException);
+ }
int written = 0;
boolean timedout = false;
int keycount = 1; //assume we can write
@@ -131,7 +148,8 @@ public class NioBlockingSelector {
}
}
if (timedout) {
- throw new SocketTimeoutException();
+ att.previousIOException = new SocketTimeoutException();
+ throw att.previousIOException;
}
} finally {
poller.remove(att, SelectionKey.OP_WRITE);
diff --git a/java/org/apache/tomcat/util/net/NioSelectorPool.java
b/java/org/apache/tomcat/util/net/NioSelectorPool.java
index ea365d9..e3d88f6 100644
--- a/java/org/apache/tomcat/util/net/NioSelectorPool.java
+++ b/java/org/apache/tomcat/util/net/NioSelectorPool.java
@@ -151,6 +151,23 @@ public class NioSelectorPool {
if (shared) {
return blockingSelector.write(buf, socket, writeTimeout);
}
+ if (socket.getSocketWrapper().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(socket.getSocketWrapper().previousIOException);
+ }
SelectionKey key = null;
int written = 0;
boolean timedout = false;
@@ -191,7 +208,8 @@ public class NioSelectorPool {
}
}
if (timedout) {
- throw new SocketTimeoutException();
+ socket.getSocketWrapper().previousIOException = new
SocketTimeoutException();
+ throw socket.getSocketWrapper().previousIOException;
}
} finally {
if (key != null) {
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 7f8020c..07580f9 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 volatile boolean upgraded = false;
private boolean secure = false;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c5dd99f..5f1ee2b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -138,6 +138,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: [email protected]
For additional commands, e-mail: [email protected]
