refactor: add a tr_urlParse() with no heap allocs (#2070)

libtransmission/utils.cc

@@ -830,7 +830,7 @@ bool tr_urlIsValidTracker(char const* url)
 
     size_t const url_len = strlen(url);
 
-    return isValidURLChars(url, url_len) && tr_urlParse(url, url_len, nullptr, nullptr, nullptr, nullptr) &&
+    return isValidURLChars(url, url_len) && tr_urlParse({ url, url_len }) &&
         (memcmp(url, "http://", 7) == 0 || memcmp(url, "https://", 8) == 0 || memcmp(url, "udp://", 6) == 0);
 }
 
@@ -846,7 +846,7 @@ bool tr_urlIsValid(char const* url, size_t url_len)
         url_len = strlen(url);
     }
 
-    return isValidURLChars(url, url_len) && tr_urlParse(url, url_len, nullptr, nullptr, nullptr, nullptr) &&
+    return isValidURLChars(url, url_len) && tr_urlParse({ url, url_len }) &&
         (memcmp(url, "http://", 7) == 0 || memcmp(url, "https://", 8) == 0 || memcmp(url, "ftp://", 6) == 0 ||
          memcmp(url, "sftp://", 7) == 0);
 }
@@ -857,48 +857,84 @@ bool tr_addressIsIP(char const* str)
     return tr_address_from_string(&tmp, str);
 }
 
-static int parse_port(char const* port, size_t port_len)
+static int parsePort(std::string_view port)
 {
-    char* const tmp = tr_strndup(port, port_len);
-    char* end = nullptr;
-    long port_num = strtol(tmp, &end, 10);
+    auto tmp = std::array<char, 16>{};
 
+    if (std::size(port) >= std::size(tmp))
+    {
+        return -1;
+    }
+
+    std::copy(std::begin(port), std::end(port), std::begin(tmp));
+    char* end = nullptr;
+    long port_num = strtol(std::data(tmp), &end, 10);
     if (*end != '\0' || port_num <= 0 || port_num >= 65536)
     {
         port_num = -1;
     }
 
-    tr_free(tmp);
-
-    return (int)port_num;
+    return int(port_num);
 }
 
-static int get_port_for_scheme(char const* scheme, size_t scheme_len)
+static std::string_view getPortForScheme(std::string_view scheme)
 {
-    struct known_scheme
-    {
-        char const* name;
-        int port;
-    };
+    auto constexpr KnownSchemes = std::array<std::pair<std::string_view, std::string_view>, 5>{ {
+        { "udp"sv, "80"sv },
+        { "ftp"sv, "21"sv },
+        { "sftp"sv, "22"sv },
+        { "http"sv, "80"sv },
+        { "https"sv, "443"sv },
+    } };
 
-    static struct known_scheme const known_schemes[] = {
-        { "udp", 80 }, //
-        { "ftp", 21 }, //
-        { "sftp", 22 }, //
-        { "http", 80 }, //
-        { "https", 443 }, //
-        { nullptr, 0 }, //
-    };
-
-    for (struct known_scheme const* s = known_schemes; s->name != nullptr; ++s)
+    for (auto const& [known_scheme, port] : KnownSchemes)
     {
-        if (scheme_len == strlen(s->name) && memcmp(scheme, s->name, scheme_len) == 0)
+        if (scheme == known_scheme)
         {
-            return s->port;
+            return port;
         }
     }
 
-    return -1;
+    return "-1"sv;
+}
+
+std::optional<tr_parsed_url_t> tr_urlParse(std::string_view url)
+{
+    // scheme
+    auto key = "://"sv;
+    auto pos = url.find(key);
+    if (pos == std::string_view::npos || pos == 0)
+    {
+        return {};
+    }
+    auto const scheme = url.substr(0, pos);
+    url.remove_prefix(pos + std::size(key));
+
+    // authority
+    key = "/"sv;
+    pos = url.find(key);
+    if (pos == 0)
+    {
+        return {};
+    }
+    auto const authority = url.substr(0, pos);
+    url.remove_prefix(std::size(authority));
+    auto const path = std::empty(url) ? "/"sv : url;
+
+    // host
+    key = ":"sv;
+    pos = authority.find(key);
+    auto const host = pos == std::string_view::npos ? authority : authority.substr(0, pos);
+    if (std::empty(host))
+    {
+        return {};
+    }
+
+    // port
+    auto const portstr = pos == std::string_view::npos ? getPortForScheme(scheme) : authority.substr(pos + std::size(key));
+    auto const port = parsePort(portstr);
+
+    return tr_parsed_url_t{ scheme, host, path, portstr, port };
 }
 
 bool tr_urlParse(char const* url, size_t url_len, char** setme_scheme, char** setme_host, int* setme_port, char** setme_path)
