Pull request #627: Fix/AG-28943 : Allow mixed case in protocol

Merge in ADGUARD-CORE-LIBS/dns-libs from fix/AG-28943-2 to dev-2.5 Squashed commit of the following: commit 58fdecd91fb344880900ceffe0e95848ed32c4d5 Author: Zholboldu Emilbekuulu <[email protected]> Date: Mon Jan 15 18:33:55 2024 +0300 [WIP] AG-28943: Moved host to lowercase condition commit 61183700c97287ed3bf86292e2ba544968c312f1 Author: Sergey Fionov <[email protected]> Date: Thu Dec 21 08:35:39 2023 +0200 DoT fix commit 47252ba62aeac189d55d498d2698de9a5369ad60 Author: Sergey Fionov <[email protected]> Date: Thu Dec 21 07:43:19 2023 +0200 Allow mixed case in protocol
AdguardTeam · Jan 16, 2024 · 1d2db4f · 1d2db4f
1 parent d4ce1ab
commit 1d2db4f
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 31 deletions.
diff --git a/upstream/include/dns/upstream/upstream.h b/upstream/include/dns/upstream/upstream.h
@@ -184,7 +184,7 @@ class Upstream : public std::enable_shared_from_this<Upstream> {
         }
     };
 
-    Error<InitError> init_url_port(bool allow_creds, bool allow_path, uint16_t default_port);
+    Error<InitError> init_url_port(bool allow_creds, bool allow_path, uint16_t default_port, bool host_to_lowercase);
 
     /** Upstream options */
     UpstreamOptions m_options;

diff --git a/upstream/test/test_upstream_utils.cpp b/upstream/test/test_upstream_utils.cpp
@@ -46,4 +46,9 @@ TEST_F(UpstreamUtilsTest, InvalidUpstreamOfflineUnknownScheme) {
     ASSERT_TRUE(err) << "Cannot be successful";
 }
 
+TEST_F(UpstreamUtilsTest, ValidUpstreamMixedCase) {
+    auto err = dns::test_upstream({"Https://dns.Adguard-Dns.com/dns-query", {"8.8.8.8"}}, timeout, false, nullptr, true);
+    ASSERT_FALSE(err) << "Cannot fail: " << err->str();
+}
+
 } // namespace ag::dns::upstream::test
