From 14fe4669e79ebd98ce0ffc8b1f51fbb983ec2d91 Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Fri, 12 Aug 2022 14:09:00 +0200
Subject: [PATCH 01/10] fix testcase comment

---
 testdata/svcb.tdir/svcb.failure-cases-01 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testdata/svcb.tdir/svcb.failure-cases-01 b/testdata/svcb.tdir/svcb.failure-cases-01
index c60151692..49b83651a 100644
--- a/testdata/svcb.tdir/svcb.failure-cases-01
+++ b/testdata/svcb.tdir/svcb.failure-cases-01
@@ -3,7 +3,7 @@ $TTL 3600
 
 @       SOA     primary admin 0 0 0 0 0
 
-; Here there are multiple instances of the same SvcParamKey in the mandatory list
+; These cases should be bnase64 encoded but aren't
 
 f21	HTTPS   1 foo.example.com. ech="123"
 f21	HTTPS   1 foo.example.com. echconfig="123"

From 6e31d1f5beb542e81d722f39cae92e59b95fad5a Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Mon, 15 Aug 2022 14:36:35 +0200
Subject: [PATCH 02/10] add dohpath parsing

---
 sldns/str2wire.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 sldns/str2wire.h |  7 +++++--
 sldns/wire2str.c | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 303d49ba6..a620738c2 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1150,6 +1150,11 @@ sldns_str2wire_svcparam_key_lookup(const char *key, size_t key_len)
 			return SVCB_KEY_IPV6HINT;
 		break;
 
+	case sizeof("dohpath")-1:
+		if (!strncmp(key, "dohpath", sizeof("dohpath")-1))
+			return SVCB_KEY_DOHPATH;
+		break;
+
 	case sizeof("ech")-1:
 		if (!strncmp(key, "ech", sizeof("ech")-1))
 			return SVCB_KEY_ECH;
@@ -1515,6 +1520,40 @@ sldns_str2wire_svcbparam_alpn_value(const char* val,
 	return LDNS_WIREPARSE_ERR_OK;
 }
 
+static int
+sldns_str2wire_svcbparam_dohpath_value(const char* val,
+	uint8_t* rd, size_t* rd_len)
+{
+	size_t val_len;
+
+	/* RFC6570#section-2.1
+	 * "The characters outside of expressions in a URI Template string are
+	 * intended to be copied literally"
+	 * Practically this means we do not have to look for "double escapes"
+	 * like in the alpn value list.
+	 */
+
+	val_len = strlen(val);
+
+	if (*rd_len < 4 + val_len) {
+		return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL;
+	}
+
+	/* draft-ietf-add-svcb-dns-06#section-5.1
+	 * The URI Template MUST contain a "dns" variable
+	 */
+	if (!(strstr(val, "?dns"))) {
+		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+	}
+
+	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
+	sldns_write_uint16(rd + 2, val_len);
+	memcpy(rd + 4, val, val_len);
+	*rd_len = 4 + val_len;
+
+	return LDNS_WIREPARSE_ERR_OK;
+}
+
 static int
 sldns_str2wire_svcparam_value(const char *key, size_t key_len,
 	const char *val, uint8_t* rd, size_t* rd_len)
@@ -1535,6 +1574,7 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len,
 		case SVCB_KEY_PORT:
 		case SVCB_KEY_IPV4HINT:
 		case SVCB_KEY_IPV6HINT:
+		case SVCB_KEY_DOHPATH:
 			return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM;
 #endif
 		default:
@@ -1566,6 +1606,8 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len,
 		return sldns_str2wire_svcbparam_ech_value(val, rd, rd_len);
 	case SVCB_KEY_ALPN:
 		return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len);
+	case SVCB_KEY_DOHPATH:
+		return sldns_str2wire_svcbparam_dohpath_value(val, rd, rd_len);
 	default:
 		str_len = strlen(val);
 		if (*rd_len < 4 + str_len)
diff --git a/sldns/str2wire.h b/sldns/str2wire.h
index baee4236f..18cfc4fa7 100644
--- a/sldns/str2wire.h
+++ b/sldns/str2wire.h
@@ -38,7 +38,8 @@ struct sldns_struct_lookup_table;
 #define SVCB_KEY_IPV4HINT		4
 #define SVCB_KEY_ECH			5
 #define SVCB_KEY_IPV6HINT		6
