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 f35647a02f2c6dced285dd839a907bd9377938da Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Dec 6 19:16:41 2022 +0000 Don't process priority frames --- java/org/apache/coyote/http2/Http2Parser.java | 39 +++++++--------------- .../apache/coyote/http2/Http2UpgradeHandler.java | 31 ++--------------- .../apache/coyote/http2/LocalStrings.properties | 1 - .../apache/coyote/http2/LocalStrings_fr.properties | 1 - .../apache/coyote/http2/LocalStrings_ja.properties | 1 - .../apache/coyote/http2/LocalStrings_ko.properties | 1 - .../coyote/http2/LocalStrings_zh_CN.properties | 1 - test/org/apache/coyote/http2/Http2TestBase.java | 13 ++++---- 8 files changed, 20 insertions(+), 68 deletions(-) diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java index 60669491d5..a68ce25dcb 100644 --- a/java/org/apache/coyote/http2/Http2Parser.java +++ b/java/org/apache/coyote/http2/Http2Parser.java @@ -254,12 +254,8 @@ class Http2Parser { Integer.toString(payloadSize)), Http2Error.PROTOCOL_ERROR); } } - if (priority) { - boolean exclusive = ByteUtil.isBit7Set(optional[optionalPos]); - int parentStreamId = ByteUtil.get31Bits(optional, optionalPos); - int weight = ByteUtil.getOneByte(optional, optionalPos + 4) + 1; - output.reprioritise(streamId, parentStreamId, exclusive, weight); - } + + // Ignore RFC 7450 priority data if present payloadSize -= optionalLen; payloadSize -= padLength; @@ -277,24 +273,15 @@ class Http2Parser { } - protected void readPriorityFrame(int streamId, ByteBuffer buffer) throws Http2Exception, IOException { - byte[] payload = new byte[5]; - if (buffer == null) { - input.fill(true, payload); - } else { - buffer.get(payload); - } - - boolean exclusive = ByteUtil.isBit7Set(payload[0]); - int parentStreamId = ByteUtil.get31Bits(payload, 0); - int weight = ByteUtil.getOneByte(payload, 4) + 1; - - if (streamId == parentStreamId) { - throw new StreamException(sm.getString("http2Parser.processFramePriority.invalidParent", - connectionId, Integer.valueOf(streamId)), Http2Error.PROTOCOL_ERROR, streamId); + protected void readPriorityFrame(int streamId, ByteBuffer buffer) throws IOException { + // RFC 7450 priority frames are ignored. Still need to treat as overhead. + try { + swallowPayload(streamId, FrameType.PRIORITY.getId(), 5, false, buffer); + } catch (ConnectionException e) { + // Will never happen because swallowPayload() is called with isPadding set + // to false } - - output.reprioritise(streamId, parentStreamId, exclusive, weight); + output.increaseOverheadCount(FrameType.PRIORITY); } @@ -780,10 +767,6 @@ class Http2Parser { void headersContinue(int payloadSize, boolean endOfHeaders); void headersEnd(int streamId) throws Http2Exception; - // Priority frames (also headers) - void reprioritise(int streamId, int parentStreamId, boolean exclusive, int weight) - throws Http2Exception; - // Reset frames void reset(int streamId, long errorCode) throws Http2Exception; @@ -815,5 +798,7 @@ class Http2Parser { * frame */ void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) throws IOException; + + void increaseOverheadCount(FrameType frameType); } } diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index c89fe44c1f..a2405994a8 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -1489,7 +1489,8 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH } - private void increaseOverheadCount(FrameType frameType) { + @Override + public void increaseOverheadCount(FrameType frameType) { // An overhead frame increases the overhead count by // overheadCountFactor. By default, this means an overhead frame // increases the overhead count by 10. A simple browser request is @@ -1708,34 +1709,6 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH } - @Override - public void reprioritise(int streamId, int parentStreamId, - boolean exclusive, int weight) throws Http2Exception { - if (streamId == parentStreamId) { - throw new ConnectionException(sm.getString("upgradeHandler.dependency.invalid", - getConnectionId(), Integer.valueOf(streamId)), Http2Error.PROTOCOL_ERROR); - } - - increaseOverheadCount(FrameType.PRIORITY); - - synchronized (priorityTreeLock) { - // Need to look up stream and parent stream inside the lock else it - // is possible for a stream to be recycled before it is - // reprioritised. This can result in incorrect references to the - // non-recycled stream being retained after reprioritisation. - AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId); - if (abstractNonZeroStream == null) { - abstractNonZeroStream = createRemoteStream(streamId); - } - AbstractStream parentStream = getAbstractNonZeroStream(parentStreamId); - if (parentStream == null) { - parentStream = this; - } - abstractNonZeroStream.rePrioritise(parentStream, exclusive, weight); - } - } - - @Override public void headersContinue(int payloadSize, boolean endOfHeaders) { // Generally, continuation frames don't impact the overhead count but if diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties index 7f7bf3a8c6..547a1880d1 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -129,7 +129,6 @@ upgradeHandler.allocate.debug=Connection [{0}], Stream [{1}], allocated [{2}] by upgradeHandler.allocate.left=Connection [{0}], Stream [{1}], [{2}] bytes unallocated - trying to allocate to children upgradeHandler.allocate.recipient=Connection [{0}], Stream [{1}], potential recipient [{2}] with weight [{3}] upgradeHandler.connectionError=Connection error -upgradeHandler.dependency.invalid=Connection [{0}], Stream [{1}], Streams may not depend on themselves upgradeHandler.fallToDebug=\n\ \ Note: further occurrences of HTTP/2 stream errors will be logged at DEBUG level. upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}] diff --git a/java/org/apache/coyote/http2/LocalStrings_fr.properties b/java/org/apache/coyote/http2/LocalStrings_fr.properties index 292e61e67a..e8551e4370 100644 --- a/java/org/apache/coyote/http2/LocalStrings_fr.properties +++ b/java/org/apache/coyote/http2/LocalStrings_fr.properties @@ -128,7 +128,6 @@ upgradeHandler.allocate.debug=Connection [{0}], Flux [{1}], [{2}] octets alloué upgradeHandler.allocate.left=Connection [{0}], Flux [{1}], [{2}] octets désalloués, essai d''allocation aux enfants upgradeHandler.allocate.recipient=Connection [{0}], Flux [{1}], receveur potentiel [{2}] avec poids [{3}] upgradeHandler.connectionError=Erreur de la connection -upgradeHandler.dependency.invalid=Connection [{0}], Flux [{1}], Un flux ne peut dépendre de lui-même upgradeHandler.fallToDebug=\n\ \ Note: les occurrences suivantes d'erreurs de stream HTTP/2 seront enregistrées au niveau DEBUG. upgradeHandler.goaway.debug=Connection [{0}], Goaway, Dernier flux [{1}], Code d''erreur [{2}], Données de débogage [{3}] diff --git a/java/org/apache/coyote/http2/LocalStrings_ja.properties b/java/org/apache/coyote/http2/LocalStrings_ja.properties index 726b3fc7ba..b0e237d260 100644 --- a/java/org/apache/coyote/http2/LocalStrings_ja.properties +++ b/java/org/apache/coyote/http2/LocalStrings_ja.properties @@ -128,7 +128,6 @@ upgradeHandler.allocate.debug=コネクション [{0}]、ストリーム [{1}] upgradeHandler.allocate.left=コネクション [{0}]、ストリーム [{1}]、[{2}] バイトが未割り当て - 子への割り当てを試みています upgradeHandler.allocate.recipient=コネクション [{0}]、ストリーム [{1}]、重み [{3}] の潜在的な受信者 [{2}] upgradeHandler.connectionError=接続エラー -upgradeHandler.dependency.invalid=コネクション [{0}]、ストリーム [{1}]、ストリームは自分自身に依存するべきではありません。 upgradeHandler.fallToDebug=注: HTTP/2 ストリームのエラーがさらに発生すると、DEBUG レベルでログに記録されます。 upgradeHandler.goaway.debug=コネクション [{0}]、Goaway、最終ストリーム [{1}]、エラーコード [{2}]、デバッグデータ [{3}] upgradeHandler.init=コネクション[{0}]、状態[{1}] diff --git a/java/org/apache/coyote/http2/LocalStrings_ko.properties b/java/org/apache/coyote/http2/LocalStrings_ko.properties index 7fe71cdccc..e8b1825382 100644 --- a/java/org/apache/coyote/http2/LocalStrings_ko.properties +++ b/java/org/apache/coyote/http2/LocalStrings_ko.properties @@ -127,7 +127,6 @@ upgradeHandler.allocate.debug=연결 [{0}], 스트림 [{1}], [{2}] 바이트를 upgradeHandler.allocate.left=연결 [{0}], 스트림 [{1}], [{2}] 바이트들이 할당 해제되었습니다 - 자식들에 할당하려 시도합니다. upgradeHandler.allocate.recipient=연결 [{0}], 스트림 [{1}], 가중치 [{3}]의 잠재적 수신자 [{2}] upgradeHandler.connectionError=연결 오류 -upgradeHandler.dependency.invalid=연결 [{0}], 스트림 [{1}], 스트림들은 자기 자신들에 의존해서는 안됩니다. upgradeHandler.goaway.debug=연결 [{0}], Goaway, 마지막 스트림 [{1}], 오류 코드 [{2}], 디버그 데이터 [{3}] upgradeHandler.init=연결 [{0}], 상태 [{1}] upgradeHandler.invalidPreface=연결 [{0}]: 유효하지 않은 연결 preface diff --git a/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties b/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties index e457333ff2..d0713e931a 100644 --- a/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties +++ b/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties @@ -128,7 +128,6 @@ upgradeHandler.allocate.debug=连接[{0}],流[{1}],已分配[{2}]字节 upgradeHandler.allocate.left=连接[{0}],流[{1}],[{2}]字节未分配 - 尝试分配给子项 upgradeHandler.allocate.recipient=(:连接[{0}],流[{1}],潜在接收者[{2}],权重为[{3}] upgradeHandler.connectionError=连接错误 -upgradeHandler.dependency.invalid=连接{0},流{1},流可能不依赖于自身 upgradeHandler.goaway.debug=连接[{0}],离开,最后的流[{1}],错误码[{2}],调试数据[{3}] upgradeHandler.init=连接[{0}],状态[{1}] upgradeHandler.invalidPreface=连接[{0}],连接前言无效 diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java index 301450011c..24d85fdc1e 100644 --- a/test/org/apache/coyote/http2/Http2TestBase.java +++ b/test/org/apache/coyote/http2/Http2TestBase.java @@ -1129,13 +1129,6 @@ public abstract class Http2TestBase extends TomcatBaseTest { return this; } - @Override - public void reprioritise(int streamId, int parentStreamId, boolean exclusive, int weight) { - lastStreamId = Integer.toString(streamId); - trace.append(lastStreamId + "-Reprioritise-[" + parentStreamId + "]-[" + exclusive + - "]-[" + weight + "]\n"); - } - @Override public void emitHeader(String name, String value) { @@ -1288,6 +1281,12 @@ public abstract class Http2TestBase extends TomcatBaseTest { public long getBytesRead() { return bytesRead; } + + + @Override + public void increaseOverheadCount(FrameType frameType) { + // NO-OP. Client doesn't track overhead. + } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org