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
commit 0092ff059eac60e35ad1156e51aa111d563c1ac7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Mar 18 12:24:09 2025 +0000
Fix BZ 69614 - invalid priority field values should be ignored
---
java/org/apache/coyote/http2/Http2Parser.java | 23 ++++++++++++++------
.../apache/coyote/http2/LocalStrings.properties | 1 +
test/org/apache/coyote/http2/TestRfc9218.java | 25 ++++++++++++++++++++++
webapps/docs/changelog.xml | 4 ++++
4 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Parser.java
b/java/org/apache/coyote/http2/Http2Parser.java
index 3175637651..7c15b3ca97 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -460,15 +460,24 @@ class Http2Parser {
ByteArrayInputStream bais = new ByteArrayInputStream(payload, 4,
payloadSize - 4);
Reader r = new BufferedReader(new InputStreamReader(bais,
StandardCharsets.US_ASCII));
- Priority p = Priority.parsePriority(r);
- if (log.isTraceEnabled()) {
-
log.trace(sm.getString("http2Parser.processFramePriorityUpdate.debug",
connectionId,
- Integer.toString(prioritizedStreamID),
Integer.toString(p.getUrgency()),
- Boolean.valueOf(p.getIncremental())));
- }
+ try {
+ Priority p = Priority.parsePriority(r);
- output.priorityUpdate(prioritizedStreamID, p);
+ if (log.isTraceEnabled()) {
+
log.trace(sm.getString("http2Parser.processFramePriorityUpdate.debug",
connectionId,
+ Integer.toString(prioritizedStreamID),
Integer.toString(p.getUrgency()),
+ Boolean.valueOf(p.getIncremental())));
+ }
+
+ output.priorityUpdate(prioritizedStreamID, p);
+ } catch (IllegalArgumentException iae) {
+ // Priority frames with invalid priority field values should be
ignored
+ if (log.isTraceEnabled()) {
+
log.trace(sm.getString("http2Parser.processFramePriorityUpdate.invalid",
connectionId,
+ Integer.toString(prioritizedStreamID)), iae);
+ }
+ }
}
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index a25ead24a7..482addf4e2 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -74,6 +74,7 @@ http2Parser.processFrameHeaders.decodingDataLeft=Data left
over after HPACK deco
http2Parser.processFrameHeaders.decodingFailed=There was an error during the
HPACK decoding of HTTP headers
http2Parser.processFrameHeaders.payload=Connection [{0}], Stream [{1}],
Processing headers payload of size [{2}]
http2Parser.processFramePriorityUpdate.debug=Connection [{0}], Stream [{1}],
Urgency [{2}], Incremental [{3}]
+http2Parser.processFramePriorityUpdate.invalid=Connection [{0}], Stream [{1}],
Priority Update frame with invalid priority field value
http2Parser.processFramePriorityUpdate.streamZero=Connection [{0}], Priority
update frame received to prioritize stream zero
http2Parser.processFramePushPromise=Connection [{0}], Stream [{1}], Push
promise frames should not be sent by the client
http2Parser.processFrameSettings.ackWithNonZeroPayload=Settings frame received
with the ACK flag set and payload present
diff --git a/test/org/apache/coyote/http2/TestRfc9218.java
b/test/org/apache/coyote/http2/TestRfc9218.java
index eb9256d2a1..1a6081f88c 100644
--- a/test/org/apache/coyote/http2/TestRfc9218.java
+++ b/test/org/apache/coyote/http2/TestRfc9218.java
@@ -17,6 +17,7 @@
package org.apache.coyote.http2;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import org.junit.Assert;
import org.junit.Test;
@@ -146,6 +147,9 @@ public class TestRfc9218 extends Http2TestBase {
// 19 - 7021 body left
// 21 - 6143 body left
+ // BZ 69614 - invalid priority update frames should be ignored
+ sendInvalidPriorityUpdate(17);
+
// Re-order the priorities
sendPriorityUpdate(17, 2, true);
@@ -191,4 +195,25 @@ public class TestRfc9218 extends Http2TestBase {
ioe.printStackTrace();
}
}
+
+
+ private void sendInvalidPriorityUpdate(int streamId) throws IOException {
+ byte[] payload = "u=1:i".getBytes(StandardCharsets.US_ASCII);
+
+ byte[] priorityUpdateFrame = new byte[13 + payload.length];
+
+ // length
+ ByteUtil.setThreeBytes(priorityUpdateFrame, 0, 4 + payload.length);
+ // type
+ priorityUpdateFrame[3] = FrameType.PRIORITY_UPDATE.getIdByte();
+ // Stream ID
+ ByteUtil.set31Bits(priorityUpdateFrame, 5, 0);
+
+ // Payload
+ ByteUtil.set31Bits(priorityUpdateFrame, 9, streamId);
+ System.arraycopy(payload, 0, priorityUpdateFrame, 13, payload.length);
+
+ os.write(priorityUpdateFrame);
+ os.flush();
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2ea3887316..6ffbe84cda 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -184,6 +184,10 @@
<bug>69607</bug>: Allow failed initialization of MD5. Based on code
submitted by Shivam Verma. (remm)
</fix>
+ <fix>
+ <bug>69614</bug>: HTTP/2 priority frames with an invalid priority field
+ value should be ignored. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
