SVCB: allow parsing generic key without value

diff --git a/pdns/rcpgenerator.cc b/pdns/rcpgenerator.cc
index 10afab9bc068b4d3124a6591c14863c93cfd3f9f..c3ec2c20d70ea5608c7027a175d365417b12ad4d 100644 (file)
--- a/pdns/rcpgenerator.cc
+++ b/pdns/rcpgenerator.cc
@@ -362,10 +362,7 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
       throw RecordTextException(e.what());
     }
 
-    if (key != SvcParam::no_default_alpn) {
-      if (d_pos == d_end || d_string.at(d_pos) != '=') {
-        throw RecordTextException("expected '=' after " + k);
-      }
+    if (d_pos != d_end && d_string.at(d_pos) == '=') {
       d_pos++; // Now on the first character after '='
       if (d_pos == d_end || d_string.at(d_pos) == ' ') {
         throw RecordTextException("expected value after " + k + "=");
@@ -374,7 +371,7 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
 
     switch (key) {
     case SvcParam::no_default_alpn:
-      if (d_pos != d_end && d_string.at(d_pos) == '=') {
+      if (d_pos != d_end && d_string.at(d_pos) != ' ') {
         throw RecordTextException(k + " key can not have values");
       }
       val.insert(SvcParam(key));
@@ -406,6 +403,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
           hints.push_back(ComboAddress(v));
         }
       }
+      if (!doAuto && hints.empty()) {
+        throw RecordTextException("value is required for SVC Param " + k);
+      }
       try {
         auto p = SvcParam(key, std::move(hints));
         p.setAutoHint(doAuto);
@@ -425,6 +425,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
         while (spos < v.length()) {
           len = v.at(spos);
           spos += 1;
+          if (len == 0) {
+            throw RecordTextException("ALPN values cannot be empty strings");
+          }
           if (len > v.length() - spos) {
             throw RecordTextException("Length of ALPN value goes over total length of alpn SVC Param");
           }
@@ -434,6 +437,14 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
       } else {
         xfrSVCBValueList(value);
       }
+      if (value.empty()) {
+        throw RecordTextException("value is required for SVC Param " + k);
+      }
+      for (const auto &alpn_value : value) {
+        if (alpn_value.empty()) {
+          throw RecordTextException("ALPN values cannot be empty strings");
+        }
+      }
       val.insert(SvcParam(key, std::move(value)));
       break;
     }
@@ -441,6 +452,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
       if (generic) {
         string v;
         xfrRFC1035CharString(v);
+        if (v.empty()) {
+          throw RecordTextException("value is required for SVC Param " + k);
+        }
         if (v.length() % 2 != 0) {
           throw RecordTextException("Wrong number of bytes in SVC Param " + k);
         }
@@ -455,6 +469,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
       }
       vector<string> parts;
       xfrSVCBValueList(parts);
+      if (parts.empty()) {
+        throw RecordTextException("value is required for SVC Param " + k);
+      }
       set<string> values(parts.begin(), parts.end());
       val.insert(SvcParam(key, std::move(values)));
       break;
@@ -472,6 +489,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
       } else {
         string portstring;
         xfrRFC1035CharString(portstring);
+        if (portstring.empty()) {
+          throw RecordTextException("value is required for SVC Param " + k);
+        }
         try {
           pdns::checked_stoi_into(port, portstring);
         } catch (const std::exception &e) {
@@ -498,12 +518,20 @@ void RecordTextReader::xfrSvcParamKeyVals(set<SvcParam>& val) // NOLINT(readabil
           d_pos++;
         }
       }
+      if (value.empty()) {
+        throw RecordTextException("value is required for SVC Param " + k);
+      }
       val.insert(SvcParam(key, value));
       break;
     }
     default: {
       string value;
       xfrRFC1035CharString(value);
+      if (!generic && value.empty()) {
+        // for generic format, we do not know.
+        // Known keys which forbid having a value need to implement a switch case, above.
+        throw RecordTextException("value is required for SVC Param " + k);
+      }
       val.insert(SvcParam(key, value));
       break;
     }

diff --git a/pdns/test-rcpgenerator_cc.cc b/pdns/test-rcpgenerator_cc.cc
index ba8357d6ad36789a4f83b1532df7a6d731ffa2c7..02960d9fcc6543bb887e9804e3c51d6808750221 100644 (file)
--- a/pdns/test-rcpgenerator_cc.cc
+++ b/pdns/test-rcpgenerator_cc.cc
@@ -92,6 +92,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_alpn) {
         source="key1=\\002h2\\002h3foobar"; // extra data
         RecordTextReader rtr5(source);
         BOOST_CHECK_THROW(rtr5.xfrSvcParamKeyVals(v), RecordTextException);
+
+        source="key1"; // no data
+        RecordTextReader rtr6(source);
+        BOOST_CHECK_THROW(rtr6.xfrSvcParamKeyVals(v), RecordTextException);
+
+        source="key1=\\000"; // no data
+        RecordTextReader rtr7(source);
+        BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_mandatory) {
@@ -180,12 +188,37 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_no_default_alpn) {
 
         // Generic
         v.clear();
-        source = "key2";
-        RecordTextReader rtr3(source);
+        RecordTextReader rtr3("key2");
         rtr3.xfrSvcParamKeyVals(v);
         BOOST_CHECK_EQUAL(v.size(), 1U);
         k = v.begin()->getKey();
         BOOST_CHECK(k == SvcParam::no_default_alpn);
+
+        v.clear();
+        RecordTextReader rtr4("key2=");
+        BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr5("key2 ipv4hint=1.2.3.4");
+        rtr5.xfrSvcParamKeyVals(v);
+        BOOST_CHECK_EQUAL(v.size(), 2U);
+        k = v.begin()->getKey();
+        BOOST_CHECK(k == SvcParam::no_default_alpn);
+
+        v.clear();
+        RecordTextReader rtr6("ipv4hint=1.2.3.4 key2");
+        rtr6.xfrSvcParamKeyVals(v);
+        BOOST_CHECK_EQUAL(v.size(), 2U);
+        k = v.begin()->getKey();
+        BOOST_CHECK(k == SvcParam::no_default_alpn);
+
+        v.clear();
+        RecordTextReader rtr7("key2=port=123 ipv4hint=1.2.3.4");
+        BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr8("key2=port=123");
+        BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv4hint) {
@@ -263,6 +296,23 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv4hint) {
         source = "key4=\\192\\000\\222"; // Wrong number of octets
         RecordTextReader rtr7(source);
         BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr8("key4=");  // must have a value
+        BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr9("ipv4hint=");  // must have a value
+        BOOST_CHECK_THROW(rtr9.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr10("ipv4hint=auto");  // special value
+        rtr10.xfrSvcParamKeyVals(v);
+        BOOST_CHECK_EQUAL(v.size(), 1U);
+        k = v.begin()->getKey();
+        BOOST_CHECK(k == SvcParam::ipv4hint);
+        BOOST_CHECK_EQUAL(v.begin()->getIPHints().size(), 0U);
+        BOOST_CHECK_EQUAL(v.begin()->getAutoHint(), true);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv6hint) {
@@ -339,6 +389,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv6hint) {
         source = "key6=\\040\\001\\015\\270\\000\\123\\000\\000\\000\\000\\000\\000\\000\\000\\000"; // wrong number of octets
         RecordTextReader rtr7(source);
         BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr8("key6="); // must have a value
+        BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr9("ipv6hint=");  // must have a value
+        BOOST_CHECK_THROW(rtr9.xfrSvcParamKeyVals(v), RecordTextException);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_port) {
@@ -412,7 +470,10 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_generic) {
 
         v.clear();
         RecordTextReader rtr3("key666");
-        BOOST_CHECK_THROW(rtr3.xfrSvcParamKeyVals(v), RecordTextException);
+        BOOST_CHECK_NO_THROW(rtr3.xfrSvcParamKeyVals(v));
+        BOOST_CHECK_EQUAL(v.size(), 1U);
+        k = v.begin()->getKey();
+        BOOST_CHECK(k == SvcParam::keyFromString("key666"));
 
         v.clear();
         source = "key666=\"blablabla\"";
@@ -490,6 +551,20 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_multiple) {
         RecordTextWriter rtw(target);
         rtw.xfrSvcParamKeyVals(v);
         BOOST_CHECK_EQUAL(target, "mandatory=alpn alpn=h2,h3 ipv4hint=192.0.2.1,192.0.2.2 ech=\"dG90YWxseSBib2d1cyBlY2hjb25maWcgdmFsdWU=\" ipv6hint=2001:db8::1 key666=\"foobar\"");
+
+        v.clear();
+        RecordTextReader rtr2("mandatory=alpn key666"); // generic key without value at the end of the string
+        BOOST_CHECK_NO_THROW(rtr2.xfrSvcParamKeyVals(v));
+        BOOST_CHECK_EQUAL(v.size(), 2U);
+
+        v.clear();
+        RecordTextReader rtr3("key666 key677=\"foo\" mandatory=alpn"); // generic key without value -not- at the end of the string
+        BOOST_CHECK_NO_THROW(rtr3.xfrSvcParamKeyVals(v));
+        BOOST_CHECK_EQUAL(v.size(), 3U);
+
+        v.clear();
+        RecordTextReader rtr4("mandatory= key666"); // non-generic key without value -not- at the end of the string
+        BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ech) {
@@ -519,6 +594,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ech) {
         BOOST_CHECK(k == SvcParam::ech);
         val = v.begin()->getECH();
         BOOST_CHECK_EQUAL(val, "echconfig");
+
+        v.clear();
+        RecordTextReader rtr3("key5");
+        BOOST_CHECK_THROW(rtr3.xfrSvcParamKeyVals(v), RecordTextException);
+
+        v.clear();
+        RecordTextReader rtr4("ech=\"\"");
+        BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException);
 }
 
 BOOST_AUTO_TEST_CASE(test_xfrNodeOrLocatorID) {

diff --git a/pdns/test-svc_records_cc.cc b/pdns/test-svc_records_cc.cc
index bb7f08d2efeadcbf08caeae4fcc93ff98158fedf..920b1083a6b5e398f689d3ddb90cc51530ca3629 100644 (file)
--- a/pdns/test-svc_records_cc.cc
+++ b/pdns/test-svc_records_cc.cc
@@ -53,6 +53,9 @@ BOOST_AUTO_TEST_CASE(test_SvcParam_keyFromString) {
     k = SvcParam::keyFromString("key666");
     BOOST_CHECK(k == 666);
 
+    k = SvcParam::keyFromString("key00666");
+    BOOST_CHECK(k == 666);
+
     BOOST_CHECK_THROW(SvcParam::keyFromString("MANDATORY"), std::invalid_argument);
 }