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 2023/01/20 19:04:54 UTC
[tomcat] 05/12: Don't process priority frames
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org