diff --git a/upstream/upstream.cpp b/upstream/upstream.cpp
@@ -67,7 +67,7 @@ struct UpstreamFactory::Impl {
 static auto get_address_scheme_iterator(std::string_view address) {
     using namespace std::placeholders;
     return std::find_if(std::begin(SCHEME_WITH_SUFFIX), std::end(SCHEME_WITH_SUFFIX), [&](auto scheme) {
-        return address.starts_with(scheme);
+        return utils::istarts_with(address, scheme);
     });
 }
 
@@ -228,7 +228,7 @@ UpstreamFactory::CreateResult UpstreamFactory::create_upstream(const UpstreamOpt
     return result;
 }
 
-Error<Upstream::InitError> Upstream::init_url_port(bool allow_creds, bool allow_path, uint16_t default_port) {
+Error<Upstream::InitError> Upstream::init_url_port(bool allow_creds, bool allow_path, uint16_t default_port, bool host_to_lowercase) {
     auto url = ada::parse<ada::url_aggregator>(m_options.address, nullptr);
     if (!url) {
         return make_error(InitError::AE_INVALID_ADDRESS, "Invalid URL");
@@ -245,8 +245,13 @@ Error<Upstream::InitError> Upstream::init_url_port(bool allow_creds, bool allow_
     if (!allow_path && !url->get_pathname().empty() && url->get_pathname() != "/") {
         return make_error(InitError::AE_INVALID_ADDRESS, "Unexpected path");
     }
-    if (allow_path && url->get_pathname().empty()) {
-        url->set_pathname("/");
+    if (!url->is_special()) {
+        if (allow_path && url->get_pathname().empty()) {
+            url->set_pathname("/");
+        }
+    }
+    if (host_to_lowercase) {
+        url->set_host(utils::to_lower(url->get_host()));
     }
     uint16_t port = url->get_port().empty()
             ? default_port

diff --git a/upstream/upstream_doh.cpp b/upstream/upstream_doh.cpp
@@ -267,7 +267,7 @@ ag::dns::DohUpstream::~DohUpstream() {
 }
 
 ag::Error<ag::dns::Upstream::InitError> ag::dns::DohUpstream::init() {
-    auto error = this->init_url_port(/*allow_creds*/ true, /*allow_path*/ true, DEFAULT_DOH_PORT);
+    auto error = this->init_url_port(/*allow_creds*/ true, /*allow_path*/ true, DEFAULT_DOH_PORT, /*host_to_lowercase*/ true);
     if (error) {
         return error;
     }
@@ -306,7 +306,7 @@ ag::Error<ag::dns::Upstream::InitError> ag::dns::DohUpstream::init() {
     m_path = m_url.get_pathname();
     log_upstream(dbg, this, "Prepared request template: {}", m_request_template);
 
-    if (m_options.address.starts_with(DohUpstream::SCHEME_H3)) {
+    if (utils::istarts_with(m_options.address, DohUpstream::SCHEME_H3)) {
         m_http_version = http::HTTP_3_0;
     } else if (!m_config.enable_http3) {
         m_http_version = http::HTTP_2_0;

diff --git a/upstream/upstream_doq.cpp b/upstream/upstream_doq.cpp
@@ -237,7 +237,7 @@ void DoqUpstream::send_requests() {
 }
 
 Error<Upstream::InitError> DoqUpstream::init() {
-    auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_DOQ_PORT);
+    auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_DOQ_PORT, /*host_to_lowercase*/ true);
     if (error) {
         return error;
     }

diff --git a/upstream/upstream_dot.cpp b/upstream/upstream_dot.cpp
@@ -59,7 +59,7 @@ class DotConnection : public DnsFramedConnection {
             m_stream = upstream->make_secured_socket(utils::TP_TCP,
                     SocketFactory::SecureSocketParameters{
                             .session_cache = &upstream->m_tls_session_cache,
-                            .server_name = upstream->m_server_name.empty() ? addr.host_str() : upstream->m_server_name,
+                            .server_name{upstream->m_url.get_hostname()},
                             .alpn = {DOT_ALPN},
                             .fingerprints = upstream->m_fingerprints,
                     });
@@ -105,21 +105,6 @@ class DotConnection : public DnsFramedConnection {
     Bootstrapper::ResolveResult m_result;
 };
 
-static std::string_view strip_dot_url(std::string_view url) {
-    assert(url.starts_with(DotUpstream::SCHEME));
-    url.remove_prefix(DotUpstream::SCHEME.length());
-    url = url.substr(0, url.find('/'));
-    return url;
-}
-
-static Result<std::string_view, Upstream::InitError> get_host_name(std::string_view url) {
-    auto split_result = utils::split_host_port(strip_dot_url(url));
-    if (split_result.has_error()) {
-        return make_error(Upstream::InitError::AE_INVALID_ADDRESS);
-    }
-    return utils::trim(split_result.value().first);
-}
-
 static Result<BootstrapperPtr, Upstream::InitError> create_bootstrapper(
         const UpstreamOptions &opts, const UpstreamFactoryConfig &config,
         const ada::url_aggregator url, uint16_t port) {
@@ -142,13 +127,10 @@ DotUpstream::DotUpstream(const UpstreamOptions &opts, const UpstreamFactoryConfi
         , m_tls_session_cache(opts.address)
         , m_fingerprints(std::move(fingerprints))
 {
-    if (auto host = get_host_name(opts.address); !host.has_error()) {
-        m_server_name = host.value();
-    }
 }
 
 Error<Upstream::InitError> DotUpstream::init() {
-    auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_DOT_PORT);
+    auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_DOT_PORT, /*host_to_lowercase*/ true);
     if (error) {
         return error;
     }

diff --git a/upstream/upstream_dot.h b/upstream/upstream_dot.h
@@ -40,8 +40,6 @@ class DotUpstream : public Upstream {
     Logger m_log;
     /** TLS connection pool */
     ConnectionPoolPtr m_pool;
-    /** DNS server name */
-    std::string m_server_name;
     /** TLS sessions cache */
     TlsSessionCache m_tls_session_cache;
     /** Bootstrapper */

diff --git a/upstream/upstream_plain.cpp b/upstream/upstream_plain.cpp
@@ -20,7 +20,7 @@ PlainUpstream::PlainUpstream(const UpstreamOptions &opts, const UpstreamFactoryC
 
 Error<Upstream::InitError> PlainUpstream::init() {
     if (m_options.address.find("://") != std::string::npos) {
-        auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_PLAIN_PORT);
+        auto error = this->init_url_port(/*allow_creds*/ false, /*allow_path*/ false, DEFAULT_PLAIN_PORT, /*host_to_lowercase*/ true);
         if (error) {
             return error;
         }