Merge pull request #1181 from simue/fix/proxy-authentication-segfault

Fix Seg-fault when setting proxy username + password

Author: COM8

cpr/curlholder.cpp

@@ -1,9 +1,9 @@
 #include "cpr/curlholder.h"
+#include "cpr/secure_string.h"
 #include <cassert>
 #include <curl/curl.h>
 #include <curl/easy.h>
 #include <string_view>
-#include "cpr/secure_string.h"
 
 namespace cpr {
 CurlHolder::CurlHolder() {

cpr/proxyauth.cpp

@@ -1,6 +1,7 @@
 #include "cpr/proxyauth.h"
 #include <string>
 #include <string_view>
+#include "cpr/secure_string.h"
 
 namespace cpr {
 
@@ -12,6 +13,14 @@ std::string_view EncodedAuthentication::GetPassword() const {
     return password;
 }
 
+const util::SecureString& EncodedAuthentication::GetUsernameUnderlying() const {
+    return username;
+}
+
+const util::SecureString& EncodedAuthentication::GetPasswordUnderlying() const {
+    return password;
+}
+
 bool ProxyAuthentication::has(const std::string& protocol) const {
     return proxyAuth_.count(protocol) > 0;
 }
@@ -24,4 +33,12 @@ std::string_view ProxyAuthentication::GetPassword(const std::string& protocol) {
     return proxyAuth_[protocol].GetPassword();
 }
 
+const util::SecureString& ProxyAuthentication::GetUsernameUnderlying(const std::string& protocol) const {
+    return proxyAuth_.at(protocol).GetUsernameUnderlying();
+}
+
+const util::SecureString& ProxyAuthentication::GetPasswordUnderlying(const std::string& protocol) const {
+    return proxyAuth_.at(protocol).GetPasswordUnderlying();
+}
+
 } // namespace cpr

cpr/session.cpp

@@ -116,6 +116,17 @@ void Session::prepareHeader() {
     curl_->chunk = chunk;
 }
 
+void Session::prepareProxy() {
+    const std::string protocol = url_.str().substr(0, url_.str().find(':'));
+    if (proxies_.has(protocol)) {
+        curl_easy_setopt(curl_->handle, CURLOPT_PROXY, proxies_[protocol].c_str());
+        if (proxyAuth_.has(protocol)) {
+            curl_easy_setopt(curl_->handle, CURLOPT_PROXYUSERNAME, proxyAuth_.GetUsernameUnderlying(protocol).c_str());
+            curl_easy_setopt(curl_->handle, CURLOPT_PROXYPASSWORD, proxyAuth_.GetPasswordUnderlying(protocol).c_str());
+        }
+    }
+}
+
 // Only supported with libcurl >= 7.61.0.
 // As an alternative use SetHeader and add the token manually.
 #if LIBCURL_VERSION_NUM >= 0x073D00
@@ -173,14 +184,8 @@ void Session::prepareCommonShared() {
     }
 
     // Proxy:
-    const std::string protocol = url_.str().substr(0, url_.str().find(':'));
-    if (proxies_.has(protocol)) {
-        curl_easy_setopt(curl_->handle, CURLOPT_PROXY, proxies_[protocol].c_str());
-        if (proxyAuth_.has(protocol)) {
-            curl_easy_setopt(curl_->handle, CURLOPT_PROXYUSERNAME, proxyAuth_.GetUsername(protocol));
-            curl_easy_setopt(curl_->handle, CURLOPT_PROXYPASSWORD, proxyAuth_.GetPassword(protocol));
-        }
-    }
+    prepareProxy();
+
     // handle NO_PROXY override passed through Proxies object
     // Example: Proxies{"no_proxy": ""} will override environment variable definition with an empty list
     const std::array<std::string, 2> no_proxy{"no_proxy", "NO_PROXY"};
@@ -658,14 +663,7 @@ cpr_off_t Session::GetDownloadFileLength() {
     cpr_off_t downloadFileLength = -1;
     curl_easy_setopt(curl_->handle, CURLOPT_URL, url_.c_str());
 
-    const std::string protocol = url_.str().substr(0, url_.str().find(':'));
-    if (proxies_.has(protocol)) {
-        curl_easy_setopt(curl_->handle, CURLOPT_PROXY, proxies_[protocol].c_str());
-        if (proxyAuth_.has(protocol)) {
-            curl_easy_setopt(curl_->handle, CURLOPT_PROXYUSERNAME, proxyAuth_.GetUsername(protocol));
-            curl_easy_setopt(curl_->handle, CURLOPT_PROXYPASSWORD, proxyAuth_.GetPassword(protocol));
-        }
-    }
+    prepareProxy();
 
     curl_easy_setopt(curl_->handle, CURLOPT_HTTPGET, 1);
     curl_easy_setopt(curl_->handle, CURLOPT_NOBODY, 1);

include/cpr/proxyauth.h

@@ -27,6 +27,8 @@ class EncodedAuthentication {
 
     [[nodiscard]] std::string_view GetUsername() const;
     [[nodiscard]] std::string_view GetPassword() const;
+    [[nodiscard]] const util::SecureString& GetUsernameUnderlying() const;
+    [[nodiscard]] const util::SecureString& GetPasswordUnderlying() const;
 
   private:
     util::SecureString username;
@@ -42,6 +44,8 @@ class ProxyAuthentication {
     [[nodiscard]] bool has(const std::string& protocol) const;
     [[nodiscard]] std::string_view GetUsername(const std::string& protocol);
     [[nodiscard]] std::string_view GetPassword(const std::string& protocol);
+    [[nodiscard]] const util::SecureString& GetUsernameUnderlying(const std::string& protocol) const;
+    [[nodiscard]] const util::SecureString& GetPasswordUnderlying(const std::string& protocol) const;
 
   private:
     std::map<std::string, EncodedAuthentication> proxyAuth_;

include/cpr/session.h

@@ -296,6 +296,7 @@ class Session : public std::enable_shared_from_this<Session> {
      **/
     void prepareCommonDownload();
     void prepareHeader();
+    void prepareProxy();
     CURLcode DoEasyPerform();
     void prepareBodyPayloadOrMultipart() const;
     /**

test/proxy_auth_tests.cpp

@@ -17,6 +17,16 @@ using namespace cpr;
 
 static HttpServer* server = new HttpServer();
 
+TEST(ProxyAuthTests, SetProxyCredentials) {
+    Url url{server->GetBaseUrl() + "/hello.html"};
+    Session session;
+    session.SetUrl(url);
+    session.SetProxies(Proxies{{"http", HTTP_PROXY}, {"https", HTTPS_PROXY}});
+    session.SetProxyAuth({{"http", EncodedAuthentication{PROXY_USER, PROXY_PASS}}, {"https", EncodedAuthentication{PROXY_USER, PROXY_PASS}}});
+    session.PrepareGet();
+    EXPECT_TRUE(true);
+}
+
 // TODO: These should be fixed after a source code implementation of a proxy
 #if defined(false)
 TEST(ProxyAuthTests, SingleProxyTest) {