Merge pull request #1385 from majestrate/default-upstream-dns-2020-10-02

set Default upstream dns

llarp/config/config.cpp

@@ -355,19 +355,38 @@ namespace llarp
   DnsConfig::defineConfigOptions(ConfigDefinition& conf, const ConfigGenParameters& params)
   {
     (void)params;
-    auto parseDNSAddr = [](auto arg) {
-      IpAddress addr{arg};
-      if (not addr.getPort())
-        addr.setPort(53);
-      return addr;
-    };
+
+    // Default, but if we get any upstream (including upstream=, i.e. empty string) we clear it
+    constexpr auto DefaultUpstreamDNS = "1.1.1.1";
+    m_upstreamDNS.emplace_back(DefaultUpstreamDNS);
+
     conf.defineOption<std::string>(
-        "dns", "upstream", false, true, std::nullopt, [=](std::string arg) {
-          m_upstreamDNS.push_back(parseDNSAddr(std::move(arg)));
+        "dns",
+        "upstream",
+        false,
+        true,
+        DefaultUpstreamDNS,
+        [=, first = true](std::string arg) mutable {
+          if (first)
+          {
+            m_upstreamDNS.clear();
+            first = false;
+          }
+          if (!arg.empty())
+          {
+            auto& addr = m_upstreamDNS.emplace_back(std::move(arg));
+            if (auto p = addr.getPort(); p && *p != 53)
+              // unbound doesn't support non-default ports so bail if the user gave one
+              throw std::invalid_argument(
+                  "Invalid [dns] upstream setting: non-default DNS ports are not supported");
+            addr.setPort(std::nullopt);
+          }
         });
 
     conf.defineOption<std::string>("dns", "bind", false, "127.3.2.1:53", [=](std::string arg) {
-      m_bind = parseDNSAddr(std::move(arg));
+      m_bind = IpAddress{std::move(arg)};
+      if (!m_bind.getPort())
+        m_bind.setPort(53);
     });
 
     // Ignored option (used by the systemd service file to disable resolvconf configuration).

llarp/config/ini.cpp

@@ -108,24 +108,19 @@ namespace llarp
       else if (kvDelim != std::string_view::npos)
       {
         // key value pair
-        std::string_view::size_type k_start = 0;
-        std::string_view::size_type k_end = kvDelim;
-        std::string_view::size_type v_start = kvDelim + 1;
-        std::string_view::size_type v_end = realLine.size() - 1;
+        std::string_view k = realLine.substr(0, kvDelim);
+        std::string_view v = realLine.substr(kvDelim + 1);
+
         // clamp whitespaces
-        while (whitespace(realLine[k_start]) && k_start != kvDelim)
-          ++k_start;
-        while (whitespace(realLine[k_end - 1]) && k_end != k_start)
-          --k_end;
-        while (whitespace(realLine[v_start]) && v_start != v_end)
-          ++v_start;
-        while (whitespace(realLine[v_end]))
-          --v_end;
-
-        // sect.k = v
-        std::string_view k = realLine.substr(k_start, k_end - k_start);
-        std::string_view v = realLine.substr(v_start, 1 + (v_end - v_start));
-        if (k.size() == 0 || v.size() == 0)
+        for (auto* x : {&k, &v})
+        {
+          while (!x->empty() && whitespace(x->front()))
+            x->remove_prefix(1);
+          while (!x->empty() && whitespace(x->back()))
+            x->remove_suffix(1);
+        }
+
+        if (k.size() == 0)
         {
           LogError(m_FileName, " invalid line (", lineno, "): '", line, "'");
           return false;

llarp/dns/server.cpp

@@ -124,9 +124,9 @@ namespace llarp
       }
       for (const auto& resolver : resolvers)
       {
-        if (not m_UnboundResolver->AddUpstreamResolver(resolver.toString()))
+        if (not m_UnboundResolver->AddUpstreamResolver(resolver.toHost()))
         {
-          llarp::LogError("Failed to add upstream DNS server: ", resolver.toString());
+          llarp::LogError("Failed to add upstream DNS server: ", resolver.toHost());
           m_UnboundResolver = nullptr;
           return false;
         }