From dd16158081777c0402412912d7da71cad202982b Mon Sep 17 00:00:00 2001
From: Jason Rhinelander <jason@imaginary.ca>
Date: Fri, 9 Sep 2022 23:44:21 -0300
Subject: [PATCH] DNS: default to 127.3.2.1 & high port on Linux

Lots of tools struggle with non-default DNS port, so keep a listener on
127.3.2.1:53 (by default).

This required various changes to the config handling to hold a vector
(instead of an optional) of defaults and values, and now allows passing
in an array of defaults instead of just a single default.
---
 llarp/config/config.cpp     |  19 +++--
 llarp/config/definition.cpp |  30 +++----
 llarp/config/definition.hpp | 159 +++++++++++++++++++-----------------
 3 files changed, 110 insertions(+), 98 deletions(-)

diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp
index 195b96f4d..9826e98f6 100644
--- a/llarp/config/config.cpp
+++ b/llarp/config/config.cpp
@@ -776,17 +776,18 @@ namespace llarp
     // Most non-linux platforms have loopback as 127.0.0.1/32, but linux uses 127.0.0.1/8 so that we
     // can bind to other 127.* IPs to avoid conflicting with something else that may be listening on
     // 127.0.0.1:53.
+    constexpr std::array DefaultDNSBind{
 #ifdef __linux__
 #ifdef WITH_SYSTEMD
-    // when we have systemd support add a random high port on loopback
-    // see https://github.com/oxen-io/lokinet/issues/1887#issuecomment-1091897282
-    constexpr Default DefaultDNSBind{"127.0.0.1:0"};
-#else
-    constexpr Default DefaultDNSBind{"127.3.2.1:53"};
+        // when we have systemd support add a random high port on loopback as well
+        // see https://github.com/oxen-io/lokinet/issues/1887#issuecomment-1091897282
+        Default{"127.0.0.1:0"},
 #endif
+        Default{"127.3.2.1:53"},
 #else
-    constexpr Default DefaultDNSBind{"127.0.0.1:53"};
+        Default{"127.0.0.1:53"},
 #endif
+    };
 
     // Default, but if we get any upstream (including upstream=, i.e. empty string) we clear it
     constexpr Default DefaultUpstreamDNS{"9.9.9.10:53"};
@@ -829,9 +830,10 @@ namespace llarp
         "query-bind",
 #ifdef __APPLE__
         Default{"127.0.0.1:1253"},
-#endif
-#ifdef _WIN32
+#elif defined(_WIN32)
         Default{"0.0.0.0:0"},
