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:45:22 UTC
[tomcat] branch main 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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 4334622e08 Fix BZ 66076. Non-blocking flush with TLS+NIO must flush network buffer
4334622e08 is described below
commit 4334622e087468184eef7b520ac5d8e631be2c21
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 e8348e90f0..d074153bbc 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1295,20 +1295,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();
}
}
@@ -1316,6 +1316,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;
@@ -1556,6 +1572,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
@@ -1588,13 +1609,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
@@ -1615,7 +1636,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;
}
}
@@ -1623,7 +1645,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) {
@@ -1642,9 +1665,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 e671f632f8..4c54200b96 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -718,6 +718,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);
@@ -732,6 +738,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;
@@ -1003,6 +1017,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
@@ -1054,7 +1075,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 3c8000881d..89663f9cee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -133,6 +133,11 @@
use <code>ReentrantLock</code> to support users wishing to experiment
with project Loom. (markt)
</scode>
+ <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