Rework the QType class

This add QType::fromString() which does error checking and throws exceptions when invalid
input is provided.

Cleans up the anti-pattern that was previously there where you could only construct a
QType object from a string by assigning to a dummy one. Also removes access to internals
like chartocode which was being used all around in place of that anti-pattern.

modules/bindbackend/bindbackend2.cc

@@ -869,7 +869,7 @@ void Bind2Backend::doEmptyNonTerminals(std::shared_ptr<recordstorage_t>& records
   }
 
   DNSResourceRecord rr;
-  rr.qtype = "#0";
+  rr.qtype = QType::fromString("#0");
   rr.content = "";
   rr.ttl = 0;
   for (auto& nt : nonterm) {

modules/geoipbackend/geoipbackend.cc

@@ -237,7 +237,7 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, std::uint32_t domainID,
         }
         else {
           string qtype = boost::to_upper_copy(rec->first.as<string>());
-          rr.qtype = qtype;
+          rr.qtype = QType::fromString(qtype);
         }
         rr.has_weight = false;
         rr.weight = 100;

modules/ldapbackend/ldapbackend.cc

@@ -180,7 +180,7 @@ void LdapBackend::extract_entry_results(const DNSName& domain, const DNSResult&
       attrname = attribute.first;
       // extract qtype string from ldap attribute name by removing the 'Record' suffix.
       qstr = attrname.substr(0, attrname.length() - 6);
-      qt = toUpper(qstr);
+      qt = QType::fromString(qstr);
 
       for (const auto& value : attribute.second) {
         if (qtype != qt && qtype != QType::ANY) {

modules/lua2backend/lua2api2.hh

@@ -140,7 +140,7 @@ public:
           if (item.second.which() == 1)
             rec.qtype = QType(boost::get<int>(item.second));
           else if (item.second.which() == 3)
-            rec.qtype = boost::get<string>(item.second);
+            rec.qtype = QType::fromString(boost::get<string>(item.second));
           else if (item.second.which() == 4)
             rec.qtype = boost::get<QType>(item.second);
           else

modules/pipebackend/pipebackend.cc

@@ -312,7 +312,7 @@ bool PipeBackend::get(DNSResourceRecord& r)
           r.auth = true;
         }
         r.qname = DNSName(parts[1 + extraFields]);
-        r.qtype = parts[3 + extraFields];
+        r.qtype = QType::fromString(parts[3 + extraFields]);
         pdns::checked_stoi_into(r.ttl, parts[4 + extraFields]);
         pdns::checked_stoi_into(r.domain_id, parts[5 + extraFields]);
 

modules/remotebackend/remotebackend.cc

@@ -252,7 +252,7 @@ bool RemoteBackend::get(DNSResourceRecord& rr)
     return false;
   }
 
-  rr.qtype = stringFromJson(d_result["result"][d_index], "qtype");
+  rr.qtype = QType::fromString(stringFromJson(d_result["result"][d_index], "qtype"));
   rr.qname = DNSName(stringFromJson(d_result["result"][d_index], "qname"));
   rr.qclass = QClass::IN;
   rr.content = stringFromJson(d_result["result"][d_index], "content");
@@ -853,7 +853,7 @@ bool RemoteBackend::searchRecords(const string& pattern, size_t maxResults, vect
 
   for (const auto& row : answer["result"].array_items()) {
     DNSResourceRecord rr;
-    rr.qtype = stringFromJson(row, "qtype");
+    rr.qtype = QType::fromString(stringFromJson(row, "qtype"));
     rr.qname = DNSName(stringFromJson(row, "qname"));
     rr.qclass = QClass::IN;
     rr.content = stringFromJson(row, "content");

pdns/backends/gsql/gsqlbackend.cc

@@ -2283,7 +2283,7 @@ void GSQLBackend::extractRecord(SSqlStatement::row_t& row, DNSResourceRecord& r)
   else
     r.qname=DNSName(row[6]);
 
-  r.qtype=row[3];
+  r.qtype = QType::fromString(row[3]);
 
   if (d_upgradeContent && DNSRecordContent::isUnknownType(row[3]) && DNSRecordContent::isRegisteredType(r.qtype, r.qclass)) {
     r.content = DNSRecordContent::upgradeContent(r.qname, r.qtype, row[0]);
@@ -2324,7 +2324,7 @@ void GSQLBackend::extractComment(SSqlStatement::row_t& row, Comment& comment)
 {
   pdns::checked_stoi_into(comment.domain_id, row[0]);
   comment.qname = DNSName(row[1]);
-  comment.qtype = row[2];
+  comment.qtype = QType::fromString(row[2]);
   pdns::checked_stoi_into(comment.modified_at, row[3]);
   comment.account = std::move(row[4]);
   comment.content = std::move(row[5]);

pdns/lua-auth4.cc

@@ -55,7 +55,7 @@ void AuthLua4::postPrepareContext() {
   d_lw->registerFunction<DNSPacket, void()>("clearRecords",[](DNSPacket &p){p.clearRecords();});
   d_lw->registerFunction<DNSPacket, void(DNSRecord&, bool)>("addRecord", [](DNSPacket &p, DNSRecord &dr, bool auth) { DNSZoneRecord dzr; dzr.dr = dr; dzr.auth = auth; p.addRecord(std::move(dzr)); });
   d_lw->registerFunction<DNSPacket, void(const vector<pair<unsigned int, DNSRecord> >&)>("addRecords", [](DNSPacket &p, const vector<pair<unsigned int, DNSRecord> >& records){ for(const auto &dr: records){ DNSZoneRecord dzr; dzr.dr = std::get<1>(dr); dzr.auth = true; p.addRecord(std::move(dzr)); }});
-  d_lw->registerFunction<DNSPacket, void(unsigned int, const DNSName&, const std::string&)>("setQuestion", [](DNSPacket &p, unsigned int opcode, const DNSName &name, const string &type){ QType qtype; qtype = type; p.setQuestion(static_cast<int>(opcode), name, static_cast<int>(qtype.getCode())); });
+  d_lw->registerFunction<DNSPacket, void(unsigned int, const DNSName&, const std::string&)>("setQuestion", [](DNSPacket &p, unsigned int opcode, const DNSName &name, const string &type){ QType qtype; qtype = QType::fromString(type); p.setQuestion(static_cast<int>(opcode), name, static_cast<int>(qtype.getCode())); });
   d_lw->registerFunction<DNSPacket, bool()>("isEmpty", [](DNSPacket &p){return p.isEmpty();});
   d_lw->registerFunction<DNSPacket, std::shared_ptr<DNSPacket>()>("replyPacket",[](DNSPacket& p){ return p.replyPacket();});
   d_lw->registerFunction<DNSPacket, bool()>("hasEDNSSubnet", [](DNSPacket &p){return p.hasEDNSSubnet();});
@@ -70,7 +70,7 @@ void AuthLua4::postPrepareContext() {
   d_lw->registerMember<DNSPacket, DNSName>("qdomainzone", [](const DNSPacket &p) -> DNSName { return p.qdomainzone; }, [](DNSPacket &p, const DNSName& name) { p.qdomainzone = name; });
 
   d_lw->registerMember<DNSPacket, std::string>("d_peer_principal", [](const DNSPacket &p) -> std::string { return p.d_peer_principal; }, [](DNSPacket &p, const std::string &princ) { p.d_peer_principal = princ; });
-  d_lw->registerMember<DNSPacket, const std::string>("qtype", [](const DNSPacket &p) ->  const std::string { return p.qtype.toString(); }, [](DNSPacket &p, const std::string &type) { p.qtype = type; });
+  d_lw->registerMember<DNSPacket, const std::string>("qtype", [](const DNSPacket &p) ->  const std::string { return p.qtype.toString(); }, [](DNSPacket &p, const std::string &type) { p.qtype = QType::fromString(type); });
 /* End of DNSPacket */
 
 

pdns/lua-base4.cc

@@ -82,7 +82,7 @@ void BaseLua4::prepareContext() {
   d_lw->writeFunction("newDRR", [](const DNSName& qname, const string& qtype, const unsigned int ttl, const string& content, boost::optional<int> domain_id, boost::optional<int> auth){
     auto drr = DNSResourceRecord();
     drr.qname = qname;
-    drr.qtype = qtype;
+    drr.qtype = QType::fromString(qtype);
     drr.ttl = ttl;
     drr.setContent(content);
     if (domain_id)
@@ -177,7 +177,7 @@ void BaseLua4::prepareContext() {
   d_lw->registerFunction<bool(cas_t::*)(const ComboAddress&)>("check",[](const cas_t& cas, const ComboAddress&ca) { return cas.count(ca)>0; });
 
   // QType
-  d_lw->writeFunction("newQType", [](const string& s) { QType q; q = s; return q; });
+  d_lw->writeFunction("newQType", [](const string& s) { QType q; q = QType::fromString(s); return q; });
   d_lw->registerFunction("getCode", &QType::getCode);
   d_lw->registerFunction("getName", &QType::toString);
   d_lw->registerEqFunction<bool(QType::*)(const QType&)>([](const QType& a, const QType& b){ return a == b;}); // operator overloading confuses LuaContext
@@ -216,7 +216,7 @@ void BaseLua4::prepareContext() {
   d_lw->registerFunction("match", (bool (NetmaskGroup::*)(const ComboAddress&) const)&NetmaskGroup::match);
 
   // DNSRecord
-  d_lw->writeFunction("newDR", [](const DNSName& name, const std::string& type, unsigned int ttl, const std::string& content, int place) { QType qtype; qtype = type; auto dr = DNSRecord(); dr.d_name = name; dr.d_type = qtype.getCode(); dr.d_ttl = ttl; dr.setContent(shared_ptr<DNSRecordContent>(DNSRecordContent::make(dr.d_type, QClass::IN, content))); dr.d_place = static_cast<DNSResourceRecord::Place>(place); return dr; });
+  d_lw->writeFunction("newDR", [](const DNSName& name, const std::string& type, unsigned int ttl, const std::string& content, int place) { QType qtype; qtype = QType::fromString(type); auto dr = DNSRecord(); dr.d_name = name; dr.d_type = qtype.getCode(); dr.d_ttl = ttl; dr.setContent(shared_ptr<DNSRecordContent>(DNSRecordContent::make(dr.d_type, QClass::IN, content))); dr.d_place = static_cast<DNSResourceRecord::Place>(place); return dr; });
   d_lw->registerMember("name", &DNSRecord::d_name);
   d_lw->registerMember("type", &DNSRecord::d_type);
   d_lw->registerMember("ttl", &DNSRecord::d_ttl);

pdns/pdnsutil.cc

@@ -1,4 +1,5 @@
 #include "dnsrecords.hh"
+#include "qtype.hh"
 #include <boost/smart_ptr/make_shared_array.hpp>
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -1497,7 +1498,7 @@ static int createZone(const DNSName &zone, const DNSName& nsname) {
   rr.qname = zone;
   rr.auth = true;
   rr.ttl = ::arg().asNum("default-ttl");
-  rr.qtype = "SOA";
+  rr.qtype = QType::fromString("SOA");
 
   string soa = ::arg()["default-soa-content"];
   boost::replace_all(soa, "@", zone.toStringNoDot());
@@ -1735,7 +1736,7 @@ static int deleteRRSet(const std::string& zone_, const std::string& name_, const
   else
     name=DNSName(name_)+zone;
 
-  QType qt(QType::chartocode(type_.c_str()));
+  QType qt(QType::fromString(type_));
   di.backend->startTransaction(zone, -1);
   di.backend->replaceRRSet(di.id, name, qt, vector<DNSResourceRecord>());
   di.backend->commitTransaction();

pdns/qtype.cc

@@ -19,12 +19,11 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
-#include "dns.hh"
-#include <iostream>
+
+#include <cstdint>
+#include <stdexcept>
 #include <string>
-#include <vector>
 #include <utility>
-#include <sstream>
 #include "qtype.hh"
 #include "misc.hh"
 
@@ -100,73 +99,76 @@ const map<const string, uint16_t> QType::names = {
 #endif
 };
 
-static map<uint16_t, const string> swapElements(const map<const string, uint16_t>& names) {
-  map<uint16_t, const string> ret;
-
-  for (const auto& n : names) {
-    ret.emplace(n.second, n.first);
-  }
-  return ret;
-}
-
-const map<uint16_t, const string> QType::numbers = swapElements(names);
-
-
-bool QType::isSupportedType() const
+static auto parsingError(const string& name) -> std::runtime_error
 {
-  return numbers.count(code) == 1;
+  string fullErrorMessage{"Unknown or invalid record type `"};
+  fullErrorMessage += name;
+  fullErrorMessage += "`";
+  return std::runtime_error{fullErrorMessage};
 }
 
-bool QType::isMetadataType() const
+static auto invalidEmptyRecordType() -> std::runtime_error
 {
-  // rfc6895 section 3.1, note ANY is 255 and falls outside the range
-  if (code == QType::OPT || (code >= rfc6895MetaLowerBound && code <= rfc6895MetaUpperBound)) {
-    return true;
-  }
-  return false;
+  string fullErrorMessage{"Invalid empty record type"};
+  return std::runtime_error{fullErrorMessage};
 }
 
-const string QType::toString() const
+auto QType::fromString(const string& name) -> QType
 {
-  const auto& name = numbers.find(code);
-  if (name != numbers.cend()) {
-    return name->second;
+  if (name.empty()) {
+    throw invalidEmptyRecordType();
   }
-  return "TYPE" + std::to_string(code);
-}
 
-uint16_t QType::chartocode(const char *p)
-{
-  string P = toUpper(p);
+  const auto& upperName = toUpper(name);
 
-  const auto& num = names.find(P);
+  const auto& num = names.find(upperName);
   if (num != names.cend()) {
     return num->second;
   }
-  if (*p == '#') {
-    return static_cast<uint16_t>(atoi(p + 1));
+
+  if (name.at(0) == '#') {
+    return pdns::checked_stoi<uint16_t>(&name[1]);
   }
 
-  if (boost::starts_with(P, "TYPE")) {
-    return static_cast<uint16_t>(atoi(p + 4));
+  if (boost::starts_with(upperName, "TYPE")) {
+    return pdns::checked_stoi<uint16_t>(&name[4]);
   }
 
-  return 0;
+  throw parsingError(name);
+}
+
+string QType::toString() const
+{
+  const auto& name = numbers.find(qtype);
+  if (name != numbers.cend()) {
+    return name->second;
+  }
+  return "TYPE" + std::to_string(qtype);
+}
+
+static map<uint16_t, const string> swapElements(const map<const string, uint16_t>& names)
+{
+  map<uint16_t, const string> ret;
+  for (const auto& name : names) {
+    ret.emplace(name.second, name.first);
+  }
+  return ret;
 }
 
-QType &QType::operator=(const char *p)
+const map<uint16_t, const string> QType::numbers = swapElements(names);
+
+bool QType::isSupportedType() const
 {
-  code = chartocode(p);
-  return *this;
+  return numbers.count(qtype) == 1;
 }
 
-QType &QType::operator=(const string &s)
+bool QType::isMetadataType() const
 {
-  code = chartocode(s.c_str());
-  return *this;
+  // rfc6895 section 3.1, note ANY is 255 and falls outside the range
+  return qtype == QType::OPT || (qtype >= rfc6895MetaLowerBound && qtype <= rfc6895MetaUpperBound);
 }
 
-const std::string QClass::toString() const
+std::string QClass::toString() const
 {
   switch (qclass) {
   case IN:
@@ -177,7 +179,7 @@ const std::string QClass::toString() const
     return "NONE";
   case ANY:
     return "ANY";
-  default :
+  default:
     return "CLASS" + std::to_string(qclass);
   }
 }