From f302e15228e94977ea9187411b6837c369263444 Mon Sep 17 00:00:00 2001 From: yhirose Date: Sun, 24 May 2026 21:17:37 -0400 Subject: [PATCH] Drop set_proxy_from_env per #2446 discussion Per @unterwegi's feedback in #2446, environment variable handling conflicts with cpp-httplib's long-standing policy of explicit configuration (e.g. set_ca_cert_path requires explicit paths instead of reading SSL_CERT_FILE / SSL_CERT_DIR). The NO_PROXY matching logic is the genuinely tricky part worth keeping in the library; getenv parsing is trivial and is left to the caller. - Remove Client::set_proxy_from_env, ClientImpl::set_proxy_from_env, and ClientImpl::apply_proxy_url - Remove ScopedEnv test helper and env-driven NoProxyTest cases - Replace the "Read proxy settings from the environment" docs with a short snippet showing how to parse no_proxy and feed set_no_proxy() - Keep set_no_proxy() and all NO_PROXY pattern matching intact --- README.md | 46 ++++---- docs-src/pages/en/cookbook/c16-proxy.md | 26 ++--- docs-src/pages/ja/cookbook/c16-proxy.md | 24 ++--- httplib.h | 138 ------------------------ test/test.cc | 100 ----------------- 5 files changed, 40 insertions(+), 294 deletions(-) diff --git a/README.md b/README.md index 730ee57..6ac03fd 100644 --- a/README.md +++ b/README.md @@ -1218,38 +1218,30 @@ Limitations: - `set_no_proxy` replaces any previously configured list; there is no append API. -#### Read proxy settings from the environment - -`set_proxy_from_env` configures the client from proxy-related -environment variables. +cpp-httplib does **not** read `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` +itself — this is consistent with `set_ca_cert_path()` and the rest of +the configuration API. If you want that behavior, parse the variables +in your application and pass the bypass patterns to `set_no_proxy()`: ```cpp -httplib::Client cli("https://api.example.com"); -cli.set_proxy_from_env(); +if (auto *v = std::getenv("no_proxy"); v && *v) { + std::vector patterns; + std::stringstream ss(v); + for (std::string item; std::getline(ss, item, ',');) { + // trim whitespace as needed + if (!item.empty()) { patterns.push_back(std::move(item)); } + } + cli.set_no_proxy(patterns); +} ``` -Variables read: - -- `https_proxy` / `HTTPS_PROXY` — used by HTTPS clients (`SSLClient`) -- `http_proxy` (lowercase only — see security note below) — used by HTTP clients -- `no_proxy` / `NO_PROXY` — comma-separated list of bypass patterns - -Returns `true` if at least one variable was found and applied. - > [!IMPORTANT] -> The uppercase `HTTP_PROXY` is intentionally ignored to mitigate the -> "httpoxy" class of bugs ([CVE-2016-5385](https://httpoxy.org/)). In -> CGI / FastCGI environments the variable name collides with the -> `HTTP_*` namespace used to expose request headers, allowing a remote -> attacker to set the proxy URL via the `Proxy:` request header. -> cpp-httplib follows curl, Go, and Python `requests` in honoring only -> the lowercase `http_proxy`. `HTTPS_PROXY` and `NO_PROXY` are safe in -> either case because their names do not begin with `HTTP_`. - -> [!NOTE] -> `set_proxy_from_env` reads `getenv` synchronously. Call it once at -> startup before issuing any requests; concurrent `setenv` from other -> threads is undefined. +> If you also read `HTTP_PROXY` from the environment, prefer the +> lowercase `http_proxy` only. The uppercase form is poisoned in +> CGI / FastCGI environments by the `Proxy:` request header +> ([CVE-2016-5385 / "httpoxy"](https://httpoxy.org/)). `HTTPS_PROXY` +> and `NO_PROXY` are safe in either case because their names do not +> begin with `HTTP_`. ### Range diff --git a/docs-src/pages/en/cookbook/c16-proxy.md b/docs-src/pages/en/cookbook/c16-proxy.md index c6fe7c4..c2628f4 100644 --- a/docs-src/pages/en/cookbook/c16-proxy.md +++ b/docs-src/pages/en/cookbook/c16-proxy.md @@ -69,23 +69,19 @@ Hostname matching is case-insensitive and uses a dot-boundary rule, so an entry Malformed entries are silently dropped. Port-specific entries such as `example.com:8080` are not supported (cpp-httplib's other host-keyed APIs are also keyed on hostname only). -## Read proxy settings from the environment +## Reading proxy settings from the environment -Call `set_proxy_from_env()` at startup to pick up proxy configuration from environment variables. +cpp-httplib does not read `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` itself. The library's configuration API is explicit by design — `set_ca_cert_path()` is the same way. If you want that behavior, parse the variables in your application and feed them to `set_proxy()` and `set_no_proxy()`. ```cpp -httplib::Client cli("https://api.example.com"); -cli.set_proxy_from_env(); +if (auto *v = std::getenv("no_proxy"); v && *v) { + std::vector patterns; + std::stringstream ss(v); + for (std::string item; std::getline(ss, item, ',');) { + if (!item.empty()) { patterns.push_back(std::move(item)); } + } + cli.set_no_proxy(patterns); +} ``` -Variables read: - -- `https_proxy` / `HTTPS_PROXY` — used by HTTPS clients -- `http_proxy` (**lowercase only**, see below) — used by HTTP clients -- `no_proxy` / `NO_PROXY` — comma-separated bypass list - -Returns `true` if at least one variable was found and applied. - -> **Security Note:** The uppercase `HTTP_PROXY` is intentionally **not** read. In CGI/FastCGI environments, the `HTTP_*` namespace is used to expose HTTP request headers, which lets a remote attacker inject an arbitrary proxy URL via the `Proxy:` request header ([CVE-2016-5385 / "httpoxy"](https://httpoxy.org/)). cpp-httplib follows curl, Go, and Python `requests` in honoring only the lowercase `http_proxy`. `HTTPS_PROXY` and `NO_PROXY` are safe in either case because their names do not begin with `HTTP_`. - -> **Note:** `set_proxy_from_env()` reads `getenv` synchronously; call it once at startup. Concurrent `setenv` from other threads while this function runs is undefined. +> **Security Note:** If you also read `HTTP_PROXY` from the environment, prefer the lowercase `http_proxy` only. The uppercase form is poisoned in CGI/FastCGI environments by the `Proxy:` request header ([CVE-2016-5385 / "httpoxy"](https://httpoxy.org/)). `HTTPS_PROXY` and `NO_PROXY` are safe in either case because their names do not begin with `HTTP_`. diff --git a/docs-src/pages/ja/cookbook/c16-proxy.md b/docs-src/pages/ja/cookbook/c16-proxy.md index a71ca28..7043ba1 100644 --- a/docs-src/pages/ja/cookbook/c16-proxy.md +++ b/docs-src/pages/ja/cookbook/c16-proxy.md @@ -71,21 +71,17 @@ cli.set_no_proxy({"internal.corp", "10.0.0.0/8", "*.dev.local"}); ## 環境変数からプロキシ設定を読み込む -`set_proxy_from_env()`を呼ぶと、起動時の環境変数からプロキシ設定をまとめて取り込めます。 +cpp-httplib本体は`HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY`を読みません。設定APIを明示的に保つ方針で、`set_ca_cert_path()`なども同様です。必要なら、アプリ側で環境変数を読んで`set_proxy()`や`set_no_proxy()`に渡します。 ```cpp -httplib::Client cli("https://api.example.com"); -cli.set_proxy_from_env(); +if (auto *v = std::getenv("no_proxy"); v && *v) { + std::vector patterns; + std::stringstream ss(v); + for (std::string item; std::getline(ss, item, ',');) { + if (!item.empty()) { patterns.push_back(std::move(item)); } + } + cli.set_no_proxy(patterns); +} ``` -読み込まれる変数: - -- `https_proxy` / `HTTPS_PROXY` — HTTPSクライアントが使用 -- `http_proxy`（**小文字のみ**、後述）— HTTPクライアントが使用 -- `no_proxy` / `NO_PROXY` — カンマ区切りのバイパスリスト - -少なくとも1つの変数が見つかって適用されたら`true`を返します。 - -> **Security Note:** 大文字の`HTTP_PROXY`は意図的に**読まれません**。CGI/FastCGI環境では`HTTP_*`という名前空間がHTTPリクエストヘッダーの公開に使われており、攻撃者が`Proxy:`リクエストヘッダーで任意のプロキシURLを差し込めてしまうためです（[CVE-2016-5385 / "httpoxy"](https://httpoxy.org/)）。curl・Go・Python `requests`と同じく、cpp-httplibも小文字の`http_proxy`しか採用しません。`HTTPS_PROXY`や`NO_PROXY`は名前が`HTTP_`で始まらないので、どちらの大文字小文字でも安全です。 - -> **Note:** `set_proxy_from_env()`は同期的に`getenv`を呼ぶだけなので、起動時に1回呼ぶことを想定しています。他スレッドが同時に`setenv`しているケースは未定義です。 +> **Security Note:** `HTTP_PROXY`をアプリ側で読む場合は、小文字の`http_proxy`だけを採用してください。大文字の方はCGI/FastCGI環境で`Proxy:`リクエストヘッダーから汚染される可能性があります（[CVE-2016-5385 / "httpoxy"](https://httpoxy.org/)）。`HTTPS_PROXY`や`NO_PROXY`は名前が`HTTP_`で始まらないので、どちらの大文字小文字でも安全です。 diff --git a/httplib.h b/httplib.h index 759cdcc..7edb0bb 100644 --- a/httplib.h +++ b/httplib.h @@ -2266,7 +2266,6 @@ public: const std::string &password); void set_proxy_bearer_token_auth(const std::string &token); void set_no_proxy(const std::vector &patterns); - bool set_proxy_from_env(); void set_logger(Logger logger); void set_error_logger(ErrorLogger error_logger); @@ -2293,7 +2292,6 @@ protected: Response &res, bool &success, Error &error); bool is_proxy_enabled_for_host(const std::string &host) const; - bool apply_proxy_url(const std::string &url); // All of: // shutdown_ssl @@ -2649,7 +2647,6 @@ public: const std::string &password); void set_proxy_bearer_token_auth(const std::string &token); void set_no_proxy(const std::vector &patterns); - bool set_proxy_from_env(); void set_logger(Logger logger); void set_error_logger(ErrorLogger error_logger); @@ -14931,140 +14928,6 @@ inline void ClientImpl::set_no_proxy(const std::vector &patterns) { no_proxy_entries_ = std::move(parsed); } -inline bool ClientImpl::apply_proxy_url(const std::string &url) { - if (url.empty()) { return false; } - - // CRLF / NUL would let a malicious env value inject extra header lines - // into a CONNECT request or a Proxy-Authorization header. - for (auto c : url) { - auto uc = static_cast(c); - if (uc < 0x20 || uc == 0x7F) { return false; } - } - - std::size_t scheme_end = 0; - bool is_https = false; - if (url.compare(0, 7, "http://") == 0) { - scheme_end = 7; - } else if (url.compare(0, 8, "https://") == 0) { - is_https = true; - scheme_end = 8; - } else { - return false; - } - - auto authority_end = url.find_first_of("/?#", scheme_end); - if (authority_end == std::string::npos) { authority_end = url.size(); } - auto authority = url.substr(scheme_end, authority_end - scheme_end); - if (authority.empty()) { return false; } - - // Split on the LAST '@' so passwords containing '@' are preserved. - std::string user; - std::string pass; - std::string host_port; - auto at_pos = authority.rfind('@'); - if (at_pos != std::string::npos) { - auto userinfo = authority.substr(0, at_pos); - host_port = authority.substr(at_pos + 1); - auto colon = userinfo.find(':'); - if (colon == std::string::npos) { - user = std::move(userinfo); - } else { - user = userinfo.substr(0, colon); - pass = userinfo.substr(colon + 1); - } - } else { - host_port = authority; - } - if (host_port.empty()) { return false; } - - std::string host; - std::string port_str; - if (host_port.front() == '[') { - auto rb = host_port.find(']'); - if (rb == std::string::npos) { return false; } - host = host_port.substr(1, rb - 1); - if (host.empty()) { return false; } - struct in6_addr tmp; - if (inet_pton(AF_INET6, host.c_str(), &tmp) != 1) { return false; } - auto rest = host_port.substr(rb + 1); - if (!rest.empty()) { - if (rest.front() != ':') { return false; } - port_str = rest.substr(1); - if (port_str.empty()) { return false; } - } - } else { - auto colon = host_port.find(':'); - if (colon == std::string::npos) { - host = host_port; - } else { - host = host_port.substr(0, colon); - port_str = host_port.substr(colon + 1); - if (port_str.empty()) { return false; } - } - if (host.empty()) { return false; } - } - - int port; - if (port_str.empty()) { - port = is_https ? 443 : 80; - } else { - int parsed = 0; - auto r = detail::from_chars(port_str.data(), - port_str.data() + port_str.size(), parsed); - if (r.ec != std::errc{} || r.ptr != port_str.data() + port_str.size()) { - return false; - } - if (parsed < 1 || parsed > 65535) { return false; } - port = parsed; - } - - // Commit only after every check has passed. - proxy_host_ = std::move(host); - proxy_port_ = port; - if (!user.empty()) { - proxy_basic_auth_username_ = std::move(user); - proxy_basic_auth_password_ = std::move(pass); - } - return true; -} - -inline bool ClientImpl::set_proxy_from_env() { - bool applied = false; - - // No cross-scheme fallback: http_proxy and https_proxy describe different - // traffic, mixing them could send HTTPS-target credentials through a - // proxy the user only authorized for HTTP. - // - // For http_proxy, lowercase ONLY: the uppercase form is poisoned in - // CGI/FastCGI environments by the "Proxy:" request header (httpoxy / - // CVE-2016-5385). HTTPS_PROXY is safe in either case because the name - // does not start with HTTP_. - const char *url_env = nullptr; - if (is_ssl()) { - url_env = std::getenv("https_proxy"); - if (!url_env || *url_env == '\0') { url_env = std::getenv("HTTPS_PROXY"); } - } else { - url_env = std::getenv("http_proxy"); - } - if (url_env && *url_env != '\0' && apply_proxy_url(url_env)) { - applied = true; - } - - const char *no_proxy_env = std::getenv("no_proxy"); - if (!no_proxy_env || *no_proxy_env == '\0') { - no_proxy_env = std::getenv("NO_PROXY"); - } - if (no_proxy_env && *no_proxy_env != '\0') { - auto entries = detail::parse_no_proxy_list(no_proxy_env); - if (!entries.empty()) { - no_proxy_entries_ = std::move(entries); - applied = true; - } - } - - return applied; -} - #ifdef CPPHTTPLIB_SSL_ENABLED inline void ClientImpl::set_digest_auth(const std::string &username, const std::string &password) { @@ -15769,7 +15632,6 @@ inline void Client::set_proxy_bearer_token_auth(const std::string &token) { inline void Client::set_no_proxy(const std::vector &patterns) { cli_->set_no_proxy(patterns); } -inline bool Client::set_proxy_from_env() { return cli_->set_proxy_from_env(); } inline void Client::set_logger(Logger logger) { cli_->set_logger(std::move(logger)); diff --git a/test/test.cc b/test/test.cc index bdf4a1a..650711b 100644 --- a/test/test.cc +++ b/test/test.cc @@ -18279,34 +18279,6 @@ TEST(KeepAliveTest, DeleteWithoutContentLengthDoesNotEatNextRequest) { namespace no_proxy_test { -#ifndef _WIN32 -class ScopedEnv { -public: - ScopedEnv(const char *name, const char *value) : name_(name) { - auto p = std::getenv(name); - had_prev_ = (p != nullptr); - if (had_prev_) { prev_ = p; } - if (value) { - ::setenv(name, value, 1); - } else { - ::unsetenv(name); - } - } - ~ScopedEnv() { - if (had_prev_) { - ::setenv(name_.c_str(), prev_.c_str(), 1); - } else { - ::unsetenv(name_.c_str()); - } - } - -private: - std::string name_; - std::string prev_; - bool had_prev_ = false; -}; -#endif // !_WIN32 - class ProxyAndTargetServers { public: ProxyAndTargetServers() { @@ -18739,75 +18711,3 @@ TEST(NoProxyTest, RedirectToBypassedHostStripsProxyAndProxyAuth) { // The first leg (going through the proxy) is allowed to carry // Proxy-Authorization; we only assert the bypassed leg does not. } - -#ifndef _WIN32 - -TEST(NoProxyTest, SetProxyFromEnv_LowercaseHttpProxy_Applied) { - no_proxy_test::ScopedEnv h("http_proxy", "http://proxy.test:3128"); - no_proxy_test::ScopedEnv H("HTTP_PROXY", nullptr); - no_proxy_test::ScopedEnv n("no_proxy", nullptr); - no_proxy_test::ScopedEnv N("NO_PROXY", nullptr); - no_proxy_test::ScopedEnv s("https_proxy", nullptr); - no_proxy_test::ScopedEnv S("HTTPS_PROXY", nullptr); - - Client cli("example.com"); - EXPECT_TRUE(cli.set_proxy_from_env()); -} - -TEST(NoProxyTest, SetProxyFromEnv_UppercaseHTTPProxy_Ignored) { - // Httpoxy mitigation: HTTP_PROXY (uppercase) must NOT be honored, - // because in CGI environments it is set from the client-supplied - // "Proxy:" header. - no_proxy_test::ScopedEnv h("http_proxy", nullptr); - no_proxy_test::ScopedEnv H("HTTP_PROXY", "http://attacker.invalid:9999"); - no_proxy_test::ScopedEnv n("no_proxy", nullptr); - no_proxy_test::ScopedEnv N("NO_PROXY", nullptr); - no_proxy_test::ScopedEnv s("https_proxy", nullptr); - no_proxy_test::ScopedEnv S("HTTPS_PROXY", nullptr); - - Client cli("example.com"); - EXPECT_FALSE(cli.set_proxy_from_env()) - << "Uppercase HTTP_PROXY must be ignored (CVE-2016-5385)"; -} - -TEST(NoProxyTest, SetProxyFromEnv_NoProxyApplied) { - no_proxy_test::ScopedEnv h("http_proxy", nullptr); - no_proxy_test::ScopedEnv H("HTTP_PROXY", nullptr); - no_proxy_test::ScopedEnv s("https_proxy", nullptr); - no_proxy_test::ScopedEnv S("HTTPS_PROXY", nullptr); - no_proxy_test::ScopedEnv n("no_proxy", "example.com"); - no_proxy_test::ScopedEnv N("NO_PROXY", nullptr); - - Client cli("example.com"); - EXPECT_TRUE(cli.set_proxy_from_env()) - << "set_proxy_from_env returns true when only NO_PROXY is set"; -} - -TEST(NoProxyTest, SetProxyFromEnv_CRLFInProxyValueRejected) { - // CR/LF in env values must be rejected at parse time so they cannot - // inject extra header lines into a CONNECT request or - // Proxy-Authorization (cf. CVE-2026-21428, CRLF injection). - no_proxy_test::ScopedEnv h("http_proxy", "http://host:8080\r\nInjected: yes"); - no_proxy_test::ScopedEnv H("HTTP_PROXY", nullptr); - no_proxy_test::ScopedEnv n("no_proxy", nullptr); - no_proxy_test::ScopedEnv N("NO_PROXY", nullptr); - no_proxy_test::ScopedEnv s("https_proxy", nullptr); - no_proxy_test::ScopedEnv S("HTTPS_PROXY", nullptr); - - Client cli("example.com"); - EXPECT_FALSE(cli.set_proxy_from_env()); -} - -TEST(NoProxyTest, SetProxyFromEnv_EmptyEnvValueIgnored) { - no_proxy_test::ScopedEnv h("http_proxy", ""); - no_proxy_test::ScopedEnv H("HTTP_PROXY", nullptr); - no_proxy_test::ScopedEnv n("no_proxy", ""); - no_proxy_test::ScopedEnv N("NO_PROXY", nullptr); - no_proxy_test::ScopedEnv s("https_proxy", nullptr); - no_proxy_test::ScopedEnv S("HTTPS_PROXY", nullptr); - - Client cli("example.com"); - EXPECT_FALSE(cli.set_proxy_from_env()); -} - -#endif // !_WIN32