WIP: Add SVCB record #110
WIP: Add SVCB record #110
Conversation
| /** | ||
| * SVCB Record Type (Service binding) | ||
| * | ||
| * https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01 |
There was a problem hiding this comment.
This link, and all other links in javadoc in this file, should ideally be using @see. For example in this case
* @see <a href="https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-01">draft-ietf-dnsop-svcb-https-01: Service binding and parameter specification via the DNS (DNS SVCB and HTTPS RRs)</a>
|
|
||
| // The first group is the key. They key can only be a-z, 0-9 or "-" | ||
| // The second group is the value. It can be a lot of things (see https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.1.1) | ||
| // except for DQUOTE (hence it can be excluded from the regex-group) |
There was a problem hiding this comment.
nit: superfluous whitespace at the front
There was a problem hiding this comment.
It's meant as an indent for the previous line (which would be too long as a single line)
|
|
||
| /** | ||
| * The priority indicates the SvcRecordType. | ||
| * https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.4 |
| /** | ||
| * SvcFieldValue | ||
| * A set of key=value pairs. | ||
| * https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.1 |
| } | ||
|
|
||
| /** | ||
| * Parses pairs according to format from https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.1.1 |
| while(matcher.find()) { | ||
| values.put(matcher.group(1), matcher.group(2)); | ||
| } | ||
| return values; |
There was a problem hiding this comment.
The map should here be made immutable.
| } | ||
|
|
||
| private String createValuesString() { | ||
| StringBuilder builder = new StringBuilder(); |
There was a problem hiding this comment.
Shouldn't there be some escaping for commas in this method?
There was a problem hiding this comment.
It seems like I have to update this regardless. There are two RFCs floating around for this RR and I based this on the older one. The one I linked in the PR description is the correct one.
Right now the wireformat is not valid.
|
I updated the PR with the comments. One thing that I think is still wrong is how the values of the key-value pairs are decoded (key decoding is fine, it's a plain UShort). According to the RFC the value is an octet String (which is why I currently simply decode with UTF-8), but the Appendix has some encoding for the value. I can't make any sense of it, but maybe you can? |
|
I'm going to close this PR. I don't have enough knowledge about the encoding and wireformat to make this work. I published my last changes (which don't work right now). SVCB support would be nice though, it'd be awesome if you could somehow finish this, if you've got time. |
|
I think it is best to mark this PR as WIP and leave it open then. |
| String blobAsString = new String(blob, StandardCharsets.UTF_8); | ||
| Matcher matcher = valuesPattern.matcher(blobAsString); | ||
| while(matcher.find()) { | ||
| values.put(matcher.group(1), matcher.group(2)); |
There was a problem hiding this comment.
Shouldn't there be some unescaping for commas in this method?
SVCB is a new record type currently in draft: https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-01
Support for it has already been added to Safari in iOS 14 (where it is used by default instead of A/AAAA), Cloudflare also added support for it just recently. Mozilla announced to implement it soon as well.
This PR adds the TYPE (RR type 64 according to the RFC) and the Data class for it.