feat: make proxy_url nullable (#7486)

* fix: clang-tidy warning

* refactor: make `proxy_url` nullable

docs/Editing-Configuration-Files.md

@@ -81,7 +81,7 @@ Here is a sample of the three basic types: respectively Boolean, Number and Stri
  * **message-level:** Number (0 = None, 1 = Critical, 2 = Error, 3 = Warn, 4 = Info, 5 = Debug, 6 = Trace; default = 4) Set verbosity of Transmission's log messages.
  * **pex-enabled:** Boolean (default = true) Enable [Peer Exchange (PEX)](https://en.wikipedia.org/wiki/Peer_exchange).
  * **pidfile:** String Path to file in which daemon PID will be stored (_transmission-daemon only_)
- * **proxy_url:** String (default = "") Proxy for HTTP(S) requests (for example, requests to tracker). Format `[scheme]://[host]:[port]`, where `scheme` is one of: `http`, `https`, `socks4`, `socks4h`, `socks5`, `socks5h`. If unspecified, or empty, no proxy is used. For more information see [curl proxy documentation](https://curl.se/libcurl/c/CURLOPT_PROXY.html)
+ * **proxy_url:** String? (default = null) Proxy for HTTP(S) requests (for example, requests to tracker). Format `[scheme]://[host]:[port]`, where `scheme` is one of: `http`, `https`, `socks4`, `socks4h`, `socks5`, `socks5h`. If null, Transmission respects the CURL environment variables. If empty string, no proxy is used. For more information see [curl proxy documentation](https://curl.se/libcurl/c/CURLOPT_PROXY.html)
  * **scrape-paused-torrents-enabled:** Boolean (default = true)
  * **script-torrent-added-enabled:** Boolean (default = false) Run a script when a torrent is added to Transmission. Environmental variables are passed in as detailed on the [Scripts](./Scripts.md) page.
  * **script-torrent-added-filename:** String (default = "") Path to script.

libtransmission/session.cc

@@ -349,9 +349,9 @@ size_t tr_session::WebMediator::clamp(int torrent_id, size_t byte_count) const
     return tor == nullptr ? 0U : tor->bandwidth().clamp(TR_DOWN, byte_count);
 }
 
-std::optional<std::string_view> tr_session::WebMediator::proxyUrl() const
+std::optional<std::string> tr_session::WebMediator::proxyUrl() const
 {
-    return session_->settings_.proxy_url;
+    return session_->settings().proxy_url;
 }
 
 void tr_session::WebMediator::run(tr_web::FetchDoneFunc&& func, tr_web::FetchResponse&& response) const

libtransmission/session.h

@@ -263,7 +263,7 @@ private:
         [[nodiscard]] std::optional<std::string> bind_address_V6() const override;
         [[nodiscard]] std::optional<std::string_view> userAgent() const override;
         [[nodiscard]] size_t clamp(int torrent_id, size_t byte_count) const override;
-        [[nodiscard]] std::optional<std::string_view> proxyUrl() const override;
+        [[nodiscard]] std::optional<std::string> proxyUrl() const override;
         [[nodiscard]] time_t now() const override;
         // runs the tr_web::fetch response callback in the libtransmission thread
         void run(tr_web::FetchDoneFunc&& func, tr_web::FetchResponse&& response) const override;
@@ -426,6 +426,7 @@ public:
         size_t upload_slots_per_torrent = 8U;
         std::array<tr_preferred_transport, TR_NUM_PREFERRED_TRANSPORT> preferred_transport = { TR_PREFER_UTP, TR_PREFER_TCP };
         std::chrono::milliseconds sleep_per_seconds_during_verify = std::chrono::milliseconds{ 100 };
+        std::optional<std::string> proxy_url;
         std::string announce_ip;
         std::string bind_address_ipv4;
         std::string bind_address_ipv6;
@@ -437,7 +438,6 @@ public:
         std::string script_torrent_added_filename;
         std::string script_torrent_done_filename;
         std::string script_torrent_done_seeding_filename;
-        std::string proxy_url;
         tr_encryption_mode encryption_mode = TR_ENCRYPTION_PREFERRED;
         tr_log_level log_level = TR_LOG_INFO;
         tr_mode_t umask = 022;

libtransmission/settings.cc

@@ -7,6 +7,7 @@
 #include <chrono>
 #include <cstddef> // size_t
 #include <cstdint> // int64_t, uint32_t
+#include <optional>
 #include <string>
 #include <string_view>
 #include <utility>
@@ -383,6 +384,30 @@ tr_variant save_string(std::string const& val)
 
 // ---
 
+bool load_nullable_string(tr_variant const& src, std::optional<std::string>* tgt)
+{
+    if (src.holds_alternative<std::nullptr_t>())
+    {
+        tgt->reset();
+        return true;
+    }
+
+    if (auto const val = src.value_if<std::string_view>())
+    {
+        *tgt = std::string{ *val };
+        return true;
+    }
+
+    return false;
+}
+
+tr_variant save_nullable_string(std::optional<std::string> const& val)
+{
+    return val ? tr_variant{ *val } : nullptr;
+}
+
+// ---
+
 bool load_tos_t(tr_variant const& src, tr_tos_t* tgt)
 {
     if (auto const val = src.value_if<std::string_view>())
@@ -477,6 +502,7 @@ Settings::Settings()
     add_type_handler(load_preferred_transport, save_preferred_transport);
     add_type_handler(load_size_t, save_size_t);
     add_type_handler(load_string, save_string);
+    add_type_handler(load_nullable_string, save_nullable_string);
     add_type_handler(load_tos_t, save_tos_t);
     add_type_handler(load_verify_added_mode, save_verify_added_mode);
 }

libtransmission/web.cc

@@ -621,9 +621,13 @@ public:
             (void)curl_easy_setopt(e, CURLOPT_COOKIEFILE, file.c_str());
         }
 
-        if (auto const& proxyUrl = mediator.proxyUrl().value_or(""); !std::empty(proxyUrl))
+        if (auto const& proxy_url = mediator.proxyUrl(); proxy_url)
         {
-            (void)curl_easy_setopt(e, CURLOPT_PROXY, proxyUrl.data());
+            (void)curl_easy_setopt(e, CURLOPT_PROXY, proxy_url->c_str());
+        }
+        else
+        {
+            (void)curl_easy_setopt(e, CURLOPT_PROXY, nullptr);
         }
 
         if (auto const& range = task.range(); range)

libtransmission/web.h

@@ -150,7 +150,7 @@ public:
         }
 
         // Return the preferred proxy url
-        [[nodiscard]] virtual std::optional<std::string_view> proxyUrl() const
+        [[nodiscard]] virtual std::optional<std::string> proxyUrl() const
         {
             return std::nullopt;
         }

tests/libtransmission/settings-test.cc

@@ -339,6 +339,48 @@ TEST_F(SettingsTest, canSaveString)
     EXPECT_EQ(ChangedValue, *val);
 }
 
