Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions minidns-core/src/main/java/org/minidns/record/Record.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public enum TYPE {
CDNSKEY(60),
OPENPGPKEY(61, OPENPGPKEY.class),
CSYNC(62),
SVCB(64, SVCB.class),
SPF(99),
UINFO(100),
UID(101),
Expand Down Expand Up @@ -399,6 +400,9 @@ public static Record<Data> parse(DataInputStream dis, byte[] data) throws IOExce
case DLV:
payloadData = DLV.parse(dis, payloadLength);
break;
case SVCB:
payloadData = SVCB.parse(dis, payloadLength, data);
break;
case UNKNOWN:
default:
payloadData = UNKNOWN.parse(dis, payloadLength, type);
Expand Down
102 changes: 102 additions & 0 deletions minidns-core/src/main/java/org/minidns/record/SVCB.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package org.minidns.record;

import org.minidns.dnsname.DnsName;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* SVCB Record Type (Service binding)
*
* https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm going to add it.

*/
class SVCB extends RRWithTarget {

/**
* The priority indicates the SvcRecordType.
* https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.4
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use @see

*/
public final int priority;

/**
* SvcFieldValue
* A set of key=value pairs.
* https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use @see

*/
public final Map<String, String> values;

// 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: superfluous whitespace at the front

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's meant as an indent for the previous line (which would be too long as a single line)

private static final Pattern valuesPattern = Pattern.compile("([a-z0-9\\-]+)=\"([^\"]*)\"");

/**
* @param priority SvcRecordType
* @param target SvcDomainName
* @param values SvcFieldValue
*/
public SVCB(int priority, DnsName target, Map<String, String> values) {
super(target);
this.priority = priority;
this.values = values;
}

public static SVCB parse(DataInputStream dis, int length, byte[] data)
throws IOException {
int priority = dis.readUnsignedShort();
DnsName target = DnsName.parse(dis, data);

byte[] valuesBlob = new byte[length - 2 - target.getRawBytes().length];
dis.readFully(valuesBlob);
return new SVCB(priority, target, parseValuesBlob(valuesBlob));
}

/**
* Parses pairs according to format from https://tools.ietf.org/html/draft-ietf-dnsop-svcb-httpssvc-01#section-2.1.1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use @see.

*/
private static Map<String, String> parseValuesBlob(byte[] blob) {
Map<String, String> values = new LinkedHashMap<>();
String blobAsString = new String(blob, StandardCharsets.UTF_8);
Matcher matcher = valuesPattern.matcher(blobAsString);
while(matcher.find()) {
values.put(matcher.group(1), matcher.group(2));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be some unescaping for commas in this method?

}
return values;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map should here be made immutable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

}

@Override
public Record.TYPE getType() {
return Record.TYPE.SVCB;
}

@Override
public void serialize(DataOutputStream dos) throws IOException {
dos.writeShort(priority);
super.serialize(dos);
dos.write(createValuesString().getBytes(StandardCharsets.UTF_8));
}

@Override
public String toString() {
return priority + " " + target + createValuesString();
}

private String createValuesString() {
StringBuilder builder = new StringBuilder();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be some escaping for commas in this method?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

for (Map.Entry<String, String> entry : values.entrySet()) {
builder.append(" ");
builder.append(entry.getKey());
builder.append("=");
builder.append("\"");
builder.append(entry.getValue());
builder.append("\"");
}
return builder.toString();
}
}
21 changes: 21 additions & 0 deletions minidns-core/src/test/java/org/minidns/record/RecordsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.minidns.constants.DnssecConstants.DigestAlgorithm;
import org.minidns.constants.DnssecConstants.SignatureAlgorithm;
import org.minidns.dnsname.DnsName;
import org.minidns.record.NSEC3.HashAlgorithm;
import org.minidns.record.Record.TYPE;
import org.junit.jupiter.api.Test;
Expand All @@ -21,7 +22,10 @@
import java.io.IOException;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import static org.minidns.Assert.assertCsEquals;
import static org.minidns.Assert.assertArrayContentEquals;
Expand All @@ -48,6 +52,23 @@ public void testARecord() throws Exception {
assertArrayEquals(new byte[] {127, 0, 0, 1}, a.getIp());
}

@Test
public void testSVCBRecord() throws Exception {
Map<String, String> values = new LinkedHashMap<>();
values.put("just", "testing");
values.put("lookma", "nopläintêxt");
values.put("even", "numbers like 1 and very long text with spaces in it.");
SVCB svcb = new SVCB(1, DnsName.from("example.com"), values);

assertEquals(TYPE.SVCB, svcb.getType());

String expectedString = "1 example.com just=\"testing\" lookma=\"nopläintêxt\" even=\"numbers like 1 and very long text with spaces in it.\"";
assertEquals(expectedString, svcb.toString());
byte[] svcbb = svcb.toByteArray();
svcb = SVCB.parse(new DataInputStream(new ByteArrayInputStream(svcbb)), svcb.length(), svcbb);
assertEquals(expectedString, svcb.toString());
}

@Test
public void testARecordInvalidIp() throws Exception {
assertThrows(IllegalArgumentException.class, () ->
Expand Down