Skip to content

HTTP/2: ignore unused PADDED flag on non-padded frames #640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arturobernalg merged 1 commit into from
Feb 26, 2026
Merged

HTTP/2: ignore unused PADDED flag on non-padded frames #640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/io/FrameInputBuffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.hc.core5.http2.H2TransportMetrics;
import org.apache.hc.core5.http2.frame.FrameConsts;
import org.apache.hc.core5.http2.frame.FrameFlag;
import org.apache.hc.core5.http2.frame.FrameType;
import org.apache.hc.core5.http2.frame.RawFrame;
import org.apache.hc.core5.http2.impl.BasicH2TransportMetrics;
import org.apache.hc.core5.util.Args;
Expand Down Expand Up @@ -112,7 +113,7 @@ public RawFrame read(final InputStream inStream) throws IOException {
final int frameLen = payloadOff + payloadLen;
fillBuffer(inStream, frameLen);

if ((flags & FrameFlag.PADDED.getValue()) > 0) {
if ((flags & FrameFlag.PADDED.getValue()) > 0 && isPaddedFrameType(type)) {
if (payloadLen == 0) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Inconsistent padding");
}
Expand All @@ -137,4 +138,10 @@ public H2TransportMetrics getMetrics() {
return metrics;
}

private static boolean isPaddedFrameType(final int type) {
return FrameType.DATA.same(type)
|| FrameType.HEADERS.same(type)
|| FrameType.PUSH_PROMISE.same(type);
}

}
11 changes: 9 additions & 2 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/FrameInputBuffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.hc.core5.http2.H2TransportMetrics;
import org.apache.hc.core5.http2.frame.FrameConsts;
import org.apache.hc.core5.http2.frame.FrameFlag;
import org.apache.hc.core5.http2.frame.FrameType;
import org.apache.hc.core5.http2.frame.RawFrame;
import org.apache.hc.core5.http2.impl.BasicH2TransportMetrics;
import org.apache.hc.core5.util.Args;
Expand Down Expand Up @@ -150,12 +151,12 @@ public RawFrame read(final ByteBuffer src, final ReadableByteChannel channel) th
}
case PAYLOAD_EXPECTED:
if (buffer.remaining() >= payloadLen) {
if ((flags & FrameFlag.PADDED.getValue()) > 0) {
if ((flags & FrameFlag.PADDED.getValue()) > 0 && isPaddedFrameType(type)) {
if (payloadLen == 0) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Inconsistent padding");
}
buffer.mark();
final int padding = buffer.get();
final int padding = buffer.get() & 0xff;
if (payloadLen < padding + 1) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Inconsistent padding");
}
Expand Down Expand Up @@ -204,6 +205,12 @@ public RawFrame read(final ReadableByteChannel channel) throws IOException {
return read(null, channel);
}

private static boolean isPaddedFrameType(final int type) {
return FrameType.DATA.same(type)
|| FrameType.HEADERS.same(type)
|| FrameType.PUSH_PROMISE.same(type);
}

public void reset() {
buffer.compact();
state = State.HEAD_EXPECTED;
Expand Down
32 changes: 32 additions & 0 deletions httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/io/TestFrameInOutBuffers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.hc.core5.http.ConnectionClosedException;
import org.apache.hc.core5.http2.H2ConnectionException;
import org.apache.hc.core5.http2.H2CorruptFrameException;
import org.apache.hc.core5.http2.H2Error;
import org.apache.hc.core5.http2.frame.FrameConsts;
import org.apache.hc.core5.http2.frame.FrameFlag;
import org.apache.hc.core5.http2.frame.FrameType;
Expand Down Expand Up @@ -244,5 +245,36 @@ void testReadFrameExceedingLimit() {
Assertions.assertThrows(H2ConnectionException.class, () -> inBuffer.read(inputStream));
}

@Test
void testPaddedBitOnUnknownFrameTypeIgnored() throws Exception {
final FrameInputBuffer inBuffer = new FrameInputBuffer(16 * 1024);
final ByteArrayInputStream inputStream = new ByteArrayInputStream(
new byte[] {
0, 0, 1, 10, 8, 0, 0, 0, 1, 42 // len=1,type=0x0a(flags=0x08),stream=1,payload=42
});

final RawFrame frame = inBuffer.read(inputStream);
Assertions.assertNotNull(frame);
Assertions.assertEquals(0x0a, frame.getType());
Assertions.assertEquals(0x08, frame.getFlags());
Assertions.assertEquals(1, frame.getStreamId());
Assertions.assertEquals(1, frame.getLength());
Assertions.assertNotNull(frame.getPayload());
Assertions.assertEquals(1, frame.getPayload().remaining());
Assertions.assertEquals(42, frame.getPayload().get() & 0xff);
}

@Test
void testPaddedValidationAppliedForDataFrame() {
final FrameInputBuffer inBuffer = new FrameInputBuffer(16 * 1024);
final ByteArrayInputStream inputStream = new ByteArrayInputStream(
new byte[] {
0, 0, 1, 0, 8, 0, 0, 0, 1, 42 // len=1,type=DATA(flags=0x08),stream=1,padLength=42 -> invalid
});

final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> inBuffer.read(inputStream));
Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode());
}

}

40 changes: 40 additions & 0 deletions httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestFrameInputBuffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,44 @@ void streamIdReservedBitMustBeIgnored() throws Exception {
assertNotNull(rawFrame);
assertEquals(1, rawFrame.getStreamId());
}

@Test
void paddedBitOnUnknownFrameTypeMustBeIgnored() throws Exception {
final byte[] frame = new byte[]{
0x00, 0x00, 0x01, 0x0a, // length=1 + type=0x0a (unknown)
0x08, // PADDED bit set (undefined for this type)
0x00, 0x00, 0x00, 0x01, // streamId = 1
0x2a // payload byte
};

final FrameInputBuffer inBuf = new FrameInputBuffer(16 * 1024);
final ReadableByteChannel ch = Channels.newChannel(new ByteArrayInputStream(frame));

final RawFrame rawFrame = inBuf.read(ch);
assertNotNull(rawFrame);
assertEquals(0x0a, rawFrame.getType());
assertEquals(0x08, rawFrame.getFlags());
assertEquals(1, rawFrame.getStreamId());
assertEquals(1, rawFrame.getLength());
assertNotNull(rawFrame.getPayload());
assertEquals(1, rawFrame.getPayload().remaining());
assertEquals(0x2a, rawFrame.getPayload().get() & 0xff);
}

@Test
void paddedSemanticsStillApplyToDataFrames() throws Exception {
// DATA frame with PADDED flag and payloadLen=1, padding=0x2a -> invalid.
// This ensures the padded-validation branch is still entered for valid frame types.
final byte[] frame = new byte[]{
0x00, 0x00, 0x01, 0x00, // length=1 + type=DATA
0x08, // PADDED
0x00, 0x00, 0x00, 0x01, // streamId = 1
0x2a // pad length byte (invalid for payloadLen=1)
};

final FrameInputBuffer inBuf = new FrameInputBuffer(16 * 1024);
final ReadableByteChannel ch = Channels.newChannel(new ByteArrayInputStream(frame));
final H2ConnectionException ex = assertThrows(H2ConnectionException.class, () -> inBuf.read(ch));
assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode());
}
}
Loading