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
commit b557e7adaf4362c8de70e7e30e0daeac3afe64f4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Mar 4 14:28:34 2020 +0000
Fix BZ 64192. Data corruption with HTTP/2 + asyncIO + TLS
https://bz.apache.org/bugzilla/show_bug.cgi?id=64192
Correctly handle the case when data needs to be returned to the read
buffer when the read buffer is non-empty.
---
.../tomcat/util/net/SocketBufferHandler.java | 50 ++++++++
.../apache/tomcat/util/net/SocketWrapperBase.java | 3 +-
.../tomcat/util/net/TestSocketBufferHandler.java | 139 +++++++++++++++++++++
webapps/docs/changelog.xml | 4 +
4 files changed, 194 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/SocketBufferHandler.java
b/java/org/apache/tomcat/util/net/SocketBufferHandler.java
index 89fa86e..d066240 100644
--- a/java/org/apache/tomcat/util/net/SocketBufferHandler.java
+++ b/java/org/apache/tomcat/util/net/SocketBufferHandler.java
@@ -16,6 +16,7 @@
*/
package org.apache.tomcat.util.net;
+import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import org.apache.tomcat.util.buf.ByteBufferUtils;
@@ -93,6 +94,55 @@ public class SocketBufferHandler {
}
+ public void unReadReadBuffer(ByteBuffer returnedData) {
+ if (isReadBufferEmpty()) {
+ configureReadBufferForWrite();
+ readBuffer.put(returnedData);
+ } else {
+ int bytesReturned = returnedData.remaining();
+ if (readBufferConfiguredForWrite) {
+ // Writes always start at position zero
+ if ((readBuffer.position() + bytesReturned) >
readBuffer.capacity()) {
+ throw new BufferOverflowException();
+ } else {
+ // Move the bytes up to make space for the returned data
+ for (int i = 0; i < readBuffer.position(); i++) {
+ readBuffer.put(i + bytesReturned, readBuffer.get(i));
+ }
+ // Insert the bytes returned
+ for (int i = 0; i < bytesReturned; i++) {
+ readBuffer.put(i, returnedData.get());
+ }
+ // Update the position
+ readBuffer.position(readBuffer.position() + bytesReturned);
+ }
+ } else {
+ // Reads will start at zero but may have progressed
+ int shiftRequired = bytesReturned - readBuffer.position();
+ if (shiftRequired > 0) {
+ if ((readBuffer.capacity() - readBuffer.limit()) <
shiftRequired) {
+ throw new BufferOverflowException();
+ }
+ // Move the bytes up to make space for the returned data
+ int oldLimit = readBuffer.limit();
+ readBuffer.limit(oldLimit + shiftRequired);
+ for (int i = readBuffer.position(); i < oldLimit; i++) {
+ readBuffer.put(i + shiftRequired, readBuffer.get(i));
+ }
+ } else {
+ shiftRequired = 0;
+ }
+ // Insert the returned bytes
+ int insertOffset = readBuffer.position() + shiftRequired -
bytesReturned;
+ for (int i = insertOffset; i < bytesReturned + insertOffset;
i++) {
+ readBuffer.put(i, returnedData.get());
+ }
+ readBuffer.position(insertOffset);
+ }
+ }
+ }
+
+
public void configureWriteBufferForWrite() {
setWriteBufferConfiguredForWrite(true);
}
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 0658ea6..a5c69c2 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -374,8 +374,7 @@ public abstract class SocketWrapperBase<E> {
*/
public void unRead(ByteBuffer returnedInput) {
if (returnedInput != null) {
- socketBufferHandler.configureReadBufferForWrite();
- socketBufferHandler.getReadBuffer().put(returnedInput);
+ socketBufferHandler.unReadReadBuffer(returnedInput);
}
}
diff --git a/test/org/apache/tomcat/util/net/TestSocketBufferHandler.java
b/test/org/apache/tomcat/util/net/TestSocketBufferHandler.java
new file mode 100644
index 0000000..5e979a8
--- /dev/null
+++ b/test/org/apache/tomcat/util/net/TestSocketBufferHandler.java
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.net;
+
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+
+@RunWith(Parameterized.class)
+public class TestSocketBufferHandler {
+
+ @Parameterized.Parameters(name = "{index}: direct[{0}]")
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<>();
+
+ parameterSets.add(new Object[] { Boolean.FALSE });
+ parameterSets.add(new Object[] { Boolean.TRUE });
+
+ return parameterSets;
+ }
+
+
+ @Parameter(0)
+ public boolean direct;
+
+
+ @Test
+ public void testReturnWhenEmpty() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+
+ validate(sbh, "WXYZ");
+ }
+
+
+ @Test
+ public void testReturnWhenWriteable() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+
+ sbh.configureReadBufferForWrite();
+ sbh.getReadBuffer().put(getBytes("AB"));
+
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+
+ validate(sbh, "WXYZAB");
+ }
+
+
+ @Test(expected = BufferOverflowException.class)
+ public void testReturnWhenWriteableAndFull() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+
+ sbh.configureReadBufferForWrite();
+ sbh.getReadBuffer().put(getBytes("ABCDEFGH"));
+
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+ }
+
+
+ @Test
+ public void testReturnWhenReadableAndUnread() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+
+ sbh.configureReadBufferForWrite();
+ sbh.getReadBuffer().put(getBytes("AB"));
+ sbh.configureReadBufferForRead();
+
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+
+ validate(sbh, "WXYZAB");
+ }
+
+
+ @Test(expected = BufferOverflowException.class)
+ public void testReturnWhenReadableAndUnreadAndFull() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+
+ sbh.configureReadBufferForWrite();
+ sbh.getReadBuffer().put(getBytes("ABCDEF"));
+ sbh.configureReadBufferForRead();
+
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+ }
+
+
+ @Test
+ public void testReturnWhenReadableAndPartiallyead() {
+ SocketBufferHandler sbh = new SocketBufferHandler(8, 8, direct);
+
+ sbh.configureReadBufferForWrite();
+ sbh.getReadBuffer().put(getBytes("ABCDEFGH"));
+ sbh.configureReadBufferForRead();
+ for (int i = 0; i < 4; i++) {
+ sbh.getReadBuffer().get();
+ }
+
+ sbh.unReadReadBuffer(ByteBuffer.wrap(getBytes("WXYZ")));
+
+ validate(sbh, "WXYZEFGH");
+ }
+
+
+ private void validate(SocketBufferHandler sbh, String expected) {
+ sbh.configureReadBufferForRead();
+ for (byte b : getBytes(expected)) {
+ Assert.assertEquals(b, sbh.getReadBuffer().get());
+ }
+ Assert.assertEquals(0, sbh.getReadBuffer().remaining());
+ }
+
+
+ private byte[] getBytes(String input) {
+ return input.getBytes(StandardCharsets.UTF_8);
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 38a525b..19feb1c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,10 @@
<code>setEnableSessionCreation</code> for <code>OpenSSLEngine</code>.
Pull request provided by Alexander Scheel. (markt)
</fix>
+ <fix>
+ <bug>64192</bug>: Correctly handle case where unread data is returned
to
+ the read buffer when the read buffer is non empty. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