+#else
+        Hidden,
 #endif
         Comment{
             "Address to bind to for sending upstream DNS requests.",
@@ -842,6 +844,7 @@ namespace llarp
         "dns",
         "bind",
         DefaultDNSBind,
+        MultiValue,
         Comment{
             "Address to bind to for handling DNS requests.",
         },
diff --git a/llarp/config/definition.cpp b/llarp/config/definition.cpp
index ccf885c15..aa199ebea 100644
--- a/llarp/config/definition.cpp
+++ b/llarp/config/definition.cpp
@@ -2,7 +2,6 @@
 #include <llarp/util/logging.hpp>
 
 #include <iterator>
-#include <sstream>
 #include <stdexcept>
 #include <cassert>
 
@@ -176,12 +175,14 @@ namespace llarp
   std::string
   ConfigDefinition::generateINIConfig(bool useValues)
   {
-    std::ostringstream oss;
+    std::string ini;
+    auto ini_append = std::back_inserter(ini);
 
     int sectionsVisited = 0;
 
     visitSections([&](const std::string& section, const DefinitionMap&) {
-      std::ostringstream sect_out;
+      std::string sect_str;
+      auto sect_append = std::back_inserter(sect_str);
 
       visitDefinitions(section, [&](const std::string& name, const OptionDefinition_ptr& def) {
         bool has_comment = false;
@@ -190,44 +191,43 @@ namespace llarp
         //       (i.e. those handled by UndeclaredValueHandler's)
         for (const std::string& comment : m_definitionComments[section][name])
         {
-          sect_out << "\n# " << comment;
+          fmt::format_to(sect_append, "\n# {}", comment);
           has_comment = true;
         }
 
         if (useValues and def->getNumberFound() > 0)
         {
-          sect_out << "\n" << name << "=" << def->valueAsString(false) << "\n";
+          for (const auto& val : def->valuesAsString())
+            fmt::format_to(sect_append, "\n{}={}\n", name, val);
         }
         else if (not(def->hidden and not has_comment))
         {
-          sect_out << "\n";
-          if (not def->required)
-            sect_out << "#";
-          sect_out << name << "=" << def->defaultValueAsString() << "\n";
+          for (const auto& val : def->defaultValuesAsString())
+            fmt::format_to(sect_append, "\n{}{}={}\n", def->required ? "" : "#", name, val);
         }
       });
 
-      auto sect_str = sect_out.str();
       if (sect_str.empty())
         return;  // Skip sections with no options
 
       if (sectionsVisited > 0)
-        oss << "\n\n";
+        ini += "\n\n";
 
-      oss << "[" << section << "]\n";
+      fmt::format_to(ini_append, "[{}]\n", section);
 
       // TODO: this will create empty objects as a side effect of map's operator[]
       // TODO: this also won't handle sections which have no definition
       for (const std::string& comment : m_sectionComments[section])
       {
-        oss << "# " << comment << "\n";
+        fmt::format_to(ini_append, "# {}\n", comment);
       }
-      oss << "\n" << sect_str;
+      ini += "\n";
+      ini += sect_str;
 
       sectionsVisited++;
     });
 
-    return oss.str();
+    return ini;
   }
 
   const OptionDefinition_ptr&
diff --git a/llarp/config/definition.hpp b/llarp/config/definition.hpp
index b3f9cd0ab..8f9225c28 100644
--- a/llarp/config/definition.hpp
+++ b/llarp/config/definition.hpp
@@ -97,12 +97,19 @@ namespace llarp
     template <typename U>
     constexpr bool is_default<U&> = is_default<remove_cvref_t<U>>;
 
+    template <typename T>
+    constexpr bool is_default_array = false;
+    template <typename T, size_t N>
+    constexpr bool is_default_array<std::array<Default<T>, N>> = true;
+    template <typename U>
+    constexpr bool is_default_array<U&> = is_default_array<remove_cvref_t<U>>;
+
     template <typename T, typename Option>
     constexpr bool is_option =
         std::is_base_of_v<
             option_flag,
             remove_cvref_t<
-                Option>> or std::is_same_v<Comment, Option> or is_default<Option> or std::is_invocable_v<remove_cvref_t<Option>, T>;
+                Option>> or std::is_same_v<Comment, Option> or is_default<Option> or is_default_array<Option> or std::is_invocable_v<remove_cvref_t<Option>, T>;
   }  // namespace config
 
   /// A base class for specifying config options and their constraints. The basic to/from string
@@ -128,8 +135,8 @@ namespace llarp
     /// Subclasses should provide their default value as a string
     ///
     /// @return the option's default value represented as a string
-    virtual std::string
-    defaultValueAsString() = 0;
+    virtual std::vector<std::string>
+    defaultValuesAsString() = 0;
 
     /// Subclasses should parse and store the provided input
     ///
@@ -143,13 +150,11 @@ namespace llarp
     virtual size_t
     getNumberFound() const = 0;
 
-    /// Subclasess should write their parsed value as a string, optionally falling back to any
-    /// specified default if `useDefault` is true.
+    /// Subclasess should write their parsed values as strings.
     ///
-    /// @param useDefault should specify whether to fallback to default when possible
-    /// @return the option's value as a string
-    virtual std::string
-    valueAsString(bool useDefault) = 0;
+    /// @return the option's value(s) as strings
+    virtual std::vector<std::string>
+    valuesAsString() = 0;
 
     /// Subclassess should call their acceptor, if present. See OptionDefinition for more details.
     ///
@@ -170,9 +175,9 @@ namespace llarp
     std::vector<std::string> comments;
   };
 
-  /// The primary type-aware implementation of OptionDefinitionBase, this templated class allows
-  /// for implementations which can use the std::ostringstream and std::istringstream for to/from
-  /// string functionality.
+  /// The primary type-aware implementation of OptionDefinitionBase, this templated class allows for
+  /// implementations which can use fmt::format for conversion to string and std::istringstream for
+  /// input from string.
   ///
   /// Note that types (T) used as template parameters here must be used verbatim when calling
   /// ConfigDefinition::getConfigValue(). Similar types such as uint32_t and int32_t cannot be
@@ -184,7 +189,7 @@ namespace llarp
     ///
     /// @param defaultValue_ is used in the following situations:
     /// 1) as the return value for getValue() if there is no parsed value and required==false
-    /// 2) as the output in defaultValueAsString(), used to generate config files
+    /// 2) as the output in defaultValuesAsString(), used to generate config files
     /// 3) as the output in valueAsString(), used to generate config files
     ///
     /// @param opts - 0 or more of config::Required, config::Hidden, config::Default{...}, etc.
@@ -202,18 +207,28 @@ namespace llarp
       (extractComments(std::forward<Options>(opts)), ...);
     }
 
-    /// Extracts a default value from an config::Default<U>; ignores anything that isn't an
-    /// config::Default<U>.
+    /// Extracts a default value from an config::Default<U> or an array of defaults (for
+    /// multi-valued options with multi-value default); ignores anything else.
     template <typename U>
     void
     extractDefault(U&& defaultValue_)
     {
-      if constexpr (config::is_default<U>)
+      if constexpr (config::is_default_array<U>)
+      {
+        if (!multiValued)
+          throw std::logic_error{"Array config defaults require multiValue mode"};
+
+        defaultValues.clear();
+        defaultValues.reserve(defaultValue_.size());
+        for (const auto& def : defaultValue_)
+          defaultValues.push_back(def.val);
+      }
+      else if constexpr (config::is_default<U>)
       {
         static_assert(
             std::is_convertible_v<decltype(std::forward<U>(defaultValue_).val), T>,
             "Cannot convert given llarp::config::Default to the required value type");
-        defaultValue = std::forward<U>(defaultValue_).val;
+        defaultValues = {std::forward<U>(defaultValue_).val};
       }
     }
 
@@ -236,33 +251,22 @@ namespace llarp
         comments = std::forward<U>(comment).comments;
     }
 
-    /// Returns the first parsed value, if available. Otherwise, provides the default value if the
-    /// option is not required. Otherwise, returns an empty optional.
+    /// Returns the first parsed value, if available. Otherwise, provides the (first) default value
+    /// if the option is not required. Otherwise, returns an empty optional.
     ///
-    /// @return an optional with the parsed value, the default value, or no value.
+    /// @return an optional with the parsed value, the (first) default value, or no value.
     std::optional<T>
     getValue() const
     {
       if (parsedValues.empty())
-        return required ? std::nullopt : defaultValue;
+      {
+        if (required || defaultValues.empty())
+          return std::nullopt;
+        return defaultValues.front();
+      }
       return parsedValues.front();
     }
 
-    /// Returns the value at the given index.
-    ///
-    /// @param index
-    /// @return the value at the given index, if it exists
-    /// @throws range_error exception if index >= size
-    T
-    getValueAt(size_t index) const
-    {
-      if (index >= parsedValues.size())
-        throw std::range_error{
-            fmt::format("no value at index {}, size: {}", index, parsedValues.size())};
-
-      return parsedValues[index];
-    }
-
     /// Returns the number of values found.
     ///
     /// @return number of values found
@@ -272,18 +276,21 @@ namespace llarp
       return parsedValues.size();
     }
 
-    std::string
-    defaultValueAsString() override
+    std::vector<std::string>
+    defaultValuesAsString() override
     {
-      if (!defaultValue)
-        return "";
-
+      if (defaultValues.empty())
+        return {};
       if constexpr (std::is_same_v<fs::path, T>)
-        return defaultValue->string();
-
-      std::ostringstream oss;
-      oss << *defaultValue;
-      return oss.str();
+        return {{defaultValues.front().u8string()}};
+      else
+      {
+        std::vector<std::string> def_strs;
+        def_strs.reserve(defaultValues.size());
+        for (const auto& v : defaultValues)
+          def_strs.push_back(fmt::format("{}", v));
+        return def_strs;
+      }
     }
 
     void
@@ -317,21 +324,21 @@ namespace llarp
       }
     }
 
-    std::string
-    valueAsString(bool useDefault) override
+    std::vector<std::string>
+    valuesAsString() override
     {
-      std::ostringstream oss;
-      if (parsedValues.size() > 0)
-        oss << parsedValues[0];
-      else if (useDefault and defaultValue)
-        oss << *defaultValue;
-
-      return oss.str();
+      if (parsedValues.empty())
+        return {};
+      std::vector<std::string> result;
+      result.reserve(parsedValues.size());
+      for (const auto& v : parsedValues)
+        result.push_back(fmt::format("{}", v));
+      return result;
     }
 
-    /// Attempts to call the acceptor function, if present. This function may throw if the value is
-    /// not acceptable. Additionally, tryAccept should not be called if the option is required and
-    /// no value has been provided.
+    /// Attempts to call the acceptor function, if present. This function may throw if the value
+    /// is not acceptable. Additionally, tryAccept should not be called if the option is required
+    /// and no value has been provided.
     ///
     /// @throws if required and no value present or if the acceptor throws
     void
@@ -350,8 +357,9 @@ namespace llarp
         if (multiValued)
         {
           // add default value in multi value mode
-          if (defaultValue and parsedValues.empty())
-            acceptor(*defaultValue);
+          if (parsedValues.empty() and not defaultValues.empty())
+            for (const auto& v : defaultValues)
+              acceptor(v);
 
           for (auto value : parsedValues)
           {
@@ -367,7 +375,7 @@ namespace llarp
       }
     }
 
-    std::optional<T> defaultValue;
+    std::vector<T> defaultValues;
     std::vector<T> parsedValues;
     std::function<void(T)> acceptor;
   };
@@ -389,8 +397,8 @@ namespace llarp
   // map of section-name to map-of-definitions
   using SectionMap = std::unordered_map<std::string, DefinitionMap>;
 
-  /// A ConfigDefinition holds an ordered set of OptionDefinitions defining the allowable values and
-  /// their constraints (specified through calls to defineOption()).
+  /// A ConfigDefinition holds an ordered set of OptionDefinitions defining the allowable values
+  /// and their constraints (specified through calls to defineOption()).
   ///
   /// The layout and grouping of the config options are modelled after the INI file format; each
   /// option has a name and is grouped under a section. Duplicate option names are allowed only if
@@ -400,9 +408,9 @@ namespace llarp
   /// Configured values (e.g. those encountered when parsing a file) can be provided through calls
   /// to addConfigValue(). These take a std::string as a value, which is automatically parsed.
   ///
-  /// The ConfigDefinition can be used to print out a full config string (or file), including fields
-  /// with defaults and optionally fields which have a specified value (values provided through
-  /// calls to addConfigValue()).
+  /// The ConfigDefinition can be used to print out a full config string (or file), including
+  /// fields with defaults and optionally fields which have a specified value (values provided
+  /// through calls to addConfigValue()).
   struct ConfigDefinition
   {
     explicit ConfigDefinition(bool relay) : relay{relay}
@@ -411,8 +419,8 @@ namespace llarp
     /// Specify the parameters and type of a configuration option. The parameters are members of
     /// OptionDefinitionBase; the type is inferred from OptionDefinition's template parameter T.
     ///
-    /// This function should be called for every option that this Configuration supports, and should
-    /// be done before any other interactions involving that option.
+    /// This function should be called for every option that this Configuration supports, and
+    /// should be done before any other interactions involving that option.
     ///
     /// @param def should be a unique_ptr to a valid subclass of OptionDefinitionBase
     /// @return `*this` for chaining calls
@@ -420,8 +428,8 @@ namespace llarp
     ConfigDefinition&
     defineOption(OptionDefinition_ptr def);
 
-    /// Convenience function which calls defineOption with a OptionDefinition of the specified type
-    /// and with parameters passed through to OptionDefinition's constructor.
+    /// Convenience function which calls defineOption with a OptionDefinition of the specified
+    /// type and with parameters passed through to OptionDefinition's constructor.
     template <typename T, typename... Params>
     ConfigDefinition&
     defineOption(Params&&... args)
@@ -433,8 +441,9 @@ namespace llarp
     /// representing the type used by the option (e.g. the type provided when defineOption() was
     /// called).
     ///
-    /// If the specified option doesn't exist, an exception will be thrown. Otherwise, the option's
-    /// parseValue() will be invoked, and should throw an exception if the string can't be parsed.
+    /// If the specified option doesn't exist, an exception will be thrown. Otherwise, the
+    /// option's parseValue() will be invoked, and should throw an exception if the string can't
+    /// be parsed.
     ///
     /// @param section is the section this value resides in
     /// @param name is the name of the value
@@ -493,9 +502,9 @@ namespace llarp
     void
     validateRequiredFields();
 
-    /// Accept all options. This will call the acceptor (if present) on each option. Note that this
-    /// should only be called if all required fields are present (that is, validateRequiredFields()
-    /// has been or could be called without throwing).
+    /// Accept all options. This will call the acceptor (if present) on each option. Note that
+    /// this should only be called if all required fields are present (that is,
+    /// validateRequiredFields() has been or could be called without throwing).
     ///
     /// @throws if any option's acceptor throws
     void