read HTTPS mandatory SvcParamKeys as 16-bit values#9585
Merged
swankjesse merged 1 commit intoJul 24, 2026
Conversation
swankjesse
approved these changes
Jul 24, 2026
swankjesse
left a comment
Collaborator
There was a problem hiding this comment.
Nice fix! Thank you.
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
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.
The new DNS reader parses SVCB and HTTPS resource records, and each SvcParam value is length-prefixed. For the mandatory parameter (key 0), RFC 9460 defines the value as a list of 2-octet SvcParamKeys, but readHttpsResourceRecord walks it one byte at a time and range-checks each byte against the supported keys. Since these records come straight off a DNS or DoH response, a resolver can send a mandatory list whose keys OkHttp doesn't support and have it slip through: a two-byte key like 0x0101 gets split into 0x01 and 0x01, both of which look like known keys, so the record is accepted when RFC 9460 says it should be treated as malformed. An odd-length value, which can't be a list of 16-bit keys at all, is likewise read without complaint. I noticed it while checking that every other 2-byte field in this reader already goes through readUShort, and this one call site still used readByte. Reading the keys as 16-bit values and requiring an even length lines the mandatory handling up with the rest of the parser and with the ipv4hint/ipv6hint length checks right next to it. Existing HTTPS records still decode unchanged, and the two new tests cover the unsupported-key and odd-length cases.