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 2021/03/15 19:25:57 UTC
[tomcat] branch master updated (7621923 -> 1a7b9d8)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 7621923 Make fields final after new approach to recycling
new f4381f7 Rename Output.swallowedPadding to onSwallowedDataFramePayload
new db907f9 Rename swallow() -> swallowPayload()
new 02c202a Add frameTypeId parameter to swallowedPayload()
new 87bb2e0 Expose the actual frameTypeId for unknown frames
new 76115a2 Fix BZ 65179 - avoid flow control window exhaustion
new 1a7b9d8 Fix failing test after flow control updates
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/coyote/http2/AbstractNonZeroStream.java | 16 +++
java/org/apache/coyote/http2/FrameType.java | 5 +
java/org/apache/coyote/http2/Http2AsyncParser.java | 13 +-
java/org/apache/coyote/http2/Http2Parser.java | 149 +++++++++++++--------
.../apache/coyote/http2/Http2UpgradeHandler.java | 24 ++--
java/org/apache/coyote/http2/RecycledStream.java | 30 ++++-
java/org/apache/coyote/http2/Stream.java | 42 ++++--
test/org/apache/coyote/http2/Http2TestBase.java | 13 +-
.../apache/coyote/http2/TestCancelledUpload.java | 73 ++++------
test/org/apache/coyote/http2/TestFlowControl.java | 131 ++++++++++++++++++
webapps/docs/changelog.xml | 6 +
11 files changed, 362 insertions(+), 140 deletions(-)
create mode 100644 test/org/apache/coyote/http2/TestFlowControl.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/06: Rename swallow() -> swallowPayload()
Posted by ma...@apache.org.
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 db907f9d3cbf604754728e0b4a7e635c1f34299a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 18:01:01 2021 +0000
Rename swallow() -> swallowPayload()
---
java/org/apache/coyote/http2/Http2AsyncParser.java | 4 ++--
java/org/apache/coyote/http2/Http2Parser.java | 18 +++++++++---------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
index ed565fd..ad2ba10 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -106,7 +106,7 @@ class Http2AsyncParser extends Http2Parser {
payload.flip();
try {
if (streamException) {
- swallow(streamId, payloadSize, false, payload);
+ swallowPayload(streamId, payloadSize, false, payload);
} else {
readSettingsFrame(flags, payloadSize, payload);
}
@@ -239,7 +239,7 @@ class Http2AsyncParser extends Http2Parser {
do {
continueParsing = false;
if (streamException) {
- swallow(streamId, payloadSize, false, payload);
+ swallowPayload(streamId, payloadSize, false, payload);
} else {
switch (frameType) {
case DATA:
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 8a852bf..a90028a 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -87,7 +87,7 @@ class Http2Parser {
try {
validateFrame(expected, frameType, streamId, flags, payloadSize);
} catch (StreamException se) {
- swallow(streamId, payloadSize, false, null);
+ swallowPayload(streamId, payloadSize, false, null);
throw se;
}
@@ -171,11 +171,11 @@ class Http2Parser {
ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize, endOfStream);
if (dest == null) {
- swallow(streamId, dataLength, false, buffer);
+ swallowPayload(streamId, dataLength, false, buffer);
// Process padding before sending any notifications in case padding
// is invalid.
if (padLength > 0) {
- swallow(streamId, padLength, true, buffer);
+ swallowPayload(streamId, padLength, true, buffer);
}
if (endOfStream) {
output.receivedEndOfStream(streamId);
@@ -183,7 +183,7 @@ class Http2Parser {
} else {
synchronized (dest) {
if (dest.remaining() < dataLength) {
- swallow(streamId, dataLength, false, buffer);
+ swallowPayload(streamId, dataLength, false, buffer);
// Client has sent more data than permitted by Window size
throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
Http2Error.FLOW_CONTROL_ERROR, streamId);
@@ -199,7 +199,7 @@ class Http2Parser {
// Process padding before sending any notifications in case
// padding is invalid.
if (padLength > 0) {
- swallow(streamId, padLength, true, buffer);
+ swallowPayload(streamId, padLength, true, buffer);
}
if (endOfStream) {
output.receivedEndOfStream(streamId);
@@ -224,7 +224,7 @@ class Http2Parser {
try {
hpackDecoder.setHeaderEmitter(output.headersStart(streamId, headersEndStream));
} catch (StreamException se) {
- swallow(streamId, payloadSize, false, buffer);
+ swallowPayload(streamId, payloadSize, false, buffer);
throw se;
}
@@ -268,7 +268,7 @@ class Http2Parser {
readHeaderPayload(streamId, payloadSize, buffer);
- swallow(streamId, padLength, true, buffer);
+ swallowPayload(streamId, padLength, true, buffer);
if (Flags.isEndOfHeaders(flags)) {
onHeadersComplete(streamId);
@@ -518,7 +518,7 @@ class Http2Parser {
protected void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize, ByteBuffer buffer)
throws IOException {
try {
- swallow(streamId, payloadSize, false, buffer);
+ swallowPayload(streamId, payloadSize, false, buffer);
} catch (ConnectionException e) {
// Will never happen because swallow() is called with mustBeZero set
// to false
@@ -540,7 +540,7 @@ class Http2Parser {
* @throws ConnectionException If the swallowed bytes are expected to have a
* value of zero but do not
*/
- protected void swallow(int streamId, int len, boolean mustBeZero, ByteBuffer byteBuffer)
+ protected void swallowPayload(int streamId, int len, boolean mustBeZero, ByteBuffer byteBuffer)
throws IOException, ConnectionException {
if (log.isDebugEnabled()) {
log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 06/06: Fix failing test after flow control updates
Posted by ma...@apache.org.
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 1a7b9d8db55c974403a355b04eca563b63e4a5d6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 19:24:08 2021 +0000
Fix failing test after flow control updates
---
.../apache/coyote/http2/TestCancelledUpload.java | 73 ++++++++--------------
1 file changed, 26 insertions(+), 47 deletions(-)
diff --git a/test/org/apache/coyote/http2/TestCancelledUpload.java b/test/org/apache/coyote/http2/TestCancelledUpload.java
index ebaaec0..77c31ba 100644
--- a/test/org/apache/coyote/http2/TestCancelledUpload.java
+++ b/test/org/apache/coyote/http2/TestCancelledUpload.java
@@ -74,26 +74,6 @@ public class TestCancelledUpload extends Http2TestBase {
// - reset the stream if further DATA frames are received
parser.readFrame(true);
- // If reset is first frame received end test here
- if (checkReset()) {
- return;
- }
-
- // Validate any WindowSize frames. Usually arrive in pairs. Depending on
- // timing, can see a reset rather than than stream update.
- while (output.getTrace().startsWith("0-WindowSize-[")) {
- String trace = output.getTrace();
- int size = Integer.parseInt(trace.substring(14, trace.length() - 2));
- output.clearTrace();
- parser.readFrame(true);
- if (checkReset()) {
- return;
- }
- Assert.assertEquals("3-WindowSize-[" + size + "]\n", output.getTrace());
- output.clearTrace();
- parser.readFrame(true);
- }
-
// Check for reset and exit if found
if (checkReset()) {
return;
@@ -107,58 +87,57 @@ public class TestCancelledUpload extends Http2TestBase {
"3-HeadersEnd\n",
output.getTrace());
output.clearTrace();
-
parser.readFrame(true);
+
// Check for reset and exit if found
if (checkReset()) {
return;
}
- // Not reset, must be request body
+ // Not window update, not reset, must be the response body
Assert.assertEquals("3-Body-0\n" +
"3-EndOfStream\n",
output.getTrace());
output.clearTrace();
-
- // There must be a reset. There may be some WindowSize frames
parser.readFrame(true);
- // Validate any WindowSize frames. Usually arrive in pairs. Depending on
- // timing, can see a reset rather than than stream update.
- while (output.getTrace().startsWith("0-WindowSize-[")) {
- String trace = output.getTrace();
- int size = Integer.parseInt(trace.substring(14, trace.length() - 2));
- output.clearTrace();
- parser.readFrame(true);
- if (checkReset()) {
- return;
- }
- Assert.assertEquals("3-WindowSize-[" + size + "]\n", output.getTrace());
- output.clearTrace();
- parser.readFrame(true);
- }
-
- // This should be the reset
- checkReset();
- Assert.assertEquals("3-RST-[3]\n", output.getTrace());
+ Assert.assertTrue(checkReset());
// If there are any more frames after this, ignore them
}
/*
- * Depending on timing, several resets may be sent.
+ * Looking for a RST frame with error type 3 (flow control error).
+ *
+ * If there is a flow control window update for stream 0 it may be followed
+ * by one for stream 3.
+ *
+ * If there is a flow control window update for stream 3 it will always be
+ * preceded by one for stream 0.
*/
private boolean checkReset() throws IOException, Http2Exception {
+ int lastConnectionFlowControlWindowUpdate = -1;
while (true) {
- if (output.getTrace().startsWith("3-RST-[3]\n")) {
+ String trace = output.getTrace();
+ if (trace.startsWith("3-RST-[3]\n")) {
+ // This is the reset we are looking for
return true;
- } else if (output.getTrace().startsWith("3-RST-[")) {
- output.clearTrace();
- parser.readFrame(true);
+ } else if (trace.startsWith("3-RST-[")) {
+ // Probably error type 8 (cancel) - ignore
+ } else if (trace.startsWith("0-WindowSize-[")) {
+ // Connection flow control window update
+ lastConnectionFlowControlWindowUpdate = Integer.parseInt(trace.substring(14, trace.length() - 2));
+ } else if (trace.startsWith("3-WindowSize-[")) {
+ // Stream flow control window update
+ // Must be same size as last connection flow control window
+ // update. True for Tomcat anyway.
+ Assert.assertEquals("3-WindowSize-[" + lastConnectionFlowControlWindowUpdate + "]\n", trace);
} else {
return false;
}
+ output.clearTrace();
+ parser.readFrame(true);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/06: Rename Output.swallowedPadding to
onSwallowedDataFramePayload
Posted by ma...@apache.org.
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 f4381f77289554dd01b97d39999c8cac08da1832
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Mar 12 09:55:38 2021 +0000
Rename Output.swallowedPadding to onSwallowedDataFramePayload
This change is in preparation for fixes to BZ 65179
---
java/org/apache/coyote/http2/Http2Parser.java | 17 +++++++++++++++--
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 4 ++--
test/org/apache/coyote/http2/Http2TestBase.java | 6 +++---
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 6b2369b..8a852bf 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -208,7 +208,7 @@ class Http2Parser {
}
}
if (Flags.hasPadding(flags)) {
- output.swallowedPadding(streamId, padLength);
+ output.onSwallowedDataFramePayload(streamId, padLength);
}
}
@@ -731,7 +731,20 @@ class Http2Parser {
ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, boolean endOfStream) throws Http2Exception;
void endRequestBodyFrame(int streamId) throws Http2Exception;
void receivedEndOfStream(int streamId) throws ConnectionException;
- void swallowedPadding(int streamId, int paddingLength) throws ConnectionException, IOException;
+ /**
+ * Notification triggered when the parser swallows some or all of a DATA
+ * frame payload without writing it to the ByteBuffer returned by
+ * {@link #startRequestBodyFrame(int, int, boolean)}.
+ *
+ * @param streamId The stream on which the payload that has been
+ * swallowed was received
+ * @param swallowedDataBytesCount The number of bytes that the parser
+ * swallowed.
+ *
+ * @throws ConnectionException
+ * @throws IOException
+ */
+ void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws ConnectionException, IOException;
// Header frames
HeaderEmitter headersStart(int streamId, boolean headersEndStream)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index c02d3b0..f39c3c1 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1503,11 +1503,11 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
- public void swallowedPadding(int streamId, int paddingLength) throws
+ public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws
ConnectionException, IOException {
AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
// +1 is for the payload byte used to define the padding length
- writeWindowUpdate(abstractNonZeroStream, paddingLength + 1, false);
+ writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount + 1, false);
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index e15c444..ea1a9ac 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1185,10 +1185,10 @@ public abstract class Http2TestBase extends TomcatBaseTest {
@Override
- public void swallowedPadding(int streamId, int paddingLength) {
+ public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) {
trace.append(streamId);
- trace.append("-SwallowedPadding-[");
- trace.append(paddingLength);
+ trace.append("-SwallowedDataFramePayload-[");
+ trace.append(swallowedDataBytesCount);
trace.append("]\n");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/06: Add frameTypeId parameter to swallowedPayload()
Posted by ma...@apache.org.
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 02c202aae1789ac532e63894a9317bcda7f1fcec
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 18:50:29 2021 +0000
Add frameTypeId parameter to swallowedPayload()
---
java/org/apache/coyote/http2/FrameType.java | 5 +++
java/org/apache/coyote/http2/Http2AsyncParser.java | 11 ++++---
java/org/apache/coyote/http2/Http2Parser.java | 38 ++++++++++++----------
3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/java/org/apache/coyote/http2/FrameType.java b/java/org/apache/coyote/http2/FrameType.java
index 0f17f6f..8734944 100644
--- a/java/org/apache/coyote/http2/FrameType.java
+++ b/java/org/apache/coyote/http2/FrameType.java
@@ -53,6 +53,11 @@ enum FrameType {
}
+ int getId() {
+ return id;
+ }
+
+
byte getIdByte() {
return (byte) id;
}
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
index ad2ba10..754cc96 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -106,7 +106,7 @@ class Http2AsyncParser extends Http2Parser {
payload.flip();
try {
if (streamException) {
- swallowPayload(streamId, payloadSize, false, payload);
+ swallowPayload(streamId, frameTypeId, payloadSize, false, payload);
} else {
readSettingsFrame(flags, payloadSize, payload);
}
@@ -170,6 +170,7 @@ class Http2AsyncParser extends Http2Parser {
private boolean validated = false;
private CompletionState state = null;
protected int payloadSize;
+ protected int frameTypeId;
protected FrameType frameType;
protected int flags;
protected int streamId;
@@ -202,7 +203,8 @@ class Http2AsyncParser extends Http2Parser {
}
parsedFrameHeader = true;
payloadSize = ByteUtil.getThreeBytes(frameHeaderBuffer, 0);
- frameType = FrameType.valueOf(ByteUtil.getOneByte(frameHeaderBuffer, 3));
+ frameTypeId = ByteUtil.getOneByte(frameHeaderBuffer, 3);
+ frameType = FrameType.valueOf(frameTypeId);
flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
}
@@ -239,7 +241,7 @@ class Http2AsyncParser extends Http2Parser {
do {
continueParsing = false;
if (streamException) {
- swallowPayload(streamId, payloadSize, false, payload);
+ swallowPayload(streamId, frameTypeId, payloadSize, false, payload);
} else {
switch (frameType) {
case DATA:
@@ -280,7 +282,8 @@ class Http2AsyncParser extends Http2Parser {
if (payload.remaining() >= 9) {
int position = payload.position();
payloadSize = ByteUtil.getThreeBytes(payload, position);
- frameType = FrameType.valueOf(ByteUtil.getOneByte(payload, position + 3));
+ frameTypeId = ByteUtil.getOneByte(payload, position + 3);
+ frameType = FrameType.valueOf(frameTypeId);
flags = ByteUtil.getOneByte(payload, position + 4);
streamId = ByteUtil.get31Bits(payload, position + 5);
streamException = false;
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index a90028a..b1c8e15 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -80,14 +80,15 @@ class Http2Parser {
}
int payloadSize = ByteUtil.getThreeBytes(frameHeaderBuffer, 0);
- FrameType frameType = FrameType.valueOf(ByteUtil.getOneByte(frameHeaderBuffer, 3));
+ int frameTypeId = ByteUtil.getOneByte(frameHeaderBuffer, 3);
+ FrameType frameType = FrameType.valueOf(frameTypeId);
int flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
int streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
try {
validateFrame(expected, frameType, streamId, flags, payloadSize);
} catch (StreamException se) {
- swallowPayload(streamId, payloadSize, false, null);
+ swallowPayload(streamId, frameTypeId, payloadSize, false, null);
throw se;
}
@@ -171,11 +172,11 @@ class Http2Parser {
ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize, endOfStream);
if (dest == null) {
- swallowPayload(streamId, dataLength, false, buffer);
+ swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false, buffer);
// Process padding before sending any notifications in case padding
// is invalid.
if (padLength > 0) {
- swallowPayload(streamId, padLength, true, buffer);
+ swallowPayload(streamId, FrameType.DATA.getId(), padLength, true, buffer);
}
if (endOfStream) {
output.receivedEndOfStream(streamId);
@@ -183,7 +184,7 @@ class Http2Parser {
} else {
synchronized (dest) {
if (dest.remaining() < dataLength) {
- swallowPayload(streamId, dataLength, false, buffer);
+ swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false, buffer);
// Client has sent more data than permitted by Window size
throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
Http2Error.FLOW_CONTROL_ERROR, streamId);
@@ -199,7 +200,7 @@ class Http2Parser {
// Process padding before sending any notifications in case
// padding is invalid.
if (padLength > 0) {
- swallowPayload(streamId, padLength, true, buffer);
+ swallowPayload(streamId, FrameType.DATA.getId(), padLength, true, buffer);
}
if (endOfStream) {
output.receivedEndOfStream(streamId);
@@ -224,7 +225,7 @@ class Http2Parser {
try {
hpackDecoder.setHeaderEmitter(output.headersStart(streamId, headersEndStream));
} catch (StreamException se) {
- swallowPayload(streamId, payloadSize, false, buffer);
+ swallowPayload(streamId, FrameType.HEADERS.getId(), payloadSize, false, buffer);
throw se;
}
@@ -268,7 +269,7 @@ class Http2Parser {
readHeaderPayload(streamId, payloadSize, buffer);
- swallowPayload(streamId, padLength, true, buffer);
+ swallowPayload(streamId, FrameType.HEADERS.getId(), padLength, true, buffer);
if (Flags.isEndOfHeaders(flags)) {
onHeadersComplete(streamId);
@@ -518,7 +519,7 @@ class Http2Parser {
protected void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize, ByteBuffer buffer)
throws IOException {
try {
- swallowPayload(streamId, payloadSize, false, buffer);
+ swallowPayload(streamId, frameType.getId(), payloadSize, false, buffer);
} catch (ConnectionException e) {
// Will never happen because swallow() is called with mustBeZero set
// to false
@@ -528,19 +529,22 @@ class Http2Parser {
/**
- * Swallow bytes.
+ * Swallow some or all of the bytes from the payload of an HTTP/2 frame.
*
- * @param streamId Stream being swallowed
- * @param len Number of bytes to swallow
- * @param mustBeZero Are the bytes required to have value zero
- * @param byteBuffer Unused for non-async parser
+ * @param streamId Stream being swallowed
+ * @param frameTypeId Type of HTTP/2 frame for which the bytes will be
+ * swallowed
+ * @param len Number of bytes to swallow
+ * @param isPadding Are the bytes to be swallowed padding bytes?
+ * @param byteBuffer Used with {@link Http2AsyncParser} to access the
+ * data that has already been read
*
* @throws IOException If an I/O error occurs reading additional bytes into
* the input buffer.
* @throws ConnectionException If the swallowed bytes are expected to have a
* value of zero but do not
*/
- protected void swallowPayload(int streamId, int len, boolean mustBeZero, ByteBuffer byteBuffer)
+ protected void swallowPayload(int streamId, int frameTypeId, int len, boolean isPadding, ByteBuffer byteBuffer)
throws IOException, ConnectionException {
if (log.isDebugEnabled()) {
log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
@@ -549,7 +553,7 @@ class Http2Parser {
if (len == 0) {
return;
}
- if (!mustBeZero && byteBuffer != null) {
+ if (!isPadding && byteBuffer != null) {
byteBuffer.position(byteBuffer.position() + len);
} else {
int read = 0;
@@ -561,7 +565,7 @@ class Http2Parser {
} else {
byteBuffer.get(buffer, 0, thisTime);
}
- if (mustBeZero) {
+ if (isPadding) {
// Validate the padding is zero since receiving non-zero padding
// is a strong indication of either a faulty client or a server
// side bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 05/06: Fix BZ 65179 - avoid flow control window exhaustion
Posted by ma...@apache.org.
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 76115a2a8681e5951aef9037120fa3babeffd9d3
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 17:57:41 2021 +0000
Fix BZ 65179 - avoid flow control window exhaustion
Ensure that the connection level flow control window from the client to
the server is updated when handling DATA frames received for completed
streams else the flow control window may become exhausted
https://bz.apache.org/bugzilla/show_bug.cgi?id=65179
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 16 +++
java/org/apache/coyote/http2/Http2Parser.java | 82 +++++++------
.../apache/coyote/http2/Http2UpgradeHandler.java | 22 ++--
java/org/apache/coyote/http2/RecycledStream.java | 30 ++++-
java/org/apache/coyote/http2/Stream.java | 42 +++++--
test/org/apache/coyote/http2/Http2TestBase.java | 7 +-
test/org/apache/coyote/http2/TestFlowControl.java | 131 +++++++++++++++++++++
webapps/docs/changelog.xml | 6 +
8 files changed, 271 insertions(+), 65 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 38761ad..0368c4f 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -16,6 +16,7 @@
*/
package org.apache.coyote.http2;
+import java.nio.ByteBuffer;
import java.util.Iterator;
import org.apache.juli.logging.Log;
@@ -31,6 +32,8 @@ abstract class AbstractNonZeroStream extends AbstractStream {
private static final Log log = LogFactory.getLog(AbstractNonZeroStream.class);
private static final StringManager sm = StringManager.getManager(AbstractNonZeroStream.class);
+ protected static final ByteBuffer ZERO_LENGTH_BYTEBUFFER = ByteBuffer.allocate(0);
+
protected final StreamStateMachine state;
private volatile int weight = Constants.DEFAULT_WEIGHT;
@@ -140,4 +143,17 @@ abstract class AbstractNonZeroStream extends AbstractStream {
final void checkState(FrameType frameType) throws Http2Exception {
state.checkFrameType(frameType);
}
+
+
+ /**
+ * Obtain the ByteBuffer to store DATA frame payload data for this stream
+ * that has been received from the client.
+ *
+ * @return {@code null} if the DATA frame payload can be swallowed, or a
+ * ByteBuffer with at least enough space remaining for the current
+ * flow control window for stream data from the client.
+ */
+ abstract ByteBuffer getInputByteBuffer();
+
+ abstract void receivedData(int payloadSize) throws Http2Exception;
}
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 7e3a86d..0895668 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -175,7 +175,7 @@ class Http2Parser {
swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false, buffer);
// Process padding before sending any notifications in case padding
// is invalid.
- if (padLength > 0) {
+ if (Flags.hasPadding(flags)) {
swallowPayload(streamId, FrameType.DATA.getId(), padLength, true, buffer);
}
if (endOfStream) {
@@ -183,9 +183,12 @@ class Http2Parser {
}
} else {
synchronized (dest) {
- if (dest.remaining() < dataLength) {
- swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false, buffer);
+ if (dest.remaining() < payloadSize) {
// Client has sent more data than permitted by Window size
+ swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false, buffer);
+ if (Flags.hasPadding(flags)) {
+ swallowPayload(streamId, FrameType.DATA.getId(), padLength, true, buffer);
+ }
throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
Http2Error.FLOW_CONTROL_ERROR, streamId);
}
@@ -199,7 +202,7 @@ class Http2Parser {
}
// Process padding before sending any notifications in case
// padding is invalid.
- if (padLength > 0) {
+ if (Flags.hasPadding(flags)) {
swallowPayload(streamId, FrameType.DATA.getId(), padLength, true, buffer);
}
if (endOfStream) {
@@ -208,9 +211,6 @@ class Http2Parser {
output.endRequestBodyFrame(streamId);
}
}
- if (Flags.hasPadding(flags)) {
- output.onSwallowedDataFramePayload(streamId, padLength);
- }
}
@@ -521,10 +521,11 @@ class Http2Parser {
try {
swallowPayload(streamId, frameTypeId, payloadSize, false, buffer);
} catch (ConnectionException e) {
- // Will never happen because swallow() is called with mustBeZero set
+ // Will never happen because swallowPayload() is called with isPadding set
// to false
+ } finally {
+ output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
}
- output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
}
@@ -550,33 +551,46 @@ class Http2Parser {
log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
Integer.toString(streamId), Integer.toString(len)));
}
- if (len == 0) {
- return;
- }
- if (!isPadding && byteBuffer != null) {
- byteBuffer.position(byteBuffer.position() + len);
- } else {
- int read = 0;
- byte[] buffer = new byte[1024];
- while (read < len) {
- int thisTime = Math.min(buffer.length, len - read);
- if (byteBuffer == null) {
- input.fill(true, buffer, 0, thisTime);
- } else {
- byteBuffer.get(buffer, 0, thisTime);
- }
- if (isPadding) {
- // Validate the padding is zero since receiving non-zero padding
- // is a strong indication of either a faulty client or a server
- // side bug.
- for (int i = 0; i < thisTime; i++) {
- if (buffer[i] != 0) {
- throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding",
- connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
+ try {
+ if (len == 0) {
+ return;
+ }
+ if (!isPadding && byteBuffer != null) {
+ byteBuffer.position(byteBuffer.position() + len);
+ } else {
+ int read = 0;
+ byte[] buffer = new byte[1024];
+ while (read < len) {
+ int thisTime = Math.min(buffer.length, len - read);
+ if (byteBuffer == null) {
+ input.fill(true, buffer, 0, thisTime);
+ } else {
+ byteBuffer.get(buffer, 0, thisTime);
+ }
+ if (isPadding) {
+ // Validate the padding is zero since receiving non-zero padding
+ // is a strong indication of either a faulty client or a server
+ // side bug.
+ for (int i = 0; i < thisTime; i++) {
+ if (buffer[i] != 0) {
+ throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding",
+ connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
+ }
}
}
+ read += thisTime;
+ }
+ }
+ } finally {
+ if (FrameType.DATA.getIdByte() == frameTypeId) {
+ if (isPadding) {
+ // Need to add 1 for the padding length bytes that was also
+ // part of the payload.
+ len += 1;
+ }
+ if (len > 0) {
+ output.onSwallowedDataFramePayload(streamId, len);
}
- read += thisTime;
}
}
}
@@ -733,7 +747,7 @@ class Http2Parser {
// Data frames
ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, boolean endOfStream) throws Http2Exception;
- void endRequestBodyFrame(int streamId) throws Http2Exception;
+ void endRequestBodyFrame(int streamId) throws Http2Exception, IOException;
void receivedEndOfStream(int streamId) throws ConnectionException;
/**
* Notification triggered when the parser swallows some or all of a DATA
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 1a38f4d..641b258 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1467,20 +1467,14 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
}
AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
- if (abstractNonZeroStream instanceof Stream) {
- Stream stream = (Stream) abstractNonZeroStream;
- stream.checkState(FrameType.DATA);
- stream.receivedData(payloadSize);
- return stream.getInputByteBuffer();
- } else {
- abstractNonZeroStream.checkState(FrameType.DATA);
- return null;
- }
+ abstractNonZeroStream.checkState(FrameType.DATA);
+ abstractNonZeroStream.receivedData(payloadSize);
+ return abstractNonZeroStream.getInputByteBuffer();
}
@Override
- public void endRequestBodyFrame(int streamId) throws Http2Exception {
+ public void endRequestBodyFrame(int streamId) throws Http2Exception, IOException {
AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
if (abstractNonZeroStream instanceof Stream) {
((Stream) abstractNonZeroStream).getInputBuffer().onDataAvailable();
@@ -1503,11 +1497,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
- public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws
- ConnectionException, IOException {
- AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
- // +1 is for the payload byte used to define the padding length
- writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount + 1, false);
+ public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws IOException {
+ AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId);
+ writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount, false);
}
diff --git a/java/org/apache/coyote/http2/RecycledStream.java b/java/org/apache/coyote/http2/RecycledStream.java
index b2b3e32..730936b 100644
--- a/java/org/apache/coyote/http2/RecycledStream.java
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -16,6 +16,8 @@
*/
package org.apache.coyote.http2;
+import java.nio.ByteBuffer;
+
/**
* Represents a closed stream in the priority tree. Used in preference to the
* full {@link Stream} as has much lower memory usage.
@@ -23,10 +25,12 @@ package org.apache.coyote.http2;
class RecycledStream extends AbstractNonZeroStream {
private final String connectionId;
+ private int remainingFlowControlWindow;
- RecycledStream(String connectionId, Integer identifier, StreamStateMachine state) {
+ RecycledStream(String connectionId, Integer identifier, StreamStateMachine state, int remainingFlowControlWindow) {
super(identifier, state);
this.connectionId = connectionId;
+ this.remainingFlowControlWindow = remainingFlowControlWindow;
}
@@ -41,5 +45,27 @@ class RecycledStream extends AbstractNonZeroStream {
void incrementWindowSize(int increment) throws Http2Exception {
// NO-OP
}
-}
+
+ @Override
+ void receivedData(int payloadSize) throws ConnectionException {
+ remainingFlowControlWindow -= payloadSize;
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * This implementation will return an zero length ByteBuffer to trigger a
+ * flow control error if more DATA frame payload than the remaining flow
+ * control window is received for this recycled stream.
+ */
+ @Override
+ ByteBuffer getInputByteBuffer() {
+ if (remainingFlowControlWindow < 0) {
+ return ZERO_LENGTH_BYTEBUFFER;
+ } else {
+ return null;
+ }
+ }
+}
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 7656002..5929cda 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -517,9 +517,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
+ @Override
final ByteBuffer getInputByteBuffer() {
if (inputBuffer == null) {
- return null;
+ // This must either be a push or an HTTP upgrade. Either way there
+ // should not be a request body so return a zero length ByteBuffer
+ // to trigger a flow control error.
+ return ZERO_LENGTH_BYTEBUFFER;
}
return inputBuffer.getInBuffer();
}
@@ -549,7 +553,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
- final void receivedData(int payloadSize) throws ConnectionException {
+ @Override
+ final void receivedData(int payloadSize) throws Http2Exception {
contentLengthReceived += payloadSize;
long contentLengthHeader = coyoteRequest.getContentLengthLong();
if (contentLengthHeader > -1 && contentLengthReceived > contentLengthHeader) {
@@ -650,6 +655,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
state.sendReset();
cancelAllocationRequests();
handler.sendStreamReset(se);
+ inputBuffer.swallowUnread();
} catch (IOException ioe) {
ConnectionException ce = new ConnectionException(
sm.getString("stream.reset.fail"), Http2Error.PROTOCOL_ERROR, ioe);
@@ -674,7 +680,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
if (log.isDebugEnabled()) {
log.debug(sm.getString("stream.recycle", getConnectionId(), getIdAsString()));
}
- handler.replaceStream(this, new RecycledStream(getConnectionId(), getIdentifier(), state));
+ handler.replaceStream(
+ this, new RecycledStream(getConnectionId(), getIdentifier(), state, getInputByteBuffer().remaining()));
}
@@ -1003,7 +1010,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
// 'write mode'.
private volatile ByteBuffer inBuffer;
private volatile boolean readInterest;
- private boolean resetReceived = false;
+ private volatile boolean closed;
+ private boolean resetReceived;
@Override
public final int doRead(ApplicationBufferHandler applicationBufferHandler)
@@ -1113,8 +1121,10 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
/*
* Called after placing some data in the inBuffer.
*/
- final synchronized boolean onDataAvailable() {
- if (readInterest) {
+ final synchronized void onDataAvailable() throws IOException {
+ if (closed) {
+ swallowUnread();
+ } else if (readInterest) {
if (log.isDebugEnabled()) {
log.debug(sm.getString("stream.inputBuffer.dispatch"));
}
@@ -1124,7 +1134,6 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
// the incoming connection and streams are processed on their
// own.
coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
- return true;
} else {
if (log.isDebugEnabled()) {
log.debug(sm.getString("stream.inputBuffer.signal"));
@@ -1132,7 +1141,6 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
synchronized (inBuffer) {
inBuffer.notifyAll();
}
- return false;
}
}
@@ -1149,13 +1157,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
private final void ensureBuffersExist() {
- if (inBuffer == null) {
+ if (inBuffer == null && !closed) {
// The client must obey Tomcat's window size when sending so
// this is the initial window size set by Tomcat that the client
// uses (i.e. the local setting is required here).
int size = handler.getLocalSettings().getInitialWindowSize();
synchronized (this) {
- if (inBuffer == null) {
+ if (inBuffer == null && !closed) {
inBuffer = ByteBuffer.allocate(size);
outBuffer = new byte[size];
}
@@ -1180,5 +1188,19 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
}
}
+
+ private final void swallowUnread() throws IOException {
+ if (inBuffer != null) {
+ synchronized (inputBuffer) {
+ closed = true;
+ int unreadByteCount = inBuffer.position();
+ if (unreadByteCount > 0) {
+ inBuffer.position(0);
+ inBuffer.limit(inBuffer.limit() - unreadByteCount);
+ handler.onSwallowedDataFramePayload(getIdAsInt(), unreadByteCount);
+ }
+ }
+ }
+ }
}
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index 012064b..3a92861 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1186,10 +1186,9 @@ public abstract class Http2TestBase extends TomcatBaseTest {
@Override
public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) {
- trace.append(streamId);
- trace.append("-SwallowedDataFramePayload-[");
- trace.append(swallowedDataBytesCount);
- trace.append("]\n");
+ // NO-OP
+ // Many tests swallow request bodies which triggers this
+ // notification. It is added to the trace to reduce noise.
}
diff --git a/test/org/apache/coyote/http2/TestFlowControl.java b/test/org/apache/coyote/http2/TestFlowControl.java
new file mode 100644
index 0000000..3841a1d
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestFlowControl.java
@@ -0,0 +1,131 @@
+/*
+ * 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.coyote.http2;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.http.MimeHeaders;
+
+public class TestFlowControl extends Http2TestBase {
+
+ /*
+ * https://tomcat.markmail.org/thread/lijsebphms7hr3zj
+ */
+ @Test
+ public void testNotFound() throws Exception {
+
+ http2Connect();
+
+ // Connection and per-stream flow control default to 64k-1 bytes
+
+ // Set up a POST request to a non-existent end-point
+ // Generate headers
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ MimeHeaders headers = new MimeHeaders();
+ headers.addValue(":method").setString("POST");
+ headers.addValue(":scheme").setString("http");
+ headers.addValue(":path").setString("/path-does-not-exist");
+ headers.addValue(":authority").setString("localhost:" + getPort());
+ headers.addValue("content-length").setLong(65536);
+ hpackEncoder.encode(headers, headersPayload);
+
+ headersPayload.flip();
+
+ ByteUtil.setThreeBytes(headersFrameHeader, 0, headersPayload.limit());
+ headersFrameHeader[3] = FrameType.HEADERS.getIdByte();
+ // Flags. end of headers (0x04)
+ headersFrameHeader[4] = 0x04;
+ // Stream id
+ ByteUtil.set31Bits(headersFrameHeader, 5, 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ // Generate body
+ // Max data payload is 16k
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload = ByteBuffer.allocate(16 * 1024);
+
+ while (dataPayload.hasRemaining()) {
+ dataPayload.put((byte) 'x');
+ }
+ dataPayload.flip();
+
+ // Size
+ ByteUtil.setThreeBytes(dataFrameHeader, 0, dataPayload.limit());
+ ByteUtil.set31Bits(dataFrameHeader, 5, 3);
+
+ // Read the 404 error page
+ // headers
+ parser.readFrame(true);
+ // body
+ parser.readFrame(true);
+ // reset (becasue the request body was not fully read)
+ parser.readFrame(true);
+ // Validate response
+ Assert.assertEquals(
+ "3-HeadersStart\n" +
+ "3-Header-[:status]-[404]\n" +
+ "3-Header-[content-type]-[text/html;charset=utf-8]\n" +
+ "3-Header-[content-language]-[en]\n" +
+ "3-Header-[content-length]-[692]\n" +
+ "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
+ "3-HeadersEnd\n" +
+ "3-Body-692\n" +
+ "3-EndOfStream\n" +
+ "3-RST-[8]\n", output.getTrace());
+ output.clearTrace();
+
+ // Write 3*16k=48k of request body
+ int count = 0;
+ while (count < 3) {
+ writeFrame(dataFrameHeader, dataPayload);
+ waitForWindowSize(0);
+ dataPayload.position(0);
+ count++;
+ }
+
+ // EOS
+ dataFrameHeader[4] = 0x01;
+ writeFrame(dataFrameHeader, dataPayload);
+ waitForWindowSize(0);
+ }
+
+
+ /*
+ * This might be unnecessary but given the potential for timing differences
+ * across different systems a more robust approach seems prudent.
+ */
+ private void waitForWindowSize(int streamId) throws Http2Exception, IOException {
+ String prefix = streamId + "-WindowSize-";
+ boolean found = false;
+ String trace;
+ do {
+ parser.readFrame(true);
+ trace = output.getTrace();
+ output.clearTrace();
+ System.out.println(trace);
+ found = trace.startsWith(prefix);
+ } while (!found);
+ }
+}
+
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8135161..05aff5e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,12 @@
Simplify the closing on an HTTP/2 stream when an error condition is
present. (markt)
</scode>
+ <fix>
+ <bug>65179</bug>: Ensure that the connection level flow control window
+ from the client to the server is updated when handling DATA frames
+ received for completed streams else the flow control window may become
+ exhausted. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/06: Expose the actual frameTypeId for unknown frames
Posted by ma...@apache.org.
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 87bb2e0f0c2d97d4be050702c06aff45eaf9bee6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 18:57:37 2021 +0000
Expose the actual frameTypeId for unknown frames
---
java/org/apache/coyote/http2/Http2AsyncParser.java | 2 +-
java/org/apache/coyote/http2/Http2Parser.java | 24 ++++++++++++++++------
.../apache/coyote/http2/Http2UpgradeHandler.java | 2 +-
test/org/apache/coyote/http2/Http2TestBase.java | 4 ++--
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
index 754cc96..f1b1f2d 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -275,7 +275,7 @@ class Http2AsyncParser extends Http2Parser {
readContinuationFrame(streamId, flags, payloadSize, payload);
break;
case UNKNOWN:
- readUnknownFrame(streamId, frameType, flags, payloadSize, payload);
+ readUnknownFrame(streamId, frameTypeId, flags, payloadSize, payload);
}
}
// See if there is a new 9 byte header and continue parsing if possible
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index b1c8e15..7e3a86d 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -124,7 +124,7 @@ class Http2Parser {
readContinuationFrame(streamId, flags, payloadSize, null);
break;
case UNKNOWN:
- readUnknownFrame(streamId, frameType, flags, payloadSize, null);
+ readUnknownFrame(streamId, frameTypeId, flags, payloadSize, null);
}
return true;
@@ -516,15 +516,15 @@ class Http2Parser {
}
- protected void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize, ByteBuffer buffer)
+ protected void readUnknownFrame(int streamId, int frameTypeId, int flags, int payloadSize, ByteBuffer buffer)
throws IOException {
try {
- swallowPayload(streamId, frameType.getId(), payloadSize, false, buffer);
+ swallowPayload(streamId, frameTypeId, payloadSize, false, buffer);
} catch (ConnectionException e) {
// Will never happen because swallow() is called with mustBeZero set
// to false
}
- output.swallowed(streamId, frameType, flags, payloadSize);
+ output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
}
@@ -776,7 +776,19 @@ class Http2Parser {
// Window size
void incrementWindowSize(int streamId, int increment) throws Http2Exception;
- // Testing
- void swallowed(int streamId, FrameType frameType, int flags, int size) throws IOException;
+ /**
+ * Notification triggered when the parser swallows the payload of an
+ * unknown frame.
+ *
+ * @param streamId The stream on which the swallowed frame was
+ * received
+ * @param frameTypeId The (unrecognised) type of swallowed frame
+ * @param flags The flags set in the header of the swallowed
+ * frame
+ * @param size The payload size of the swallowed frame
+ *
+ * @throws IOException
+ */
+ void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) throws IOException;
}
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index f39c3c1..1a38f4d 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1768,7 +1768,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
- public void swallowed(int streamId, FrameType frameType, int flags, int size)
+ public void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size)
throws IOException {
// NO-OP.
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index ea1a9ac..012064b 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1172,10 +1172,10 @@ public abstract class Http2TestBase extends TomcatBaseTest {
@Override
- public void swallowed(int streamId, FrameType frameType, int flags, int size) {
+ public void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) {
trace.append(streamId);
trace.append(",");
- trace.append(frameType);
+ trace.append(frameTypeId);
trace.append(",");
trace.append(flags);
trace.append(",");
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org