feat: honor http_proxy environment variables

.github/workflows/ci.yml

@@ -193,6 +193,25 @@ jobs:
         run: bash scripts/start-android.sh
         timeout-minutes: 20
 
+      - name: Add sentry.native.test hostname
+        if: ${{ runner.os == 'Windows' }}
+        # The path is usually C:\Windows\System32\drivers\etc\hosts
+        run: |
+          Add-Content -Path $env:SystemRoot\System32\drivers\etc\hosts -Value "127.0.0.1 sentry.native.test"
+        shell: powershell
+
+      - name: Print hosts file
+        if: ${{ runner.os == 'Windows' }}
+        run: type $env:SystemRoot\System32\drivers\etc\hosts
+        shell: powershell
+
+      - name: Add sentry.native.test hostname
+        if: ${{ runner.os == 'macOS' || runner.os == 'Linux' }}
+        run: |
+          echo "127.0.0.1   sentry.native.test" | sudo tee -a /etc/hosts
+          cat /etc/hosts
+        shell: bash
+
       - name: Test
         shell: bash
         run: |

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 **Features**:
 
+- Ensure support for `http_proxy` and `https_proxy` environment variables across all transports. ([#1111](https://github.com/getsentry/sentry-native/pull/1111))
 - Auto-detect the latest GDK and Windows SDK for the XBox build. ([#1124](https://github.com/getsentry/sentry-native/pull/1124))
 - Enable debug-option by default when running in a debug-build. ([#1128](https://github.com/getsentry/sentry-native/pull/1128))
 
@@ -23,7 +24,7 @@
 
 - Add support for Xbox Series X/S. ([#1100](https://github.com/getsentry/sentry-native/pull/1100))
 - Add option to set debug log level. ([#1107](https://github.com/getsentry/sentry-native/pull/1107))
-- Add `traces_sampler` ([#1108](https://github.com/getsentry/sentry-native/pull/1108))
+- Add `traces_sampler`. ([#1108](https://github.com/getsentry/sentry-native/pull/1108))
 - Provide support for C++17 compilers when using the `crashpad` backend. ([#1110](https://github.com/getsentry/sentry-native/pull/1110), [crashpad#116](https://github.com/getsentry/crashpad/pull/116), [mini_chromium#1](https://github.com/getsentry/mini_chromium/pull/1))
 
 ## 0.7.17

CONTRIBUTING.md

@@ -38,6 +38,9 @@ be done manually.
 
 Creates a python virtualenv, and runs all the tests through `pytest`.
 
+To run our `HTTP` proxy tests, one must add `127.0.0.1  sentry.native.test` to the `hosts` file. This is required since some transports bypass the proxy otherwise (for [example on Windows](https://learn.microsoft.com/en-us/windows/win32/wininet/enabling-internet-functionality#listing-the-proxy-bypass)).
+
+
 **Running integration tests manually**:
 
     $ pytest --verbose --maxfail=1 --capture=no tests/
@@ -147,6 +150,8 @@ The example currently supports the following commands:
 - `override-sdk-name`: Changes the SDK name via the options at runtime.
 - `stack-overflow`: Provokes a stack-overflow.
 - `http-proxy`: Uses a localhost `HTTP` proxy on port 8080.
+- `http-proxy-auth`: Uses a localhost `HTTP` proxy on port 8080 with `user:password` as authentication.
+- `http-proxy-ipv6`: Uses a localhost `HTTP` proxy on port 8080 using IPv6 notation.
 - `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080.
 - `capture-transaction`: Captures a transaction.
 - `traces-sampler`: Installs a traces sampler callback function when used alongside `capture-transaction`.

examples/example.c

@@ -271,6 +271,13 @@ main(int argc, char **argv)
     if (has_arg(argc, argv, "http-proxy")) {
         sentry_options_set_proxy(options, "http://127.0.0.1:8080");
     }
+    if (has_arg(argc, argv, "http-proxy-auth")) {
+        sentry_options_set_proxy(
+            options, "http://user:password@127.0.0.1:8080");
+    }
+    if (has_arg(argc, argv, "http-proxy-ipv6")) {
+        sentry_options_set_proxy(options, "http://[::1]:8080");
+    }
 
     if (has_arg(argc, argv, "socks5-proxy")) {
         sentry_options_set_proxy(options, "socks5://127.0.0.1:1080");

src/backends/sentry_backend_crashpad.cpp

@@ -439,9 +439,17 @@ crashpad_backend_startup(
     if (minidump_url) {
         SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url);
     }
+    auto proxy_url = "";
+    if (strncmp(options->dsn->raw, "https", 5) == 0) {
+        proxy_url = getenv("https_proxy");
+    } else if (strncmp(options->dsn->raw, "http", 4) == 0) {
+        proxy_url = getenv("http_proxy");
+    }
+    proxy_url = options->proxy ? options->proxy
+        : proxy_url != NULL    ? proxy_url
+                               : "";
     bool success = data->client->StartHandler(handler, database, database,
-        minidump_url ? minidump_url : "", options->proxy ? options->proxy : "",
-        annotations, arguments,
+        minidump_url ? minidump_url : "", proxy_url, annotations, arguments,
         /* restartable */ true,
         /* asynchronous_start */ false, attachments);
     sentry_free(minidump_url);

src/sentry_utils.c

@@ -33,7 +33,7 @@ is_scheme_valid(const char *scheme_name)
 }
 
 int
-sentry__url_parse(sentry_url_t *url_out, const char *url)
+sentry__url_parse(sentry_url_t *url_out, const char *url, bool requires_path)
 {
     bool has_username;
     int result = 0;
@@ -145,8 +145,20 @@ sentry__url_parse(sentry_url_t *url_out, const char *url)
         ptr = tmp;
     }
 
+    if (url_out->port == 0) {
+        if (sentry__string_eq(url_out->scheme, "https")) {
+            url_out->port = 443;
+        } else if (sentry__string_eq(url_out->scheme, "http")) {
+            url_out->port = 80;
+        }
+    }
+
     if (!*ptr) {
-        goto error;
+        if (requires_path) {
+            goto error;
+        }
+        result = 0;
+        goto cleanup;
     }
 
     /* end of netloc */
@@ -177,14 +189,6 @@ sentry__url_parse(sentry_url_t *url_out, const char *url)
         url_out->fragment = sentry__string_clone_n_unchecked(ptr, tmp - ptr);
     }
 
-    if (url_out->port == 0) {
-        if (sentry__string_eq(url_out->scheme, "https")) {
-            url_out->port = 443;
-        } else if (sentry__string_eq(url_out->scheme, "http")) {
-            url_out->port = 80;
-        }
-    }
-
     result = 0;
     goto cleanup;
 
@@ -228,7 +232,8 @@ sentry__dsn_new_n(const char *raw_dsn, size_t raw_dsn_len)
     dsn->refcount = 1;
 
     dsn->raw = sentry__string_clone_n(raw_dsn, raw_dsn_len);
-    if (!dsn->raw || !dsn->raw[0] || sentry__url_parse(&url, dsn->raw) != 0) {
+    if (!dsn->raw || !dsn->raw[0]
+        || sentry__url_parse(&url, dsn->raw, true) != 0) {
         goto exit;
     }
 

src/sentry_utils.h

@@ -35,8 +35,10 @@ typedef struct {
 /**
  * Parse the given `url` into the pre-allocated `url_out` parameter.
  * Returns 0 on success.
+ * `requires_path` flags whether the url needs a / after the host(:port) section
  */
-int sentry__url_parse(sentry_url_t *url_out, const char *url);
+int sentry__url_parse(
+    sentry_url_t *url_out, const char *url, bool requires_path);
 
 /**
  * This will free all the internal members of `url`, but not `url` itself, as

src/transports/sentry_transport_winhttp.c

@@ -17,6 +17,8 @@ typedef struct {
     sentry_dsn_t *dsn;
     wchar_t *user_agent;
     wchar_t *proxy;
+    wchar_t *proxy_username;
+    wchar_t *proxy_password;
     sentry_rate_limiter_t *ratelimiter;
     HINTERNET session;
     HINTERNET connect;
@@ -51,10 +53,35 @@ sentry__winhttp_bgworker_state_free(void *_state)
     sentry__dsn_decref(state->dsn);
     sentry__rate_limiter_free(state->ratelimiter);
     sentry_free(state->user_agent);
+    sentry_free(state->proxy_username);
+    sentry_free(state->proxy_password);
     sentry_free(state->proxy);
     sentry_free(state);
 }
 
+// Function to extract and set credentials
+static void
+set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy)
+{
+    sentry_url_t url;
+    sentry__url_parse(&url, proxy, false);
+    if (url.username && url.password) {
+        // Convert user and pass to LPCWSTR
+        int user_wlen
+            = MultiByteToWideChar(CP_UTF8, 0, url.username, -1, NULL, 0);
+        int pass_wlen
+            = MultiByteToWideChar(CP_UTF8, 0, url.password, -1, NULL, 0);
+        wchar_t *user_w = (wchar_t *)malloc(user_wlen * sizeof(wchar_t));
+        wchar_t *pass_w = (wchar_t *)malloc(pass_wlen * sizeof(wchar_t));
+        MultiByteToWideChar(CP_UTF8, 0, url.username, -1, user_w, user_wlen);
+        MultiByteToWideChar(CP_UTF8, 0, url.password, -1, pass_w, pass_wlen);
+
+        state->proxy_username = user_w;
+        state->proxy_password = pass_w;
+    }
+    sentry__url_cleanup(&url);
+}
+
 static int
 sentry__winhttp_transport_start(
     const sentry_options_t *opts, void *transport_state)
@@ -66,12 +93,23 @@ sentry__winhttp_transport_start(
     state->user_agent = sentry__string_to_wstr(opts->user_agent);
     state->debug = opts->debug;
 
-    sentry__bgworker_setname(bgworker, opts->transport_thread_name);
+    char *proxy = "";
+    if (strncmp(opts->dsn->raw, "https", 5) == 0) {
+        proxy = getenv("https_proxy");
+    } else if (strncmp(opts->dsn->raw, "http", 4) == 0) {
+        proxy = getenv("http_proxy");
+    }
+    proxy = opts->proxy ? opts->proxy : proxy != NULL ? proxy : "";
 
     // ensure the proxy starts with `http://`, otherwise ignore it
-    if (opts->proxy && strstr(opts->proxy, "http://") == opts->proxy) {
-        const char *ptr = opts->proxy + 7;
+    if (proxy && strstr(proxy, "http://") == proxy) {
+        const char *ptr = proxy + 7;
+        const char *at_sign = strchr(ptr, '@');
         const char *slash = strchr(ptr, '/');
+        if (at_sign && (!slash || at_sign < slash)) {
+            ptr = at_sign + 1;
+            set_proxy_credentials(state, proxy);
+        }
         if (slash) {
             char *copy = sentry__string_clone_n(ptr, slash - ptr);
             state->proxy = sentry__string_to_wstr(copy);
@@ -103,6 +141,7 @@ sentry__winhttp_transport_start(
         SENTRY_WARN("`WinHttpOpen` failed");
         return 1;
     }
+
     return sentry__bgworker_start(bgworker);
 }
 
@@ -209,6 +248,12 @@ sentry__winhttp_send_task(void *_envelope, void *_state)
     SENTRY_DEBUGF(
         "sending request using winhttp to \"%s\":\n%S", req->url, headers);
 
+    if (state->proxy_username && state->proxy_password) {
+        WinHttpSetCredentials(state->request, WINHTTP_AUTH_TARGET_PROXY,
+            WINHTTP_AUTH_SCHEME_BASIC, state->proxy_username,
+            state->proxy_password, 0);
+    }
+
     if (WinHttpSendRequest(state->request, headers, (DWORD)-1,
             (LPVOID)req->body, (DWORD)req->body_len, (DWORD)req->body_len, 0)) {
         WinHttpReceiveResponse(state->request, NULL);

tests/__init__.py

@@ -12,17 +12,24 @@
 
 sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 
-
 # https://docs.pytest.org/en/latest/assert.html#assert-details
 pytest.register_assert_rewrite("tests.assertions")
+from tests.assertions import assert_no_proxy_request
 
 
-def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456):
+def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False):
     url = urllib.parse.urlsplit(httpserver.url_for("/{}".format(id)))
     # We explicitly use `127.0.0.1` here, because on Windows, `localhost` will
     # first try `::1` (the ipv6 loopback), retry a couple times and give up
     # after a timeout of 2 seconds, falling back to the ipv4 loopback instead.
     host = url.netloc.replace("localhost", "127.0.0.1")
+    if proxy_host:
+        # To avoid bypassing the proxy for requests to localhost, we need to add this mapping
+        # to the hosts file & make the DSN using this alternate hostname
+        # see https://learn.microsoft.com/en-us/windows/win32/wininet/enabling-internet-functionality#listing-the-proxy-bypass
+        host = host.replace("127.0.0.1", "sentry.native.test")
+        _check_sentry_native_resolves_to_localhost()
+
     return urllib.parse.urlunsplit(
         (
             url.scheme,
@@ -34,12 +41,12 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456):
     )
 
 
-def is_proxy_running(host, port):
+def _check_sentry_native_resolves_to_localhost():
     try:
-        with socket.create_connection((host, port), timeout=1):
-            return True
-    except ConnectionRefusedError:
-        return False
+        resolved_ip = socket.gethostbyname("sentry.native.test")
+        assert resolved_ip == "127.0.0.1"
+    except socket.gaierror:
+        pytest.skip("sentry.native.test does not resolve to localhost")
 
 
 def run(cwd, exe, args, env=dict(os.environ), **kwargs):

tests/assertions.py

@@ -345,3 +345,15 @@ def assert_gzip_file_header(output):
 
 def assert_gzip_content_encoding(req):
     assert req.content_encoding == "gzip"
+
+
+def assert_no_proxy_request(stdout):
+    assert "POST" not in stdout
+
+
+def assert_failed_proxy_auth_request(stdout):
+    assert (
+        "POST" in stdout
+        and "407 Proxy Authentication Required" in stdout
+        and "200 OK" not in stdout
+    )

tests/leaks.txt

@@ -11,3 +11,5 @@ leak:SCDynamicStoreCopyProxiesWithOptions
 # This is a known issue in ASAN packaged with llvm15/16 (and below): https://github.com/google/sanitizers/issues/1501
 # I cannot reproduce it with the current brew llvm package (18.1.7). TODO: remove when GHA macOS runner image updates the llvm package.
 leak:realizeClassWithoutSwift
+
+leak:Curl_getaddrinfo_ex

tests/proxy.py

@@ -0,0 +1,73 @@
+import os
+import socket
+import subprocess
+import time
+
+import pytest
+
+from tests import assert_no_proxy_request
+
+
+def setup_proxy_env_vars(port):
+    os.environ["http_proxy"] = f"http://localhost:{port}"
+    os.environ["https_proxy"] = f"http://localhost:{port}"
+
+
+def cleanup_proxy_env_vars():
+    del os.environ["http_proxy"]
+    del os.environ["https_proxy"]
+
+
+def is_proxy_running(host, port):
+    try:
+        with socket.create_connection((host, port), timeout=1):
+            return True
+    except ConnectionRefusedError:
+        return False
+
+
+def start_mitmdump(proxy_type, proxy_auth: str = None):
+    # start mitmdump from terminal
+    proxy_process = None
+    if proxy_type == "http-proxy":
+        proxy_command = ["mitmdump"]
+        if proxy_auth:
+            proxy_command += ["-v", "--proxyauth", proxy_auth]
+        proxy_process = subprocess.Popen(
+            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+        )
+        time.sleep(5)  # Give mitmdump some time to start
+        if not is_proxy_running("localhost", 8080):
+            pytest.fail("mitmdump (HTTP) did not start correctly")
+    elif proxy_type == "socks5-proxy":
+        proxy_command = ["mitmdump", "--mode", "socks5"]
+        if proxy_auth:
+            proxy_command += ["-v", "--proxyauth", proxy_auth]
+        proxy_process = subprocess.Popen(
+            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+        )
+        time.sleep(5)  # Give mitmdump some time to start
+        if not is_proxy_running("localhost", 1080):
+            pytest.fail("mitmdump (SOCKS5) did not start correctly")
+    return proxy_process
+
+
+def proxy_test_finally(
+    expected_logsize,
+    httpserver,
+    proxy_process,
+    proxy_log_assert=assert_no_proxy_request,
+):
+    if proxy_process:
+        # Give mitmdump some time to get a response from the mock server
+        time.sleep(0.5)
+        proxy_process.terminate()
+        proxy_process.wait()
+        stdout, stderr = proxy_process.communicate()
+        if expected_logsize == 0:
+            # don't expect any incoming requests to make it through the proxy
+            proxy_log_assert(stdout)
+        else:
+            # request passed through successfully
+            assert "POST" in stdout and "200 OK" in stdout
+    assert len(httpserver.log) == expected_logsize