@@ -967,7 +1003,8 @@ bool tr_urlParse(char const* url, size_t url_len, char** setme_scheme, char** se
 
     if (setme_port != nullptr)
     {
-        *setme_port = port_len > 0 ? parse_port(host_end + 1, port_len) : get_port_for_scheme(scheme, scheme_len);
+        auto const tmp = port_len > 0 ? std::string_view{ host_end + 1, port_len } : getPortForScheme({ scheme, scheme_len });
+        *setme_port = parsePort(tmp);
     }
 
     if (setme_path != nullptr)

libtransmission/utils.h

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <inttypes.h>
+#include <optional>
 #include <stdarg.h>
 #include <stddef.h> /* size_t */
 #include <string>
@@ -283,6 +284,17 @@ bool tr_urlIsValidTracker(char const* url);
 /** @brief return true if the url is a [ http, https, ftp, sftp ] url that Transmission understands */
 bool tr_urlIsValid(char const* url, size_t url_len);
 
+struct tr_parsed_url_t
+{
+    std::string_view scheme;
+    std::string_view host;
+    std::string_view path;
+    std::string_view portstr;
+    int port = -1;
+};
+
+std::optional<tr_parsed_url_t> tr_urlParse(std::string_view url);
+
 /** @brief parse a URL into its component parts
     @return True on success or false if an error occurred */
 bool tr_urlParse(char const* url, size_t url_len, char** setme_scheme, char** setme_host, int* setme_port, char** setme_path)

tests/libtransmission/utils-test.cc

@@ -239,6 +239,14 @@ TEST_F(UtilsTest, url)
     tr_free(path);
     tr_free(host);
 
+    auto parsed = tr_urlParse(url);
+    EXPECT_TRUE(parsed);
+    EXPECT_EQ("http"sv, parsed->scheme);
+    EXPECT_EQ("1"sv, parsed->host);
+    EXPECT_EQ("/"sv, parsed->path);
+    EXPECT_EQ("80"sv, parsed->portstr);
+    EXPECT_EQ(80, parsed->port);
+
     url = "http://www.some-tracker.org/some/path";
     scheme = nullptr;
     host = nullptr;
@@ -252,6 +260,14 @@ TEST_F(UtilsTest, url)
     tr_free(path);
     tr_free(host);
 
+    parsed = tr_urlParse(url);
+    EXPECT_TRUE(parsed);
+    EXPECT_EQ("http"sv, parsed->scheme);
+    EXPECT_EQ("www.some-tracker.org"sv, parsed->host);
+    EXPECT_EQ("/some/path"sv, parsed->path);
+    EXPECT_EQ("80"sv, parsed->portstr);
+    EXPECT_EQ(80, parsed->port);
+
     url = "http://www.some-tracker.org:8080/some/path";
     scheme = nullptr;
     host = nullptr;
@@ -264,6 +280,14 @@ TEST_F(UtilsTest, url)
     tr_free(scheme);
     tr_free(path);
     tr_free(host);
+
+    parsed = tr_urlParse(url);
+    EXPECT_TRUE(parsed);
+    EXPECT_EQ("http"sv, parsed->scheme);
+    EXPECT_EQ("www.some-tracker.org"sv, parsed->host);
+    EXPECT_EQ("/some/path"sv, parsed->path);
+    EXPECT_EQ("8080"sv, parsed->portstr);
+    EXPECT_EQ(8080, parsed->port);
 }
 
 TEST_F(UtilsTest, trHttpUnescape)