-#define SVCPARAMKEY_COUNT		7
+#define SVCB_KEY_DOHPATH		7
+#define SVCPARAMKEY_COUNT		8
 
 #define MAX_NUMBER_OF_SVCPARAMS	64
 
@@ -234,7 +235,9 @@ uint8_t* sldns_wirerr_get_rdatawl(uint8_t* rr, size_t len, size_t dname_len);
 #define LDNS_WIREPARSE_ERR_SVCB_IPV6_TOO_MANY_ADDRESSES 383
 #define LDNS_WIREPARSE_ERR_SVCB_ALPN_KEY_TOO_LARGE 384
 #define LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE 385
-#define LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA 386
+#define LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH 386
+#define LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA 387
+
 
 /**
  * Get reference to a constant string for the (parse) error.
diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 74d1b62df..5740aee77 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -171,6 +171,8 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
 		"Alpn strings need to be smaller than 255 chars"},
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE,
 		"No-default-alpn should not have a value" },
+	{ LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH,
+		"Dohpath must have '?dns' in the URI template variable" },
 	{ LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA,
 		"General SVCParam error" },
 	{ 0, NULL }
@@ -224,7 +226,7 @@ sldns_lookup_table* sldns_tsig_errors = sldns_tsig_errors_data;
 /* draft-ietf-dnsop-svcb-https-06: 6. Initial SvcParamKeys */
 const char *svcparamkey_strs[] = {
 	"mandatory", "alpn", "no-default-alpn", "port",
-	"ipv4hint", "ech", "ipv6hint"
+	"ipv4hint", "ech", "ipv6hint", "dohpath"
 };
 
 char* sldns_wire2str_pkt(uint8_t* data, size_t len)
@@ -1144,6 +1146,29 @@ static int sldns_wire2str_svcparam_ech2str(char** s,
 	return w + size;
 }
 
+static int sldns_wire2str_svcparam_dohpath2str(char** s,
+	size_t* slen, uint16_t data_len, uint8_t* data)
+{
+	int w = 0;
+	uint16_t i;
+
+	assert(data_len > 0); /* Guaranteed by sldns_wire2str_svcparam_scan */
+
+	w += sldns_str_print(s, slen, "=\"");
+
+	/* RC6570#section-2.1 specifies that the '\' (and other non-letter
+	 * characters in the URI) are "intended to be copied literally" */
+	for (i = 0; i < data_len; i++) {
+		// @TODO do a check like isprint()?
+
+		w += sldns_str_print(s, slen, "%c", data[i]);
+	}
+
+	w += sldns_str_print(s, slen, "\"");
+
+	return w;
+}
+
 int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen)
 {
 	uint8_t ch;
@@ -1174,6 +1199,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
 	 	case SVCB_KEY_IPV4HINT:
 	 	case SVCB_KEY_IPV6HINT:
 	 	case SVCB_KEY_MANDATORY:
+	 	case SVCB_KEY_DOHPATH:
 	 		return -1;
 	 	default:
 	 		return written_chars;
@@ -1201,6 +1227,9 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
 	case SVCB_KEY_ECH:
 		r = sldns_wire2str_svcparam_ech2str(s, slen, data_len, *d);
 		break;
+	case SVCB_KEY_DOHPATH:
+		r = sldns_wire2str_svcparam_dohpath2str(s, slen, data_len, *d);
+		break;
 	default:
 		r = sldns_str_print(s, slen, "=\"");
 

From 73b3b3206276e29dd70d7fa337b90200913b4b6b Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Mon, 15 Aug 2022 14:41:06 +0200
Subject: [PATCH 03/10] fix styling nits

---
 sldns/str2wire.c | 22 +++++++++++-----------
 sldns/wire2str.c | 14 +++++++-------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index a620738c2..5497af999 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -357,7 +357,7 @@ rrinternal_get_delims(sldns_rdf_type rdftype, size_t r_cnt, size_t r_max)
 					break;
 	default                       :	break;
 	}
-	return "\n\t "; 
+	return "\n\t ";
 }
 
 /* Syntactic sugar for sldns_rr_new_frm_str_internal */
@@ -448,7 +448,7 @@ rrinternal_parse_unknown(sldns_buffer* strbuf, char* token, size_t token_len,
 			sldns_buffer_position(strbuf));
 	}
 	hex_data_size = (size_t)atoi(token);