+TEST_F(SettingsTest, canLoadNullableString)
+{
+    static auto constexpr Key = TR_KEY_proxy_url;
+    static auto constexpr ChangedValue = std::string_view{ "http://127.0.0.1" };
+
+    auto settings = tr_session::Settings{};
+    EXPECT_EQ(std::nullopt, settings.proxy_url);
+
+    auto map = tr_variant::Map{ 1U };
+    map.try_emplace(Key, ChangedValue);
+    settings.load(std::move(map));
+    EXPECT_EQ(ChangedValue, settings.proxy_url);
+
+    map = tr_variant::Map{ 1U };
+    map.try_emplace(Key, nullptr);
+    settings.load(std::move(map));
+    EXPECT_EQ(std::nullopt, settings.proxy_url);
+}
+
+TEST_F(SettingsTest, canSaveNullableString)
+{
+    static auto constexpr Key = TR_KEY_proxy_url;
+    static auto constexpr ChangedValue = std::string_view{ "http://127.0.0.1" };
+
+    auto settings = tr_session::Settings{};
+    EXPECT_EQ(std::nullopt, settings.proxy_url);
+
+    settings.proxy_url = ChangedValue;
+    auto var = settings.save();
+    auto* map = var.get_if<tr_variant::Map>();
+    ASSERT_NE(map, nullptr);
+    auto const sv = map->value_if<std::string_view>(Key);
+    EXPECT_EQ(ChangedValue, sv);
+
+    settings.proxy_url = std::nullopt;
+    var = settings.save();
+    map = var.get_if<tr_variant::Map>();
+    ASSERT_NE(map, nullptr);
+    auto const null_p = map->value_if<std::nullptr_t>(Key);
+    EXPECT_TRUE(null_p);
+}
+
 TEST_F(SettingsTest, canLoadTos)
 {
     static auto constexpr Key = TR_KEY_peer_socket_tos;