Apply suggestions from code review

Co-authored-by: Willem Toorop <willem@nlnetlabs.nl>

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 9f94f69cda5fa019c158af1b06f0e13e4393103a..55c38e12db4fefba89ca1659c05ffc7ff9c26f75 100644 (file)
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -853,7 +853,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len,
                }
                /* The root label is one more character, so smaller
                 * than 1 + 1 means no Svcparam Keys */
-               if (rdata_len < 2)
+               if (rdata_len < 2 || *rdata != 0)
                        return LDNS_WIREPARSE_ERR_OK;
 
                rdata_len -= 1;
@@ -1348,26 +1348,26 @@ sldns_str2wire_svcbparam_mandatory(const char* val, uint8_t* rd, size_t* rd_len)
         * So long as we do not distinguish between running Unbound as a primary
         * or as a secondary, we default to secondary behavior and we ignore the
         * semantic errors. */
-
+#ifdef SVCB_SEMANTIC_ERRORS
        /* In draft-ietf-dnsop-svcb-https-06 Section 8
         * automatically mandatory MUST NOT appear in its own value-list
         */
-       // if (sldns_read_uint16(rd + 4) == SVCB_KEY_MANDATORY)
-       //      return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY;
+       if (sldns_read_uint16(rd + 4) == SVCB_KEY_MANDATORY)
+               return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY;
 
        /* Guarantee key uniqueness. After the sort we only need to
         * compare neighbouring keys */
-       // if (count > 1) {
-       //      for (i = 0; i < count - 1; i++) {
-       //              uint8_t* current_pos = (rd + 4 + (sizeof(uint16_t) * i));
-       //              uint16_t key = sldns_read_uint16(current_pos);
-
-       //              if (key == sldns_read_uint16(current_pos + 2)) {
-       //                      return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY;
-       //              }
-       //      }
-       // }
+       if (count > 1) {
+               for (i = 0; i < count - 1; i++) {
+                       uint8_t* current_pos = (rd + 4 + (sizeof(uint16_t) * i));
+                       uint16_t key = sldns_read_uint16(current_pos);
 
+                       if (key == sldns_read_uint16(current_pos + 2)) {
+                               return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY;
+                       }
+               }
+       }
+#endif
        return LDNS_WIREPARSE_ERR_OK;
 }
 

diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 5bfa9f43da7f2169a2d5ca87d2fa3881df8f9caf..6a177ec0b06cec63a6d38941132ee0aaccbddfd0 100644 (file)
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -989,7 +989,6 @@ static int sldns_wire2str_svcparam_port2str(char** s,
        if (data_len != 2)
                return -1; /* wireformat error, a short is 2 bytes */
        w = sldns_str_print(s, slen, "=%d", (int)sldns_read_uint16(data));
-       data += 2;
 
        return w;
 }