andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Permit h2 PRIORITY frames with a dependency on the connection (#10473)

Browse Source 
Motivation:

Setting a dependency on the connection is normal and permitted; streams
actually default to depending on the connection. Using a PRIORITY frame
with a dependency on the connection could reset a previous PRIORITY,
change the relative weight, or make all streams dependent on one stream.

The previous code was disallowing these usages as it considered
depending on the connection to be a validation failure.

Modifications:

Loosen validation check to also allow depending on the connection. Fix
error message when the validation check fails.

Result:

Setting a dependency on connection would be permitted. Fixes #10416
This commit is contained in:
Eric Anderson 2020-08-11 13:50:56 -07:00 committed by Norman Maurer
parent ccca21015b
commit adcffb6260
2 changed files with 35 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java
View File

@ -274,7 +274,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
int streamDependency, short weight, boolean exclusive, ChannelPromise promise) {
try {
verifyStreamId(streamId, STREAM_ID);
verifyStreamId(streamDependency, STREAM_DEPENDENCY);
verifyStreamOrConnectionId(streamDependency, STREAM_DEPENDENCY);
verifyWeight(weight);
ByteBuf buf = ctx.alloc().buffer(PRIORITY_FRAME_LENGTH);
@ -602,11 +602,11 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
}
private static void verifyStreamId(int streamId, String argumentName) {
checkPositive(streamId, "streamId");
checkPositive(streamId, argumentName);
}
private static void verifyStreamOrConnectionId(int streamId, String argumentName) {
checkPositiveOrZero(streamId, "streamId");
checkPositiveOrZero(streamId, argumentName);
}
private static void verifyWeight(short weight) {

32
codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java
View File

@ -277,6 +277,38 @@ public class DefaultHttp2FrameWriterTest {
assertEquals(expectedOutbound, outbound);
}
@Test
public void writePriority() {
frameWriter.writePriority(
ctx, /* streamId= */ 1, /* dependencyId= */ 2, /* weight= */ (short) 256, /* exclusive= */ true, promise);
expectedOutbound = Unpooled.copiedBuffer(new byte[] {
(byte) 0x00, (byte) 0x00, (byte) 0x05, // payload length = 5
(byte) 0x02, // payload type = 2
(byte) 0x00, // flags = 0x00
(byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x01, // stream id = 1
(byte) 0x80, (byte) 0x00, (byte) 0x00, (byte) 0x02, // dependency id = 2 | exclusive = 1 << 63
(byte) 0xFF, // weight = 255 (implicit +1)
});
assertEquals(expectedOutbound, outbound);
}
@Test
public void writePriorityDefaults() {
frameWriter.writePriority(
ctx, /* streamId= */ 1, /* dependencyId= */ 0, /* weight= */ (short) 16, /* exclusive= */ false, promise);
expectedOutbound = Unpooled.copiedBuffer(new byte[] {
(byte) 0x00, (byte) 0x00, (byte) 0x05, // payload length = 5
(byte) 0x02, // payload type = 2
(byte) 0x00, // flags = 0x00
(byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x01, // stream id = 1
(byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, // dependency id = 0 | exclusive = 0 << 63
(byte) 0x0F, // weight = 15 (implicit +1)
});
assertEquals(expectedOutbound, outbound);
}
private byte[] headerPayload(int streamId, Http2Headers headers, byte padding) throws Http2Exception, IOException {
if (padding == 0) {
return headerPayload(streamId, headers);