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 0f27398e47 Fix BZ 66023 - improve handling of HTTP upgrade with
request body
0f27398e47 is described below
commit 0f27398e47000f8b4bc5559433383f3f36535e0e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 29 11:17:39 2022 +0100
Fix BZ 66023 - improve handling of HTTP upgrade with request body
https://bz.apache.org/bugzilla/show_bug.cgi?id=66023
The previous fix (BZ 65726) made the saved request body available to the
request object but didn't update the processor. This patch extends the
previous fix to make the request and processor objects consistent. It
addresses the issue identified in 66023 and should address any
additional, related issues. If a use case has been missed, the fix
should be as simple as an update to the SavedRequestStreamInputBuffer.
---
java/org/apache/coyote/http2/Stream.java | 97 ++++++++++++++++++++--
test/org/apache/coyote/http2/Http2TestBase.java | 43 ++++++++--
.../coyote/http2/TestHttp2UpgradeHandler.java | 49 +++++++++--
webapps/docs/changelog.xml | 4 +
4 files changed, 171 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 70861146c0..72cf285c22 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -34,6 +34,7 @@ import org.apache.coyote.Request;
import org.apache.coyote.Response;
import org.apache.coyote.http11.HttpOutputBuffer;
import org.apache.coyote.http11.OutputFilter;
+import org.apache.coyote.http11.filters.SavedRequestInputFilter;
import org.apache.coyote.http2.HpackDecoder.HeaderEmitter;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -99,12 +100,13 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
if (coyoteRequest == null) {
// HTTP/2 new request
this.coyoteRequest = new Request();
- this.inputBuffer = new StreamInputBuffer();
+ this.inputBuffer = new StandardStreamInputBuffer();
this.coyoteRequest.setInputBuffer(inputBuffer);
} else {
// HTTP/2 Push or HTTP/1.1 upgrade
this.coyoteRequest = coyoteRequest;
- this.inputBuffer = null;
+ this.inputBuffer = new SavedRequestStreamInputBuffer(
+ (SavedRequestInputFilter) coyoteRequest.getInputBuffer());
// Headers have been read by this point
state.receivedStartOfHeaders();
if (HTTP_UPGRADE_STREAM.equals(identifier)) {
@@ -1024,7 +1026,27 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
- class StreamInputBuffer implements InputBuffer {
+ abstract class StreamInputBuffer implements InputBuffer {
+
+ abstract void receiveReset();
+
+ abstract void swallowUnread() throws IOException;
+
+ abstract void notifyEof();
+
+ abstract ByteBuffer getInBuffer();
+
+ abstract void onDataAvailable() throws IOException;
+
+ abstract boolean isReadyForRead();
+
+ abstract boolean isRequestBodyFullyRead();
+
+ abstract void insertReplayedBody(ByteChunk body);
+ }
+
+
+ class StandardStreamInputBuffer extends StreamInputBuffer {
/* Two buffers are required to avoid various multi-threading issues.
* These issues arise from the fact that the Stream (or the
@@ -1197,7 +1219,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
- private final ByteBuffer getInBuffer() {
+ final ByteBuffer getInBuffer() {
ensureBuffersExist();
return inBuffer;
}
@@ -1224,7 +1246,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
- private final void receiveReset() {
+ final void receiveReset() {
if (inBuffer != null) {
synchronized (inBuffer) {
resetReceived = true;
@@ -1233,7 +1255,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
}
- private final void notifyEof() {
+ final void notifyEof() {
if (inBuffer != null) {
synchronized (inBuffer) {
inBuffer.notifyAll();
@@ -1241,7 +1263,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
}
- private final void swallowUnread() throws IOException {
+ final void swallowUnread() throws IOException {
synchronized (this) {
closed = true;
}
@@ -1267,4 +1289,65 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
}
}
+
+
+ class SavedRequestStreamInputBuffer extends StreamInputBuffer {
+
+ private final SavedRequestInputFilter inputFilter;
+
+ SavedRequestStreamInputBuffer(SavedRequestInputFilter inputFilter) {
+ this.inputFilter = inputFilter;
+ }
+
+
+ @Override
+ public int doRead(ApplicationBufferHandler handler) throws IOException
{
+ return inputFilter.doRead(handler);
+ }
+
+ @Override
+ public int available() {
+ return inputFilter.available();
+ }
+
+ @Override
+ void receiveReset() {
+ // NO-OP
+ }
+
+ @Override
+ void swallowUnread() throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ void notifyEof() {
+ // NO-OP
+ }
+
+ @Override
+ ByteBuffer getInBuffer() {
+ return null;
+ }
+
+ @Override
+ void onDataAvailable() throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ boolean isReadyForRead() {
+ return true;
+ }
+
+ @Override
+ boolean isRequestBodyFullyRead() {
+ return inputFilter.isFinished();
+ }
+
+ @Override
+ void insertReplayedBody(ByteChunk body) {
+ // NO-OP
+ }
+ }
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java
b/test/org/apache/coyote/http2/Http2TestBase.java
index feadb2718c..03ac865093 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.net.Socket;
@@ -1378,20 +1379,44 @@ public abstract class Http2TestBase extends
TomcatBaseTest {
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
// Request bodies are unusal with GET but not illegal
+ doPost(req, resp);
+ }
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
long total = 0;
long read = 0;
- byte[] buffer = new byte[1024];
- try (InputStream is = req.getInputStream()) {
- while ((read = is.read(buffer)) > 0) {
- total += read;
+
+ if ("true".equals(req.getParameter("useReader"))) {
+ char[] buffer = new char[1024];
+
+ try (InputStream is = req.getInputStream();
+ InputStreamReader reader = new InputStreamReader(is,
StandardCharsets.UTF_8);) {
+ while ((read = reader.read(buffer)) > 0) {
+ total += read;
+ }
}
- }
- resp.setContentType("text/plain");
- resp.setCharacterEncoding("UTF-8");
- PrintWriter pw = resp.getWriter();
- pw.print("Total bytes read from request body [" + total + "]");
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ PrintWriter pw = resp.getWriter();
+ pw.print("Total chars read from request body [" + total + "]");
+
+ } else {
+ byte[] buffer = new byte[1024];
+ try (InputStream is = req.getInputStream()) {
+ while ((read = is.read(buffer)) > 0) {
+ total += read;
+ }
+ }
+
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ PrintWriter pw = resp.getWriter();
+ pw.print("Total bytes read from request body [" + total + "]");
+ }
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java
b/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java
index ebdb14ac02..e818205b2d 100644
--- a/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java
+++ b/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java
@@ -72,18 +72,54 @@ public class TestHttp2UpgradeHandler extends Http2TestBase {
@Test
- public void testUpgradeWithRequestBody() throws Exception {
- doTestUpgradeWithRequestBody(false);
+ public void testUpgradeWithRequestBodyGet() throws Exception {
+ doTestUpgradeWithRequestBody(false, false, false);
}
@Test
- public void testUpgradeWithRequestBodyTooBig() throws Exception {
- doTestUpgradeWithRequestBody(true);
+ public void testUpgradeWithRequestBodyGetTooBig() throws Exception {
+ doTestUpgradeWithRequestBody(false, false, true);
}
- private void doTestUpgradeWithRequestBody(boolean tooBig) throws Exception
{
+ @Test
+ public void testUpgradeWithRequestBodyPost() throws Exception {
+ doTestUpgradeWithRequestBody(true, false, false);
+ }
+
+
+ @Test
+ public void testUpgradeWithRequestBodyPostTooBig() throws Exception {
+ doTestUpgradeWithRequestBody(true, false, true);
+ }
+
+
+ @Test
+ public void testUpgradeWithRequestBodyGetReader() throws Exception {
+ doTestUpgradeWithRequestBody(false, true, false);
+ }
+
+
+ @Test
+ public void testUpgradeWithRequestBodyGetReaderTooBig() throws Exception {
+ doTestUpgradeWithRequestBody(false, true, true);
+ }
+
+
+ @Test
+ public void testUpgradeWithRequestBodyPostReader() throws Exception {
+ doTestUpgradeWithRequestBody(true, true, false);
+ }
+
+
+ @Test
+ public void testUpgradeWithRequestBodyPostReaderTooBig() throws Exception {
+ doTestUpgradeWithRequestBody(true, true, true);
+ }
+
+
+ private void doTestUpgradeWithRequestBody(boolean usePost, boolean
useReader, boolean tooBig) throws Exception {
enableHttp2();
Tomcat tomcat = getTomcatInstance();
@@ -100,7 +136,8 @@ public class TestHttp2UpgradeHandler extends Http2TestBase {
openClientConnection();
- byte[] upgradeRequest = ("GET / HTTP/1.1\r\n" +
+ byte[] upgradeRequest = ((usePost ? "POST" : "GET") +
+ " /" + (useReader ? "?useReader=true " : " ") + "HTTP/1.1\r\n"
+
"Host: localhost:" + getPort() + "\r\n" +
"Content-Length: 18\r\n" +
"Connection: Upgrade,HTTP2-Settings\r\n" +
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 08da2bd672..b14dbc6b61 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,10 @@
of the OS that uses kernel 5.10 or later. Thanks to Christopher Gual
for
the research into this issue. (markt)
</fix>
+ <fix>
+ <bug>66023</bug>: Improve the fix for <bug>65726</bug> and support HTTP
+ upgrade with a request body for a wider set of use cases. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Webapps">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
