Fix setting identifier truncation in SettingsFrame.serialize_body - #167
Merged
Kriechi merged 2 commits intoApr 10, 2026
Merged
Conversation
The bitmask `setting & 0xFF` truncates the 16-bit setting identifier to only 8 bits. This silently corrupts any setting ID above 255 during serialization. The identifier field in SETTINGS frames is 16 bits wide (RFC 9113, Section 6.5.1), and `_STRUCT_HL` already uses the `H` format (unsigned short, 16 bits) for it. However, `& 0xFF` discards the upper byte. For the standard settings (0x01 through 0x08) this has no visible effect since they all fit in 8 bits. But RFC 8701 GREASE values like 0x0a0a or 0x1a1a, as well as any future extension settings > 0xFF, would be silently mangled on a serialize round-trip. Change the mask to `& 0xFFFF` to match the actual field width.
Member
|
@bysiber thanks for your contribution! I added some changes to improve the code comments and added new tests to cover the correct masking behaviour. Please review them before I merge this! Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SettingsFrame.serialize_bodymasks the setting identifier with& 0xFF, which truncates it to 8 bits. The identifier field in HTTP/2 SETTINGS frames is 16 bits wide (RFC 9113, Section 6.5.1), and_STRUCT_HLalready packs it as an unsigned short (H, 16 bits).Standard settings (0x01–0x08) fit in a single byte, so this has gone unnoticed. But any setting identifier above 255 — for example GREASE values from RFC 8701 (
0x0a0a,0x1a1a, …) or future extensions — gets silently corrupted:parse_bodyalready reads the full 16-bit identifier correctly (_STRUCT_HL.unpack), so this also breaks the serialize→parse round-trip for those values.This changes the mask from
0xFFto0xFFFF.