Add support for SVCB and HTTPS resource records (RFC 9460) - #222
Open
mrideout wants to merge 4 commits into
Open
Add support for SVCB and HTTPS resource records (RFC 9460)#222mrideout wants to merge 4 commits into
mrideout wants to merge 4 commits into
Conversation
Contributor
|
I did not review the code myself, but offer these AI reviews (recommending accepting the PR): https://chat.deepseek.com/share/gf010o0oduhpe6eqke |
Contributor
Author
|
@keithrbennett, thanks for sharing those reviews! It's great to see that my changes please our new AI overlords. :) I'm happy to make any adjustments if @alexdalitz (or other maintainers) have additional feedback once they have a chance to take a look. |
Owner
|
I’m afraid it will be another couple of weeks before I get a chance to look at this.But the total solar eclipse was awesome!!!Sent from my iPhoneOn 13 Aug 2026, at 02:01, Matthew Rideout ***@***.***> wrote:mrideout left a comment (alexdalitz/dnsruby#222)
@keithrbennett, thanks for sharing those reviews! It's great to see that my changes please our new AI overlords. :) I'm happy to make any adjustments if @alexdalitz (or other maintainers) have additional feedback once they have a chance to take a look.
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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.
Implements SVCB (type 64) and HTTPS (type 65), plus the two fixes they turned out to need: a
ZoneReaderfix for quoted values that open mid-token, and RFC 5952 canonical output fromIPv6#to_s. Each has its own "Prerequisite" section below and can be broken out into separate pull requests if you'd like.lib/dnsruby/resource/SVCB.rb(+HTTPS.rb)SVCBfollows the usualRRsubclass shape (from_hash/from_data/from_string,rdata_to_string,encode_rdata/decode_rdata);HTTPSsubclasses it, since RFC 9460 Section 9 gives the two the same wire and presentation formats. Registered incode_mappers.rb(Types),resource/IN.rb(ClassInsensitiveTypes) andresource/generic.rb, and added to the RR list inREADME.md.mandatory,alpn,no-default-alpn,port,ipv4hint,ipv6hint(RFC 9460),ech(RFC 9848),dohpath(RFC 9461),ohttp(RFC 9540) anddocpath(RFC 9953). Any other key reads and writes losslessly askeyNNNNNwith an opaque value.\Xand\DDDescapes resolved before value-list splitting, and value-list\,/\\. Output escapes anything that would not survive a re-parse.ZoneReaderandRR.new_from_stringstrip them, so a record may be wrapped across lines. The cost is that a parenthesis inside a SvcParamValue is lost, sodohpath="/a(b)c"loads as/abc, though RFC 1035 Section 5.1 treats an unescaped parenthesis inside a quoted string as data. Two independent layers strip it, and both predate this PR.RR.new_from_stringstrips parentheses from the rdata of every type outside the%w(NAPTR TXT)allowlist inresource/RR.rb, which accounts for the quoted spelling;ZoneReaderremoves its own grouping parentheses before the rdata is typed at all, which accounts for the unquoted one. AddingSVCBandHTTPSto that allowlist would therefore fix only half of it, and the other half needs the reader to track quote and escape state, too broad a change to fold in here. Dnsruby's own output round-trips regardless: it writes a parenthesis as\040/\041, which survives both.DecodeErrorrather thanArgumentErrororRangeError, so a zone loader rescuing one line at a time survives a bad record.ZoneReaderqualifies a relative TargetName against the origin. The TargetName sits mid-RDATA, before the SvcParams, out of reach of the existing trailing-name logic.test/tc_svcb.rbadds 37 tests / 293 assertions, including every RFC 9460 Appendix D wire vector (Figures 2-10) and the Appendix D.3 records a parser must reject.test/tc_zone_reader.rbgains relative and absolute TargetNames, multi-line records, and quoted SvcParamValues both bare and inside grouping parentheses.Prerequisite:
ZoneReaderreattaches a quoted tail as it was writtenZoneReadersplits a line at its first", parses the front half and glues the tail back on with a space. That works for TXT, whose quotes wrap a whole token, but not for SVCB, where they open mid-token:alpn="h2,h3"becamealpn= "h2,h3"and failed to parse. The space is now added only where there was one to begin with.That also fixes an existing bug: an unquoted character string containing
\"was split in two, soTXT \"escapedread as"", "\"escaped"rather than as one string, though RFC 1035 Section 5.1 makes\"a literal quote.ZoneReaderandRR.createnow agree on these inputs.Prerequisite:
IPv6#to_semits RFC 5952 canonical formA behavior change to an existing public API, so worth a look on its own. Isolated in commit
70230fdif you would rather take it separately.IPv6#to_semitted uppercase hex and could shorten a shorter run of zero fields than RFC 5952 prescribes. It now emits lowercase hex, no leading zeros, and::for the longest run of all-zero fields (never a single field), with mixed notation for::ffff:0:0/96. It is in this PR becauseipv6hintprints IPv6 addresses into zone files, where people read and diff them. Settling the form before the record type ships avoids changing its output again in a later release.AAAA,IPSECKEYandAPLrecords, and log output. Code that string-compares the old uppercase form needs updating. Parsing and wire format are unchanged, and the old output always re-parsed correctly, so this is presentation only.IPAddr#to_sandinet_ntop: the deprecated IPv4-compatible::/96range (RFC 4291 Section 2.5.5.1) keeps the hexadecimal form, because a dotted quad there would imply a semantic that no longer exists.test/tc_ipv6.rbcovers each RFC 5952 rule; onetest/tc_ipseckey.rbexpectation is updated.Testing
bundle exec rake test_offlinepasses: 186 runs, 1754 assertions, 0 failures, 0 errors, and again underRUBYOPT="--enable-frozen-string-literal --debug-frozen-string-literal"to match the CI job.tc_svcb.rbandtc_ipv6.rbare added totest/ts_offline.rb. Release notes are filed under## Unreleased.