From 40ae788c2e64d018b4e58cd4210bb96434d0100d Mon Sep 17 00:00:00 2001 From: Mark Thomas Date: Tue, 18 Mar 2025 12:24:09 +0000 Subject: [PATCH] Fix BZ 69614 - invalid priority field values should be ignored Origin: https://github.com/apache/tomcat/commit/40ae788c2e64d018b4e58cd4210bb96434d0100d --- java/org/apache/coyote/http2/Http2Parser.java | 23 +++++++++++------ .../coyote/http2/LocalStrings.properties | 1 + test/org/apache/coyote/http2/TestRfc9218.java | 25 +++++++++++++++++++ 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 db7c2fd496a2..90c1142616b7 100644 --- a/java/org/apache/coyote/http2/Http2Parser.java +++ b/java/org/apache/coyote/http2/Http2Parser.java @@ -477,15 +477,24 @@ protected void readPriorityUpdateFrame(int payloadSize, ByteBuffer buffer) throw 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 6ab82e8bdb9b..114f546017d4 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -77,6 +77,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 eb9256d2a140..1a6081f88c99 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 void testPriority() throws Exception { // 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 void testPriority() throws Exception { 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(); + } }