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/05/26 06:47:27 UTC
[tomcat] branch 8.5.x updated: Fix BZ 66076. Non-blocking flush with TLS+NIO must flush network buffer
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new bd6cf00963 Fix BZ 66076. Non-blocking flush with TLS+NIO must flush network buffer
bd6cf00963 is described below
commit bd6cf009630f3ff49056d246413765c2c05cc372
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon May 23 10:17:02 2022 +0100
Fix BZ 66076. Non-blocking flush with TLS+NIO must flush network buffer
https://bz.apache.org/bugzilla/show_bug.cgi?id=66076
---
java/org/apache/tomcat/util/net/NioEndpoint.java | 43 ++++++++++++++++------
.../apache/tomcat/util/net/SocketWrapperBase.java | 23 +++++++++++-
webapps/docs/changelog.xml | 5 +++
3 files changed, 59 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index ab9b2552f5..a6eaac0b3a 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1245,20 +1245,20 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
@Override
protected boolean flushNonBlocking() throws IOException {
- boolean dataLeft = !socketBufferHandler.isWriteBufferEmpty();
+ boolean dataLeft = socketOrNetworkBufferHasDataLeft();
// Write to the socket, if there is anything to write
if (dataLeft) {
doWrite(false);
- dataLeft = !socketBufferHandler.isWriteBufferEmpty();
+ dataLeft = socketOrNetworkBufferHasDataLeft();
}
if (!dataLeft && !nonBlockingWriteBuffer.isEmpty()) {
dataLeft = nonBlockingWriteBuffer.write(this, false);
- if (!dataLeft && !socketBufferHandler.isWriteBufferEmpty()) {
+ if (!dataLeft && socketOrNetworkBufferHasDataLeft()) {
doWrite(false);
- dataLeft = !socketBufferHandler.isWriteBufferEmpty();
+ dataLeft = socketOrNetworkBufferHasDataLeft();
}
}
@@ -1266,6 +1266,22 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
}
+ /*
+ * https://bz.apache.org/bugzilla/show_bug.cgi?id=66076
+ *
+ * When using TLS an additional buffer is used for the encrypted data
+ * before it is written to the network. It is possible for this network
+ * output buffer to contain data while the socket write buffer is empty.
+ *
+ * For NIO with non-blocking I/O, this case is handling by ensuring that
+ * flush only returns false (i.e. no data left to flush) if all buffers
+ * are empty.
+ */
+ private boolean socketOrNetworkBufferHasDataLeft() {
+ return !socketBufferHandler.isWriteBufferEmpty() || getSocket().getOutboundRemaining() > 0;
+ }
+
+
@Override
protected void doWrite(boolean block, ByteBuffer buffer) throws IOException {
int n = 0;
@@ -1510,6 +1526,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
return inline;
}
+ @Override
+ protected boolean hasOutboundRemaining() {
+ return getSocket().getOutboundRemaining() > 0;
+ }
+
@Override
public void run() {
// Perform the IO operation
@@ -1542,13 +1563,13 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
} else {
boolean doWrite = true;
// Write from main buffer first
- if (!socketBufferHandler.isWriteBufferEmpty()) {
+ if (socketOrNetworkBufferHasDataLeft()) {
// There is still data inside the main write buffer, it needs to be written first
socketBufferHandler.configureWriteBufferForRead();
do {
nBytes = getSocket().write(socketBufferHandler.getWriteBuffer());
- } while (!socketBufferHandler.isWriteBufferEmpty() && nBytes > 0);
- if (!socketBufferHandler.isWriteBufferEmpty()) {
+ } while (socketOrNetworkBufferHasDataLeft() && nBytes > 0);
+ if (socketOrNetworkBufferHasDataLeft()) {
doWrite = false;
}
// Preserve a negative value since it is an error
@@ -1569,7 +1590,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
updateLastWrite();
}
}
- if (nBytes != 0 || !buffersArrayHasRemaining(buffers, offset, length)) {
+ if (nBytes != 0 || (!buffersArrayHasRemaining(buffers, offset, length) &&
+ !socketOrNetworkBufferHasDataLeft())) {
completionDone = false;
}
}
@@ -1577,7 +1599,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
setError(e);
}
}
- if (nBytes > 0 || (nBytes == 0 && !buffersArrayHasRemaining(buffers, offset, length))) {
+ if (nBytes > 0 || (nBytes == 0 && !buffersArrayHasRemaining(buffers, offset, length) &&
+ !socketOrNetworkBufferHasDataLeft())) {
// The bytes processed are only updated in the completion handler
completion.completed(Long.valueOf(nBytes), this);
} else if (nBytes < 0 || getError() != null) {
@@ -1596,9 +1619,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
}
}
}
-
}
-
}
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 4a3d489a9a..abfbcef2f2 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -722,6 +722,12 @@ public abstract class SocketWrapperBase<E> {
}
+ /**
+ * Writes all remaining data from the buffers and blocks until the write is
+ * complete.
+ *
+ * @throws IOException If an IO error occurs during the write
+ */
protected void flushBlocking() throws IOException {
doWrite(true);
@@ -736,6 +742,14 @@ public abstract class SocketWrapperBase<E> {
}
+ /**
+ * Writes as much data as possible from any that remains in the buffers.
+ *
+ * @return <code>true</code> if data remains to be flushed after this method
+ * completes, otherwise <code>false</code>.
+ *
+ * @throws IOException If an IO error occurs during the write
+ */
protected abstract boolean flushNonBlocking() throws IOException;
@@ -1002,6 +1016,13 @@ public abstract class SocketWrapperBase<E> {
*/
protected abstract boolean isInline();
+ protected boolean hasOutboundRemaining() {
+ // NIO2 and APR never have remaining outbound data when the
+ // completion handler is called. NIO needs to override this.
+ return false;
+ }
+
+
/**
* Process the operation using the connector executor.
* @return true if the operation was accepted, false if the executor
@@ -1053,7 +1074,7 @@ public abstract class SocketWrapperBase<E> {
boolean completion = true;
if (state.check != null) {
CompletionHandlerCall call = state.check.callHandler(currentState, state.buffers, state.offset, state.length);
- if (call == CompletionHandlerCall.CONTINUE) {
+ if (call == CompletionHandlerCall.CONTINUE || (!state.read && state.hasOutboundRemaining())) {
complete = false;
} else if (call == CompletionHandlerCall.NONE) {
completion = false;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 13860f4258..7d430b94bb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -124,6 +124,11 @@
</subsection>
<subsection name="Coyote">
<changelog>
+ <fix>
+ <bug>66076</bug>: When using TLS with non-blocking writes and the NIO
+ connector, ensure that flushing the buffers attempts to empty all of the
+ output buffers. (markt)
+ </fix>
<fix>
<bug>66084</bug>: Correctly calculate bytes written to a response. Pull
request <pr>516</pr> provided by aooohan HanLi. (markt)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org