-	if(hex_data_size > LDNS_MAX_RDFLEN || 
+	if(hex_data_size > LDNS_MAX_RDFLEN ||
 		*rr_cur_len + hex_data_size > *rr_len) {
 		return RET_ERR(LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL,
 			sldns_buffer_position(strbuf));
@@ -567,7 +567,7 @@ sldns_parse_rdf_token(sldns_buffer* strbuf, char* token, size_t token_len,
 	/* check if not quoted yet, and we have encountered quotes */
 	if(!*quoted && sldns_rdf_type_maybe_quoted(rdftype) &&
 		slen >= 2 &&
-		(token[0] == '"' || token[0] == '\'') && 
+		(token[0] == '"' || token[0] == '\'') &&
 		(token[slen-1] == '"' || token[slen-1] == '\'')) {
 		/* move token two smaller (quotes) with endnull */
 		memmove(token, token+1, slen-2);
@@ -785,7 +785,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len,
 			token[2]=='\t')) {
 			was_unknown_rr_format = 1;
 			if((status=rrinternal_parse_unknown(strbuf, token,
-				token_len, rr, rr_len, &rr_cur_len, 
+				token_len, rr, rr_len, &rr_cur_len,
 				pre_data_pos)) != 0)
 				return status;
 		} else if(token_strlen > 0 || quoted) {
@@ -844,7 +844,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len,
 	if (rr_type == LDNS_RR_TYPE_SVCB || rr_type == LDNS_RR_TYPE_HTTPS) {
 		size_t rdata_len = rr_cur_len - dname_len - 10;
 		uint8_t *rdata = rr+dname_len + 10;
-		
+
 		/* skip 1st rdata field SvcPriority (uint16_t) */
 		if (rdata_len < sizeof(uint16_t))
 			return LDNS_WIREPARSE_ERR_OK;
@@ -1482,7 +1482,7 @@ sldns_str2wire_svcbparam_alpn_value(const char* val,
 	size_t      str_len;
 	size_t      dst_len;
 	size_t      val_len;
-	
+
 	val_len = strlen(val);
 
 	if (val_len > sizeof(unescaped_dst)) {
@@ -1516,7 +1516,7 @@ sldns_str2wire_svcbparam_alpn_value(const char* val,
 	sldns_write_uint16(rd + 2, dst_len);
 	memcpy(rd + 4, unescaped_dst, dst_len);
 	*rd_len = 4 + dst_len;
-	
+
 	return LDNS_WIREPARSE_ERR_OK;
 }
 
@@ -1635,7 +1635,7 @@ static int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_
 	/* case: key=value */
 	if (eq_pos != NULL && eq_pos[1]) {
 		val_in = eq_pos + 1;
-		
+
 		/* unescape characters and "" blocks */
 		if (*val_in == '"') {
 			val_in++;
@@ -1652,11 +1652,11 @@ static int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_
 		}
 		*val_out = 0;
 
-		return sldns_str2wire_svcparam_value(str, eq_pos - str, 
-		                                         unescaped_val[0] ? unescaped_val : NULL, rd, rd_len);
+		return sldns_str2wire_svcparam_value(str, eq_pos - str,
+		        unescaped_val[0] ? unescaped_val : NULL, rd, rd_len);
 	}
 	/* case: key= */
-	else if (eq_pos != NULL && !(eq_pos[1])) { 
+	else if (eq_pos != NULL && !(eq_pos[1])) {
 		return sldns_str2wire_svcparam_value(str, eq_pos - str, NULL, rd, rd_len);
 	}
 	/* case: key */
diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 5740aee77..d0b89cb75 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -159,7 +159,7 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
 		"Mandatory SvcParamKey is missing"},
 	{ LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY,
 		"Keys in SvcParam mandatory MUST be unique" },
-	{ LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY, 
+	{ LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY,
 		"mandatory MUST not be included as mandatory parameter" },
 	{ LDNS_WIREPARSE_ERR_SVCB_PORT_VALUE_SYNTAX,
 		"Could not parse port SvcParamValue" },
@@ -489,7 +489,7 @@ int sldns_wire2str_rr_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen,
 	uint8_t* rr = *d;
 	size_t rrlen = *dlen, dname_off, rdlen, ordlen;
 	uint16_t rrtype = 0;
-	
+
 	if(*dlen >= 3 && (*d)[0]==0 &&
 		sldns_read_uint16((*d)+1)==LDNS_RR_TYPE_OPT) {
 		/* perform EDNS OPT processing */
@@ -1121,7 +1121,7 @@ static int sldns_wire2str_svcparam_alpn2str(char** s,
 			w += sldns_str_print(s, slen, "%s", ",");
 	}
 	w += sldns_str_print(s, slen, "\"");
-	
+
 	return w;
 }
 
@@ -1141,7 +1141,7 @@ static int sldns_wire2str_svcparam_ech2str(char** s,
 	(*s) += size;
 	(*slen) -= size;
 
-	w += sldns_str_print(s, slen, "\"");	
+	w += sldns_str_print(s, slen, "\"");
 
 	return w + size;
 }
@@ -1187,7 +1187,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
 
 	/* verify that we have data_len data */
 	if (data_len > *dlen)
-		return -1; 
+		return -1;
 
 	written_chars += sldns_print_svcparamkey(s, slen, svcparamkey);
 	if (!data_len) {
@@ -1251,7 +1251,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
 	}
 	if (r <= 0)
 		return -1; /* wireformat error */
-	
+
 	written_chars += r;
 	*d    += data_len;
 	*dlen -= data_len;
@@ -1580,7 +1580,7 @@ int sldns_wire2str_nsec_scan(uint8_t** d, size_t* dl, char** s, size_t* sl)
 	unsigned i, bit, window, block_len;
 	uint16_t t;
 	int w = 0;
-	
+
 	/* check for errors */
 	while(pl) {
 		if(pl < 2) return -1;

From b465e0cfc03392f9b82b8286e26856ba7e940e2f Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Mon, 15 Aug 2022 16:12:22 +0200
Subject: [PATCH 04/10] add testcase and fix comment

---
 sldns/str2wire.c                               | 2 +-
 testdata/svcb.tdir/svcb.success-cases.zone     | 5 +++++
 testdata/svcb.tdir/svcb.success-cases.zone.cmp | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 5497af999..9f1fb0fca 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -698,7 +698,7 @@ static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
 				mandatory = svcparams[i];
 		}
 
-		/* 4. verify that all the SvcParamKeys in mandatory are present */
+		/* Verify that all the SvcParamKeys in mandatory are present */
 		if(mandatory) {
 			/* Divide by sizeof(uint16_t)*/
 			uint16_t mandatory_nkeys = sldns_read_uint16(mandatory + 2) / sizeof(uint16_t);
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index 5d6339542..d63456369 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -45,3 +45,8 @@ s08     HTTPS   0 . ( key11=a key12=a key13=a key14=a key15=a key16=a key17=a ke
 ; maximum alpn size allowed (255 characters)
 
 s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" )
+
+; dohpath can be (non-)quoted and MUST contain "?dns"
+
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index e504e7b18..545333ba8 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -8,3 +8,5 @@ s06.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
\ No newline at end of file

From 8d939691a3a661ffce31bdcb3da9cb07b88ee65b Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Fri, 19 Aug 2022 14:48:47 +0200
Subject: [PATCH 05/10] implement @wcawijngaards' review comment

---
 sldns/wire2str.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index d0b89cb75..75753f910 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -1157,11 +1157,14 @@ static int sldns_wire2str_svcparam_dohpath2str(char** s,
 	w += sldns_str_print(s, slen, "=\"");
 
 	/* RC6570#section-2.1 specifies that the '\' (and other non-letter
-	 * characters in the URI) are "intended to be copied literally" */
+	 * characters in the URI) are "intended to be copied literally" (as 
+	 * opposed to the alpn printing) */
 	for (i = 0; i < data_len; i++) {
-		// @TODO do a check like isprint()?
-
-		w += sldns_str_print(s, slen, "%c", data[i]);
+		if (!isprint(data[i])) {
+			w += sldns_str_print(s, slen, "\\%03u", (unsigned) data[i]);
+		} else {
+			w += sldns_str_print(s, slen, "%c", data[i]);
+		}
 	}
 
 	w += sldns_str_print(s, slen, "\"");

From f3fa3634431806759d61470458fef123dbf1f1f8 Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Wed, 24 Aug 2022 12:38:08 +0200
Subject: [PATCH 06/10] implement @gthess' review comments; fix check on
 compulsory text and add tests

---
 sldns/str2wire.c                               | 14 ++++++++++++--
 testdata/svcb.tdir/svcb.failure-cases-05       |  8 ++++++++
 testdata/svcb.tdir/svcb.success-cases.zone     |  1 +
 testdata/svcb.tdir/svcb.success-cases.zone.cmp |  3 ++-
 testdata/svcb.tdir/svcb.test                   |  6 ++++++
 5 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 testdata/svcb.tdir/svcb.failure-cases-05

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 9f1fb0fca..90648245b 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1525,6 +1525,7 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	uint8_t* rd, size_t* rd_len)
 {
 	size_t val_len;
+	char* open_bracket, * close_bracket, * expr_ptr;
 
 	/* RFC6570#section-2.1
 	 * "The characters outside of expressions in a URI Template string are
@@ -1542,8 +1543,17 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	/* draft-ietf-add-svcb-dns-06#section-5.1
 	 * The URI Template MUST contain a "dns" variable
 	 */
-	if (!(strstr(val, "?dns"))) {
-		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+	open_bracket = strchr(val, '{');
+	close_bracket = strchr(val, '}');
+
+	if (!open_bracket && !close_bracket) {
+		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;	
+	} else {
+		expr_ptr = strstr(open_bracket+1, "?dns");
+
+		if (!expr_ptr || !((close_bracket - expr_ptr) >= 4 ) ) {
+			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+		}
 	}
 
 	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
diff --git a/testdata/svcb.tdir/svcb.failure-cases-05 b/testdata/svcb.tdir/svcb.failure-cases-05
new file mode 100644
index 000000000..67246c954
--- /dev/null
+++ b/testdata/svcb.tdir/svcb.failure-cases-05
@@ -0,0 +1,8 @@
+$ORIGIN failure-cases.
+$TTL 3600
+
+@       SOA     primary admin 0 0 0 0 0
+
+; Dohpath must have '?dns' in the URI template variable
+
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?d}
\ No newline at end of file
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index d63456369..f625c6c5e 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -50,3 +50,4 @@ s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-queryéè{?dns}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index 545333ba8..9075cc6b5 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -9,4 +9,5 @@ s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
-_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
\ No newline at end of file
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query\195\169\195\168{?dns}"
diff --git a/testdata/svcb.tdir/svcb.test b/testdata/svcb.tdir/svcb.test
index 17330e08f..88a9e95ff 100644
--- a/testdata/svcb.tdir/svcb.test
+++ b/testdata/svcb.tdir/svcb.test
@@ -75,6 +75,12 @@ then
 	echo "Failure case 04: 256 is too many characters for an alpn; maximum is 255"
 	echo "Incorrectly succeeded"
 	exit 1
+
+elif $PRE/readzone svcb.failure-cases-05
+then
+	echo "Dohpath must have '?dns' in the URI template variable"
+	echo "Incorrectly succeeded"
+	exit 1
 else
 	echo "All failure cases test successfully"
 fi

From b642c5fe1f34cba894d8779a0a97f367cdc20d5b Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Thu, 25 Aug 2022 14:06:13 +0200
Subject: [PATCH 07/10] add better URI template checking

---
 sldns/str2wire.c                              | 37 ++++++++++++++-----
 sldns/wire2str.c                              |  2 +-
 testdata/svcb.tdir/svcb.success-cases.zone    |  3 +-
 .../svcb.tdir/svcb.success-cases.zone.cmp     |  3 +-
 4 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 90648245b..d210e6cd0 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1525,7 +1525,9 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	uint8_t* rd, size_t* rd_len)
 {
 	size_t val_len;
-	char* open_bracket, * close_bracket, * expr_ptr;
+	char* open_bracket, * close_bracket;
+	const char* next_char;
+	uint8_t expr_found = 0;
 
 	/* RFC6570#section-2.1
 	 * "The characters outside of expressions in a URI Template string are
@@ -1541,19 +1543,36 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	}
 
 	/* draft-ietf-add-svcb-dns-06#section-5.1
-	 * The URI Template MUST contain a "dns" variable
+	 * "The URI Template MUST contain a "dns" variable"
+	 * A URI Template is alowed to have multiple variables
 	 */
-	open_bracket = strchr(val, '{');
-	close_bracket = strchr(val, '}');
+	next_char = val;
+	while (next_char && *next_char != '\0') {
+		char* c;
 
-	if (!open_bracket && !close_bracket) {
-		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;	
-	} else {
-		expr_ptr = strstr(open_bracket+1, "?dns");
+		open_bracket = strchr(next_char, '{');
+		if (!open_bracket) {
+			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+			break;
+		}
 
-		if (!expr_ptr || !((close_bracket - expr_ptr) >= 4 ) ) {
+		close_bracket = strchr(open_bracket, '}');
+		if (!close_bracket) {
 			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+
 		}
+		for (c = open_bracket+1; (close_bracket - c) >= 4; c++) {
+			if (c[0] == '?' && c[1] == 'd' && c[2] == 'n'
+				&& c[3] == 's') {
+				expr_found++;
+			}
+		}
+
+		next_char = close_bracket+1;
+	}
+
+	if (expr_found != 1) {
+		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
 	}
 
 	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 75753f910..5bb13f03b 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -172,7 +172,7 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE,
 		"No-default-alpn should not have a value" },
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH,
-		"Dohpath must have '?dns' in the URI template variable" },
+		"Dohpath must contain a correct URI template variable which contains '?dns'" },
 	{ LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA,
 		"General SVCParam error" },
 	{ 0, NULL }
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index f625c6c5e..fbe1fcb5f 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -49,5 +49,6 @@ s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ; dohpath can be (non-)quoted and MUST contain "?dns"
 
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
-_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcd}{!abcd}{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcdabcd?dns?defedf}
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-queryéè{?dns}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index 9075cc6b5..91ea71682 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -9,5 +9,6 @@ s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
-_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcd}{!abcd}{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcdabcd?dns?defedf}"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query\195\169\195\168{?dns}"

From 5bf4c505db86ef597528012272f834a5c553fddf Mon Sep 17 00:00:00 2001
From: TCY16 <tom@nlnetlabs.nl>
Date: Fri, 28 Oct 2022 17:40:18 +0200
Subject: [PATCH 08/10] remove value check

---
 sldns/str2wire.c                         | 36 ------------------------
 sldns/str2wire.h                         |  3 +-
 sldns/wire2str.c                         | 31 +-------------------
 testdata/svcb.tdir/svcb.failure-cases-05 |  8 ------
 4 files changed, 2 insertions(+), 76 deletions(-)
 delete mode 100644 testdata/svcb.tdir/svcb.failure-cases-05

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index d210e6cd0..5633f5428 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1525,9 +1525,6 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	uint8_t* rd, size_t* rd_len)
 {
 	size_t val_len;
-	char* open_bracket, * close_bracket;
-	const char* next_char;
-	uint8_t expr_found = 0;
 
 	/* RFC6570#section-2.1
 	 * "The characters outside of expressions in a URI Template string are
@@ -1542,39 +1539,6 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 		return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL;
 	}
 
-	/* draft-ietf-add-svcb-dns-06#section-5.1
-	 * "The URI Template MUST contain a "dns" variable"
-	 * A URI Template is alowed to have multiple variables
-	 */
-	next_char = val;
-	while (next_char && *next_char != '\0') {
-		char* c;
-
-		open_bracket = strchr(next_char, '{');
-		if (!open_bracket) {
-			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
-			break;
-		}
-
-		close_bracket = strchr(open_bracket, '}');
-		if (!close_bracket) {
-			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
-
-		}
-		for (c = open_bracket+1; (close_bracket - c) >= 4; c++) {
-			if (c[0] == '?' && c[1] == 'd' && c[2] == 'n'
-				&& c[3] == 's') {
-				expr_found++;
-			}
-		}
-
-		next_char = close_bracket+1;
-	}
-
-	if (expr_found != 1) {
-		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
-	}
-
 	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
 	sldns_write_uint16(rd + 2, val_len);
 	memcpy(rd + 4, val, val_len);
diff --git a/sldns/str2wire.h b/sldns/str2wire.h
index 18cfc4fa7..5e4d146d3 100644
--- a/sldns/str2wire.h
+++ b/sldns/str2wire.h
@@ -235,8 +235,7 @@ uint8_t* sldns_wirerr_get_rdatawl(uint8_t* rr, size_t len, size_t dname_len);
 #define LDNS_WIREPARSE_ERR_SVCB_IPV6_TOO_MANY_ADDRESSES 383
 #define LDNS_WIREPARSE_ERR_SVCB_ALPN_KEY_TOO_LARGE 384
 #define LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE 385
-#define LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH 386
-#define LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA 387
+#define LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA 386
 
 
 /**
diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 5bb13f03b..e6278ff56 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -171,8 +171,6 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
 		"Alpn strings need to be smaller than 255 chars"},
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE,
 		"No-default-alpn should not have a value" },
-	{ LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH,
-		"Dohpath must contain a correct URI template variable which contains '?dns'" },
 	{ LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA,
 		"General SVCParam error" },
 	{ 0, NULL }
@@ -1146,32 +1144,6 @@ static int sldns_wire2str_svcparam_ech2str(char** s,
 	return w + size;
 }
 
-static int sldns_wire2str_svcparam_dohpath2str(char** s,
-	size_t* slen, uint16_t data_len, uint8_t* data)
-{
-	int w = 0;
-	uint16_t i;
-
-	assert(data_len > 0); /* Guaranteed by sldns_wire2str_svcparam_scan */
-
-	w += sldns_str_print(s, slen, "=\"");
-
-	/* RC6570#section-2.1 specifies that the '\' (and other non-letter
-	 * characters in the URI) are "intended to be copied literally" (as 
-	 * opposed to the alpn printing) */
-	for (i = 0; i < data_len; i++) {
-		if (!isprint(data[i])) {
-			w += sldns_str_print(s, slen, "\\%03u", (unsigned) data[i]);
-		} else {
-			w += sldns_str_print(s, slen, "%c", data[i]);
-		}
-	}
-
-	w += sldns_str_print(s, slen, "\"");
-
-	return w;
-}
-
 int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen)
 {
 	uint8_t ch;
@@ -1231,8 +1203,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
 		r = sldns_wire2str_svcparam_ech2str(s, slen, data_len, *d);
 		break;
 	case SVCB_KEY_DOHPATH:
-		r = sldns_wire2str_svcparam_dohpath2str(s, slen, data_len, *d);
-		break;
+		/* fallthrough */
 	default:
 		r = sldns_str_print(s, slen, "=\"");
 
diff --git a/testdata/svcb.tdir/svcb.failure-cases-05 b/testdata/svcb.tdir/svcb.failure-cases-05
deleted file mode 100644
index 67246c954..000000000
--- a/testdata/svcb.tdir/svcb.failure-cases-05
+++ /dev/null
@@ -1,8 +0,0 @@
-$ORIGIN failure-cases.
-$TTL 3600
-
-@       SOA     primary admin 0 0 0 0 0
-
-; Dohpath must have '?dns' in the URI template variable
-
-_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?d}
\ No newline at end of file

From ade710a9fd95092e1590a93e097c8512fc278e7e Mon Sep 17 00:00:00 2001
From: George Thessalonikefs <george@nlnetlabs.nl>
Date: Mon, 3 Jul 2023 10:10:16 +0200
Subject: [PATCH 09/10] - For #739: minor cleanup for testcases.

---
 testdata/svcb.tdir/svcb.failure-cases-01       | 2 +-
 testdata/svcb.tdir/svcb.success-cases.zone     | 9 ++++++++-
 testdata/svcb.tdir/svcb.success-cases.zone.cmp | 4 ++++
 testdata/svcb.tdir/svcb.test                   | 7 +------
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/testdata/svcb.tdir/svcb.failure-cases-01 b/testdata/svcb.tdir/svcb.failure-cases-01
index 49b83651a..6d57584f3 100644
--- a/testdata/svcb.tdir/svcb.failure-cases-01
+++ b/testdata/svcb.tdir/svcb.failure-cases-01
@@ -3,7 +3,7 @@ $TTL 3600
 
 @       SOA     primary admin 0 0 0 0 0
 
-; These cases should be bnase64 encoded but aren't
+; These cases should be base64 encoded but aren't
 
 f21	HTTPS   1 foo.example.com. ech="123"
 f21	HTTPS   1 foo.example.com. echconfig="123"
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index fbe1fcb5f..c3d015ec0 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -47,7 +47,14 @@ s08     HTTPS   0 . ( key11=a key12=a key13=a key14=a key15=a key16=a key17=a ke
 s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" )
 
 ; dohpath can be (non-)quoted and MUST contain "?dns"
-
+; currently there is no validation from Unbound, it can be anything
+; maybe needs changing if Unbound is the primary authoritative for SVCB records.
+; Then SVCB_SEMANTIC_CHECKS parts of the code could be used per authoritative role.
+
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=""
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/"
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcd}{!abcd}{?dns}
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcdabcd?dns?defedf}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index 91ea71682..3a42393ba 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -8,6 +8,10 @@ s06.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+_dns.doh.example.	7200	IN	SVCB	\# 26 000103646F68076578616D706C65000001000302683200070000
+_dns.doh.example.	7200	IN	SVCB	\# 26 000103646F68076578616D706C65000001000302683200070000
+_dns.doh.example.	7200	IN	SVCB	\# 26 000103646F68076578616D706C65000001000302683200070000
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcd}{!abcd}{?dns}"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcdabcd?dns?defedf}"
diff --git a/testdata/svcb.tdir/svcb.test b/testdata/svcb.tdir/svcb.test
index 88a9e95ff..280c58fc8 100644
--- a/testdata/svcb.tdir/svcb.test
+++ b/testdata/svcb.tdir/svcb.test
@@ -66,7 +66,7 @@ then
 
 elif $PRE/readzone svcb.failure-cases-03
 then
-	echo "Failure case 02: 65 SvcParams is too many SvcParams; the limit is 64"
+	echo "Failure case 03: 65 SvcParams is too many SvcParams; the limit is 64"
 	echo "Incorrectly succeeded"
 	exit 1
 
@@ -76,11 +76,6 @@ then
 	echo "Incorrectly succeeded"
 	exit 1
 
-elif $PRE/readzone svcb.failure-cases-05
-then
-	echo "Dohpath must have '?dns' in the URI template variable"
-	echo "Incorrectly succeeded"
-	exit 1
 else
 	echo "All failure cases test successfully"
 fi

From 5be7f1ef8aa9731ae30ffbfc9c9a0a239763ece7 Mon Sep 17 00:00:00 2001
From: George Thessalonikefs <george@nlnetlabs.nl>
Date: Mon, 3 Jul 2023 10:51:34 +0200
Subject: [PATCH 10/10] - Code cleanup for sldns_str2wire_svcparam_key_lookup.

---
 sldns/str2wire.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 5633f5428..45e247613 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1123,41 +1123,40 @@ sldns_str2wire_svcparam_key_lookup(const char *key, size_t key_len)
 			return key_value;
 
 	} else switch (key_len) {
-	case sizeof("mandatory")-1:
-		if (!strncmp(key, "mandatory", sizeof("mandatory")-1))
-			return SVCB_KEY_MANDATORY;
-		if (!strncmp(key, "echconfig", sizeof("echconfig")-1))
-			return SVCB_KEY_ECH; /* allow "echconfig" as well as "ech" */
+	case 3:
+		if (!strncmp(key, "ech", key_len))
+			return SVCB_KEY_ECH;
 		break;
 
-	case sizeof("alpn")-1:
-		if (!strncmp(key, "alpn", sizeof("alpn")-1))
+	case 4:
+		if (!strncmp(key, "alpn", key_len))
 			return SVCB_KEY_ALPN;
-		if (!strncmp(key, "port", sizeof("port")-1))
+		if (!strncmp(key, "port", key_len))
 			return SVCB_KEY_PORT;
 		break;
 
-	case sizeof("no-default-alpn")-1:
-		if (!strncmp( key  , "no-default-alpn"
-		            , sizeof("no-default-alpn")-1))
-			return SVCB_KEY_NO_DEFAULT_ALPN;
+	case 7:
+		if (!strncmp(key, "dohpath", key_len))
+			return SVCB_KEY_DOHPATH;
 		break;
 
-	case sizeof("ipv4hint")-1:
-		if (!strncmp(key, "ipv4hint", sizeof("ipv4hint")-1))
+	case 8:
+		if (!strncmp(key, "ipv4hint", key_len))
 			return SVCB_KEY_IPV4HINT;
-		if (!strncmp(key, "ipv6hint", sizeof("ipv6hint")-1))
+		if (!strncmp(key, "ipv6hint", key_len))
 			return SVCB_KEY_IPV6HINT;
 		break;
 
-	case sizeof("dohpath")-1:
-		if (!strncmp(key, "dohpath", sizeof("dohpath")-1))
-			return SVCB_KEY_DOHPATH;
+	case 9:
+		if (!strncmp(key, "mandatory", key_len))
+			return SVCB_KEY_MANDATORY;
+		if (!strncmp(key, "echconfig", key_len))
+			return SVCB_KEY_ECH; /* allow "echconfig" as well as "ech" */
 		break;
 
-	case sizeof("ech")-1:
-		if (!strncmp(key, "ech", sizeof("ech")-1))
-			return SVCB_KEY_ECH;
+	case 15:
+		if (!strncmp(key, "no-default-alpn", key_len))
+			return SVCB_KEY_NO_DEFAULT_ALPN;
 		break;
 
 	default: