Merge pull request #290

fee8424 Allow name@domain.tld for OpenAlias lookups (warptangent)
a0fe18f Revert "Allow name@domain.tld for OpenAlias lookups" (warptangent)

5 files changed, 57 insertions, 26 deletions

diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp
index 347cf758a..98e3d0ea6 100644
--- a/src/common/dns_utils.cpp
+++ b/src/common/dns_utils.cpp
@@ -205,15 +205,13 @@ std::vector<std::string> DNSResolver::get_ipv4(const std::string& url, bool& dns
   dnssec_valid = false;
   char urlC[1000];  // waaaay too big, but just in case...
 
-  std::string url_copy{url};
-  if (!check_address_syntax(url_copy))
+  strncpy(urlC, url.c_str(), 999);
+  urlC[999] = '\0';
+  if (!check_address_syntax(urlC))
   {
     return addresses;
   }
 
-  strncpy(urlC, url_copy.c_str(), 999);
-  urlC[999] = '\0';
-
   // destructor takes care of cleanup
   ub_result_ptr result;
 
@@ -241,15 +239,14 @@ std::vector<std::string> DNSResolver::get_ipv6(const std::string& url, bool& dns
   dnssec_valid = false;
   char urlC[1000];  // waaaay too big, but just in case...
 
-  std::string url_copy{url};
-  if (!check_address_syntax(url_copy))
+  strncpy(urlC, url.c_str(), 999);
+  urlC[999] = '\0';
+
+  if (!check_address_syntax(urlC))
   {
     return addresses;
   }
 
-  strncpy(urlC, url_copy.c_str(), 999);
-  urlC[999] = '\0';
-
   ub_result_ptr result;
 
   // call DNS resolver, blocking.  if return value not zero, something went wrong
@@ -276,15 +273,14 @@ std::vector<std::string> DNSResolver::get_txt_record(const std::string& url, boo
   dnssec_valid = false;
   char urlC[1000];  // waaaay too big, but just in case...
 
-  std::string url_copy{url};
-  if (!check_address_syntax(url_copy))
+  strncpy(urlC, url.c_str(), 999);
+  urlC[999] = '\0';
+
+  if (!check_address_syntax(urlC))
   {
     return records;
   }
 
-  strncpy(urlC, url_copy.c_str(), 999);
-  urlC[999] = '\0';
-
   ub_result_ptr result;
 
   // call DNS resolver, blocking.  if return value not zero, something went wrong
@@ -308,6 +304,19 @@ std::vector<std::string> DNSResolver::get_txt_record(const std::string& url, boo
   return records;
 }
 
+std::string DNSResolver::get_dns_format_from_oa_address(const std::string& oa_addr)
+{
+  std::string addr(oa_addr);
+  auto first_at = addr.find("@");
+  if (first_at == std::string::npos)
+    return addr;
+
+  // convert name@domain.tld to name.domain.tld
+  addr.replace(first_at, 1, ".");
+
+  return addr;
+}
+
 DNSResolver& DNSResolver::instance()
 {
   static DNSResolver* staticInstance = NULL;
@@ -318,17 +327,13 @@ DNSResolver& DNSResolver::instance()
   return *staticInstance;
 }
 
-bool DNSResolver::check_address_syntax(std::string& addr)
+bool DNSResolver::check_address_syntax(const std::string& addr)
 {
   // if string doesn't contain a dot, we won't consider it a url for now.
-  auto first_dot = addr.find(".");
-  if (first_dot == std::string::npos)
+  if (addr.find(".") == std::string::npos)
   {
     return false;
   }
-
-  // allow name@domain.tld to work
-  addr.replace(first_dot, 1, "@");
   return true;
 }
 
diff --git a/src/common/dns_utils.h b/src/common/dns_utils.h
index 4e48acb09..d98523a30 100644
--- a/src/common/dns_utils.h
+++ b/src/common/dns_utils.h
@@ -101,6 +101,18 @@ public:
    std::vector<std::string> get_txt_record(const std::string& url, bool& dnssec_available, bool& dnssec_valid);
 
   /**
+   * @brief Gets a DNS address from OpenAlias format
+   *
+   * If the address looks good, but contains one @ symbol, replace that with a .
+   * e.g. donate@getmonero.org becomes donate.getmonero.org
+   *
+   * @param oa_addr  OpenAlias address
+   *
+   * @return dns_addr  DNS address
+   */
+  std::string get_dns_format_from_oa_address(const std::string& oa_addr);
+
+  /**
    * @brief Gets the singleton instance of DNSResolver
    *
    * @return returns a pointer to the singleton
@@ -112,14 +124,11 @@ private:
   /**
    * @brief Checks a string to see if it looks like a URL
    *
-   * If the address looks good, but contains one @ symbol, replace that with a .
-   * e.g. donate@getmonero.org becomes donate.getmonero.org
-   *
    * @param addr the string to be checked
    *
    * @return true if it looks enough like a URL, false if not
    */
-  bool check_address_syntax(std::string& addr);
+  bool check_address_syntax(const std::string& addr);
 
   DNSResolverData *m_data;
 }; // class DNSResolver
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 6698e7296..7928e1c62 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -1016,7 +1016,8 @@ std::vector<std::string> wallet2::addresses_from_url(const std::string& url, boo
   std::vector<std::string> addresses;
   // get txt records
   bool dnssec_available, dnssec_isvalid;
-  auto records = tools::DNSResolver::instance().get_txt_record(url, dnssec_available, dnssec_isvalid);
+  std::string oa_addr = tools::DNSResolver::instance().get_dns_format_from_oa_address(url);
+  auto records = tools::DNSResolver::instance().get_txt_record(oa_addr, dnssec_available, dnssec_isvalid);
 
   // TODO: update this to allow for conveying that dnssec was not available
   if (dnssec_available && dnssec_isvalid)
diff --git a/tests/unit_tests/address_from_url.cpp b/tests/unit_tests/address_from_url.cpp
index fe2f072de..99df75d74 100644
--- a/tests/unit_tests/address_from_url.cpp
+++ b/tests/unit_tests/address_from_url.cpp
@@ -93,6 +93,14 @@ TEST(AddressFromURL, Success)
   {
     EXPECT_STREQ(addr.c_str(), addresses[0].c_str());
   }
+
+  // OpenAlias address with an @ instead of first .
+  addresses = tools::wallet2::addresses_from_url("donate@getmonero.org", dnssec_result);
+  EXPECT_EQ(1, addresses.size());
+  if (addresses.size() == 1)
+  {
+    EXPECT_STREQ(addr.c_str(), addresses[0].c_str());
+  }
 }
 
 TEST(AddressFromURL, Failure)
diff --git a/tests/unit_tests/dns_resolver.cpp b/tests/unit_tests/dns_resolver.cpp
index 6717e990a..0709a773f 100644
--- a/tests/unit_tests/dns_resolver.cpp
+++ b/tests/unit_tests/dns_resolver.cpp
@@ -148,4 +148,12 @@ TEST(DNSResolver, GetTXTRecord)
   {
     std::cout << "TXT record for donate.getmonero.org: " << rec << std::endl;
   }
+
+  // replace first @ with .
+  std::string addr = tools::DNSResolver::instance().get_dns_format_from_oa_address("donate@getmonero.org");
+  EXPECT_STREQ("donate.getmonero.org", addr.c_str());
+
+  // no change
+  addr = tools::DNSResolver::instance().get_dns_format_from_oa_address("donate.getmonero.org");
+  EXPECT_STREQ("donate.getmonero.org", addr.c_str());
 }