You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2026/02/19 12:07:41 UTC
[PR] HTTP/2: fix frame header parsing and validate SETTINGS ACK length [httpcomponents-core]
arturobernalg opened a new pull request, #628:
URL: https://github.com/apache/httpcomponents-core/pull/628
Parse the 24-bit Length field as an unsigned value, treat flags as an unsigned byte, and mask out the reserved bit from the Stream Identifier.
Also enforce RFC 9113 ยง6.5: SETTINGS frames with ACK set MUST have an empty payload (Length = 0); otherwise raise FRAME_SIZE_ERROR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] HTTP/2: fix frame header parsing and validate SETTINGS ACK length [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #628:
URL: https://github.com/apache/httpcomponents-core/pull/628#discussion_r2829004571
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/io/FrameInputBuffer.java:
##########
@@ -103,11 +104,17 @@ public RawFrame read(final InputStream inStream) throws IOException {
final int payloadLen = (buffer[off] & 0xff) << 16 | (buffer[off + 1] & 0xff) << 8 | (buffer[off + 2] & 0xff);
final int type = buffer[off + 3] & 0xff;
final int flags = buffer[off + 4] & 0xff;
- final int streamId = Math.abs(buffer[off + 5] & 0xff) << 24 | (buffer[off + 6] & 0xff << 16) | (buffer[off + 7] & 0xff) << 8 | (buffer[off + 8] & 0xff);
+ // HTTP/2 stream id is 31-bit; ignore the reserved (MSB) bit.
+ // Note: mask before shift; `b & 0xff << 16` would drop the byte due to operator precedence.
+ final int streamId = ((buffer[off + 5] & 0x7f) << 24) | ((buffer[off + 6] & 0xff) << 16) | ((buffer[off + 7] & 0xff) << 8) | (buffer[off + 8] & 0xff);
if (payloadLen > maxFramePayloadSize) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Frame size exceeds maximum");
}
+ if (type == FrameType.SETTINGS.getValue() && (flags & FrameFlag.ACK.getValue()) != 0 && payloadLen != 0) {
Review Comment:
> @arturobernalg This is the wrong place and not the buffer's job to validate frames. Move this check to the protocol handler.
@ok2c yeah. The check of the `payloadLen > maxFramePayloadSize` make me doubt and i put it there. I just moved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] HTTP/2: fix frame header parsing and validate SETTINGS ACK length [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #628:
URL: https://github.com/apache/httpcomponents-core/pull/628#discussion_r2828630295
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/io/FrameInputBuffer.java:
##########
@@ -103,11 +104,17 @@ public RawFrame read(final InputStream inStream) throws IOException {
final int payloadLen = (buffer[off] & 0xff) << 16 | (buffer[off + 1] & 0xff) << 8 | (buffer[off + 2] & 0xff);
final int type = buffer[off + 3] & 0xff;
final int flags = buffer[off + 4] & 0xff;
- final int streamId = Math.abs(buffer[off + 5] & 0xff) << 24 | (buffer[off + 6] & 0xff << 16) | (buffer[off + 7] & 0xff) << 8 | (buffer[off + 8] & 0xff);
+ // HTTP/2 stream id is 31-bit; ignore the reserved (MSB) bit.
+ // Note: mask before shift; `b & 0xff << 16` would drop the byte due to operator precedence.
+ final int streamId = ((buffer[off + 5] & 0x7f) << 24) | ((buffer[off + 6] & 0xff) << 16) | ((buffer[off + 7] & 0xff) << 8) | (buffer[off + 8] & 0xff);
if (payloadLen > maxFramePayloadSize) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Frame size exceeds maximum");
}
+ if (type == FrameType.SETTINGS.getValue() && (flags & FrameFlag.ACK.getValue()) != 0 && payloadLen != 0) {
Review Comment:
@arturobernalg This is the wrong place and not the buffer's job to validate frames. Move this check to the protocol handler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] HTTP/2: fix frame header parsing and validate SETTINGS ACK length [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg merged PR #628:
URL: https://github.com/apache/httpcomponents-core/pull/628
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org