From 5a42813c9840f986e122947b1c8c7e099468eef3 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:56:29 +0100 Subject: [PATCH 01/55] read http_proxy from env. variable --- src/sentry_options.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry_options.c b/src/sentry_options.c index 0cad8e335..c972b06ef 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -34,6 +34,7 @@ sentry_options_new(void) opts->release = sentry__string_clone(getenv("SENTRY_RELEASE")); opts->environment = sentry__string_clone(getenv("SENTRY_ENVIRONMENT")); #endif + sentry_options_set_proxy(opts, getenv("http_proxy")); if (!opts->environment) { opts->environment = sentry__string_clone("production"); } From 9565c21f62bfe4f60546f2a59892cc539ac83b30 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:24:26 +0100 Subject: [PATCH 02/55] add proxy read from env option --- include/sentry.h | 11 +++++++++++ src/sentry_core.c | 4 ++++ src/sentry_options.c | 23 ++++++++++++++++++++++- src/sentry_options.h | 5 +++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/include/sentry.h b/include/sentry.h index e50f61a75..4596c25fb 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1013,6 +1013,17 @@ SENTRY_API void sentry_options_set_http_proxy_n( SENTRY_API const char *sentry_options_get_http_proxy( const sentry_options_t *opts); +/** + * Sets whether to read the proxy settings from the environment. + */ +SENTRY_API void sentry_options_set_read_proxy_from_environment( + sentry_options_t *opts, int val); +/** + * Returns whether to read the proxy settings from the environment. + */ +SENTRY_API int sentry_options_get_read_proxy_from_environment( + const sentry_options_t *opts); + /** * Configures the path to a file containing ssl certificates for * verification. diff --git a/src/sentry_core.c b/src/sentry_core.c index 57d30b415..ec1747523 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -145,6 +145,10 @@ sentry_init(sentry_options_t *options) } } + if (options->read_proxy_from_environment) { + sentry__set_proxy_from_environment(options); + } + uint64_t last_crash = 0; // and then we will start the backend, since it requires a valid run diff --git a/src/sentry_options.c b/src/sentry_options.c index c972b06ef..0599b885f 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -34,7 +34,6 @@ sentry_options_new(void) opts->release = sentry__string_clone(getenv("SENTRY_RELEASE")); opts->environment = sentry__string_clone(getenv("SENTRY_ENVIRONMENT")); #endif - sentry_options_set_proxy(opts, getenv("http_proxy")); if (!opts->environment) { opts->environment = sentry__string_clone("production"); } @@ -277,6 +276,28 @@ sentry_options_get_http_proxy(const sentry_options_t *opts) return sentry_options_get_proxy(opts); } +void +sentry_options_set_read_proxy_from_environment(sentry_options_t *opts, int val) +{ + opts->read_proxy_from_environment = !!val; +} + +int +sentry_options_get_read_proxy_from_environment(const sentry_options_t *opts) +{ + return opts->read_proxy_from_environment; +} + +void +sentry__set_proxy_from_environment(sentry_options_t *opts) +{ + sentry_options_set_proxy(opts, getenv("http_proxy")); + const char *https_proxy = getenv("https_proxy"); + if (https_proxy) { + sentry_options_set_proxy(opts, https_proxy); + } +} + void sentry_options_set_ca_certs(sentry_options_t *opts, const char *path) { diff --git a/src/sentry_options.h b/src/sentry_options.h index 07a2fdbe6..cb914bc54 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -49,6 +49,7 @@ typedef struct sentry_options_s { bool require_user_consent; bool symbolize_stacktraces; bool system_crash_reporter_enabled; + bool read_proxy_from_environment; sentry_attachment_t *attachments; sentry_run_t *run; @@ -79,4 +80,8 @@ typedef struct sentry_options_s { */ sentry_options_t *sentry__options_incref(sentry_options_t *options); +/** + * Sets the proxy value by reading it from the environment. + */ +void sentry__set_proxy_from_environment(sentry_options_t *opts); #endif From 3abce7614ab680cede18a7df7a9773aabc40605f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:26:07 +0100 Subject: [PATCH 03/55] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fff8eda9d..f423de5e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Features**: - Add option to set debug log level. ([#1107](https://github.com/getsentry/sentry-native/pull/1107)) +- Honor `http_proxy` environment variables ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) ## 0.7.17 From 32a9d6012c1a7c4fb31bdede22fa085b1e29f46c Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:49:19 +0100 Subject: [PATCH 04/55] Add proxy-from-env command + fix order of init --- CONTRIBUTING.md | 1 + examples/example.c | 4 ++++ src/sentry_core.c | 8 ++++---- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 041d99d24..890b50992 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,6 +148,7 @@ The example currently supports the following commands: - `stack-overflow`: Provokes a stack-overflow. - `http-proxy`: Uses a localhost `HTTP` proxy on port 8080. - `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080. +- `proxy-from-env`: Reads the proxy settings from the environment variables `https_proxy` and `http_proxy` (in this order of precedence). Only on Windows using crashpad with its WER handler module: diff --git a/examples/example.c b/examples/example.c index 4065cd40b..5bc24b988 100644 --- a/examples/example.c +++ b/examples/example.c @@ -244,6 +244,10 @@ main(int argc, char **argv) sentry_options_set_proxy(options, "socks5://127.0.0.1:1080"); } + if (has_arg(argc, argv, "proxy-from-env")) { + sentry_options_set_read_proxy_from_environment(options, true); + } + sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { diff --git a/src/sentry_core.c b/src/sentry_core.c index ec1747523..a85aab005 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -138,6 +138,10 @@ sentry_init(sentry_options_t *options) "the provided DSN \"%s\" is not valid", raw_dsn ? raw_dsn : ""); } + if (options->read_proxy_from_environment) { + sentry__set_proxy_from_environment(options); + } + if (transport) { if (sentry__transport_startup(transport, options) != 0) { SENTRY_WARN("failed to initialize transport"); @@ -145,10 +149,6 @@ sentry_init(sentry_options_t *options) } } - if (options->read_proxy_from_environment) { - sentry__set_proxy_from_environment(options); - } - uint64_t last_crash = 0; // and then we will start the backend, since it requires a valid run From 8081074ba498585d8f212e3e20ee6accb5246d21 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 13 Jan 2025 10:19:30 +0100 Subject: [PATCH 05/55] reorder https->http proxy fetching --- src/sentry_options.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_options.c b/src/sentry_options.c index 0599b885f..3d2b76485 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -291,10 +291,11 @@ sentry_options_get_read_proxy_from_environment(const sentry_options_t *opts) void sentry__set_proxy_from_environment(sentry_options_t *opts) { - sentry_options_set_proxy(opts, getenv("http_proxy")); const char *https_proxy = getenv("https_proxy"); if (https_proxy) { sentry_options_set_proxy(opts, https_proxy); + } else { + sentry_options_set_proxy(opts, getenv("http_proxy")); } } From 0d3e09c50b824405311d7e58b17ec9d7d37c407b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:18:52 +0100 Subject: [PATCH 06/55] add proxy_auth tests --- CONTRIBUTING.md | 2 ++ examples/example.c | 8 ++++++++ tests/test_integration_http.py | 19 ++++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 890b50992..3cbd175b1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,7 +147,9 @@ 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. - `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080. +- `socks5-proxy-auth`: Uses a localhost `SOCKS5` proxy on port 1080 with `user:password` as authentication. - `proxy-from-env`: Reads the proxy settings from the environment variables `https_proxy` and `http_proxy` (in this order of precedence). Only on Windows using crashpad with its WER handler module: diff --git a/examples/example.c b/examples/example.c index 5bc24b988..80e23f038 100644 --- a/examples/example.c +++ b/examples/example.c @@ -239,10 +239,18 @@ 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, "socks5-proxy")) { sentry_options_set_proxy(options, "socks5://127.0.0.1:1080"); } + if (has_arg(argc, argv, "socks5-proxy-auth")) { + sentry_options_set_proxy( + options, "socks5://user:password@127.0.0.1:1080"); + } if (has_arg(argc, argv, "proxy-from-env")) { sentry_options_set_read_proxy_from_environment(options, true); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 15543aaa0..dcfa43df0 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -625,7 +625,8 @@ def test_capture_minidump(cmake, httpserver): ], ) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) -def test_capture_proxy(cmake, httpserver, run_args, proxy_status): +@pytest.mark.parametrize("proxy_auth", [(["off"]), (["on"])]) +def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -635,12 +636,18 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if proxy_status == ["on"]: # start mitmdump from terminal if run_args == ["http-proxy"]: - proxy_process = subprocess.Popen(["mitmdump"]) + proxy_command = ["mitmdump"] + if proxy_auth == ["on"]: + proxy_command.append("--proxyauth=user:password") + proxy_process = subprocess.Popen(proxy_command) 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 run_args == ["socks5-proxy"]: - proxy_process = subprocess.Popen(["mitmdump", "--mode", "socks5"]) + proxy_command = ["mitmdump", "--mode", "socks5"] + if proxy_auth == ["on"]: + proxy_command.append("--proxyauth=user:password") + proxy_process = subprocess.Popen(proxy_command) time.sleep(5) # Give mitmdump some time to start if not is_proxy_running("localhost", 1080): pytest.fail("mitmdump (SOCKS5) did not start correctly") @@ -651,12 +658,14 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - + current_run_arg = run_args[0] + if proxy_auth == ["on"]: + current_run_arg += "-auth" run( tmp_path, "sentry_example", ["log", "start-session", "capture-event"] - + run_args, # only passes if given proxy is running + + [current_run_arg], # only passes if given proxy is running check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) From 81abb92808ad8283ab5b0141ea4a938dc580a96c Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:51:38 +0100 Subject: [PATCH 07/55] fix CHANGELOG.md --- CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c190f5d..f9f9e1094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,18 @@ # Changelog +## Unreleased + +**Features**: +- Honor `http_proxy` environment variables. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) + ## 0.7.18 **Features**: - 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)) -- Honor `http_proxy` environment variables ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) ## 0.7.17 From d6edb129ea5d4a98aefee66be375fbfcae30ae9e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:52:49 +0100 Subject: [PATCH 08/55] fix CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9f9e1094..b0f2bdc04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased **Features**: + - Honor `http_proxy` environment variables. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) ## 0.7.18 From c3a75751b30031f7be63e6e7934be53f71134c07 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:15:33 +0100 Subject: [PATCH 09/55] fix test to avoid cmake build failed From fb4680811992ca1f5a8381af9e585286aae50b59 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:17:48 +0100 Subject: [PATCH 10/55] add WinHTTP proxy username and password PoC --- src/transports/sentry_transport_winhttp.c | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index b72751160..0ba41b6d8 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/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; @@ -55,6 +57,38 @@ sentry__winhttp_bgworker_state_free(void *_state) sentry_free(state); } +// Function to extract and set credentials TODO replace with `sentry__url_parse` +void +set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy) +{ + const char *at_sign = strchr(proxy, '@'); + if (at_sign) { + const char *credentials = proxy + 7; // Skip "http://" + char *colon = strchr(credentials, ':'); + if (colon && colon < at_sign) { + size_t user_len = colon - credentials; + size_t pass_len = at_sign - colon - 1; + char *user = (char *)malloc(user_len + 1); + char *pass = (char *)malloc(pass_len + 1); + strncpy(user, credentials, user_len); + user[user_len] = '\0'; + strncpy(pass, colon + 1, pass_len); + pass[pass_len] = '\0'; + + // Convert user and pass to LPCWSTR + int user_wlen = MultiByteToWideChar(CP_UTF8, 0, user, -1, NULL, 0); + int pass_wlen = MultiByteToWideChar(CP_UTF8, 0, pass, -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, user, -1, user_w, user_wlen); + MultiByteToWideChar(CP_UTF8, 0, pass, -1, pass_w, pass_wlen); + + state->proxy_user = user_w; + state->proxy_pass = pass_w; + } + } +} + static int sentry__winhttp_transport_start( const sentry_options_t *opts, void *transport_state) @@ -71,7 +105,12 @@ sentry__winhttp_transport_start( // ensure the proxy starts with `http://`, otherwise ignore it if (opts->proxy && strstr(opts->proxy, "http://") == opts->proxy) { const char *ptr = opts->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, opts->proxy); + } if (slash) { char *copy = sentry__string_clone_n(ptr, slash - ptr); state->proxy = sentry__string_to_wstr(copy); @@ -103,6 +142,8 @@ sentry__winhttp_transport_start( SENTRY_WARN("`WinHttpOpen` failed"); return 1; } + + set_proxy_credentials(state, opts->proxy); return sentry__bgworker_start(bgworker); } @@ -209,6 +250,11 @@ sentry__winhttp_send_task(void *_envelope, void *_state) SENTRY_DEBUGF( "sending request using winhttp to \"%s\":\n%S", req->url, headers); + if (state->proxy_user && state->proxy_pass) { + WinHttpSetCredentials(state->request, WINHTTP_AUTH_TARGET_PROXY, + WINHTTP_AUTH_SCHEME_BASIC, state->proxy_user, state->proxy_pass, 0); + } + if (WinHttpSendRequest(state->request, headers, (DWORD)-1, (LPVOID)req->body, (DWORD)req->body_len, (DWORD)req->body_len, 0)) { WinHttpReceiveResponse(state->request, NULL); From aa1c8c29e1b5954c84b660df8d3ef0309684b7cf Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:24:02 +0100 Subject: [PATCH 11/55] remove double set_proxy_credentials call --- src/transports/sentry_transport_winhttp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 0ba41b6d8..434bf969a 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -143,7 +143,6 @@ sentry__winhttp_transport_start( return 1; } - set_proxy_credentials(state, opts->proxy); return sentry__bgworker_start(bgworker); } From 86b1f6c7a41548e96b116ba929c90d52c30f261b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:51:40 +0100 Subject: [PATCH 12/55] fix typos and some mem cleaning --- src/transports/sentry_transport_winhttp.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 434bf969a..0cbe5d024 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -53,6 +53,8 @@ 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); } @@ -83,8 +85,11 @@ set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy) MultiByteToWideChar(CP_UTF8, 0, user, -1, user_w, user_wlen); MultiByteToWideChar(CP_UTF8, 0, pass, -1, pass_w, pass_wlen); - state->proxy_user = user_w; - state->proxy_pass = pass_w; + state->proxy_username = user_w; + state->proxy_password = pass_w; + + sentry_free(user); + sentry_free(pass); } } } @@ -249,9 +254,10 @@ sentry__winhttp_send_task(void *_envelope, void *_state) SENTRY_DEBUGF( "sending request using winhttp to \"%s\":\n%S", req->url, headers); - if (state->proxy_user && state->proxy_pass) { + if (state->proxy_username && state->proxy_password) { WinHttpSetCredentials(state->request, WINHTTP_AUTH_TARGET_PROXY, - WINHTTP_AUTH_SCHEME_BASIC, state->proxy_user, state->proxy_pass, 0); + WINHTTP_AUTH_SCHEME_BASIC, state->proxy_username, + state->proxy_password, 0); } if (WinHttpSendRequest(state->request, headers, (DWORD)-1, From a38ea1f3c8ba1c520e802724b962871e7ee60fc6 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 20 Jan 2025 16:11:44 +0100 Subject: [PATCH 13/55] add docs --- include/sentry.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index 2641308c1..7f133e679 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1015,6 +1015,14 @@ SENTRY_API const char *sentry_options_get_http_proxy( /** * Sets whether to read the proxy settings from the environment. + * + * If `http_proxy` or `https_proxy` is a non-null environment variable, + * this overwrites the proxy set by `sentry_options_set_http_proxy` + * during `sentry_init`, even if the explicit set function is called after + * `sentry_options_set_read_proxy_from_environment`. + * + * To re-enable using the manually set proxy, one should call this function + * again with `0`. */ SENTRY_API void sentry_options_set_read_proxy_from_environment( sentry_options_t *opts, int val); From 80156665d4f4d40edaac53e232ae2fb8bb41e73b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 21 Jan 2025 15:39:03 +0100 Subject: [PATCH 14/55] user + test 'sentry__url_parse' for proxy auth --- src/sentry_utils.c | 27 +++++++------ src/sentry_utils.h | 4 +- src/transports/sentry_transport_winhttp.c | 48 +++++++++-------------- tests/unit/test_utils.c | 47 ++++++++++++++++------ 4 files changed, 71 insertions(+), 55 deletions(-) diff --git a/src/sentry_utils.c b/src/sentry_utils.c index 62c36758f..35d975a3c 100644 --- a/src/sentry_utils.c +++ b/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; } diff --git a/src/sentry_utils.h b/src/sentry_utils.h index e7d833df5..ea6e955d3 100644 --- a/src/sentry_utils.h +++ b/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 diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 0cbe5d024..bdba408a4 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -59,38 +59,26 @@ sentry__winhttp_bgworker_state_free(void *_state) sentry_free(state); } -// Function to extract and set credentials TODO replace with `sentry__url_parse` -void +// Function to extract and set credentials +static void set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy) { - const char *at_sign = strchr(proxy, '@'); - if (at_sign) { - const char *credentials = proxy + 7; // Skip "http://" - char *colon = strchr(credentials, ':'); - if (colon && colon < at_sign) { - size_t user_len = colon - credentials; - size_t pass_len = at_sign - colon - 1; - char *user = (char *)malloc(user_len + 1); - char *pass = (char *)malloc(pass_len + 1); - strncpy(user, credentials, user_len); - user[user_len] = '\0'; - strncpy(pass, colon + 1, pass_len); - pass[pass_len] = '\0'; - - // Convert user and pass to LPCWSTR - int user_wlen = MultiByteToWideChar(CP_UTF8, 0, user, -1, NULL, 0); - int pass_wlen = MultiByteToWideChar(CP_UTF8, 0, pass, -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, user, -1, user_w, user_wlen); - MultiByteToWideChar(CP_UTF8, 0, pass, -1, pass_w, pass_wlen); - - state->proxy_username = user_w; - state->proxy_password = pass_w; - - sentry_free(user); - sentry_free(pass); - } + sentry_url_t url; + sentry__url_parse(&url, proxy, false); + if (url.username && url.password) { + state->proxy_username = sentry__string_to_wstr(url.username); + // 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; } } diff --git a/tests/unit/test_utils.c b/tests/unit/test_utils.c index 9fb5421f7..e2b6b99c4 100644 --- a/tests/unit/test_utils.c +++ b/tests/unit/test_utils.c @@ -27,18 +27,24 @@ SENTRY_TEST(iso_time) TEST_CHECK_INT_EQUAL(roundtrip, usec); } +static void +check_url(const sentry_url_t *url) +{ + TEST_CHECK_STRING_EQUAL(url->scheme, "http"); + TEST_CHECK_STRING_EQUAL(url->host, "example.com"); + TEST_CHECK_INT_EQUAL(url->port, 80); + TEST_CHECK_STRING_EQUAL(url->username, "username"); + TEST_CHECK_STRING_EQUAL(url->password, "password"); +} + SENTRY_TEST(url_parsing_complete) { sentry_url_t url; TEST_CHECK_INT_EQUAL( sentry__url_parse( - &url, "http://username:password@example.com/foo/bar?x=y#z"), + &url, "http://username:password@example.com/foo/bar?x=y#z", true), 0); - TEST_CHECK_STRING_EQUAL(url.scheme, "http"); - TEST_CHECK_STRING_EQUAL(url.host, "example.com"); - TEST_CHECK_INT_EQUAL(url.port, 80); - TEST_CHECK_STRING_EQUAL(url.username, "username"); - TEST_CHECK_STRING_EQUAL(url.password, "password"); + check_url(&url); TEST_CHECK_STRING_EQUAL(url.path, "/foo/bar"); TEST_CHECK_STRING_EQUAL(url.query, "x=y"); TEST_CHECK_STRING_EQUAL(url.fragment, "z"); @@ -49,13 +55,10 @@ SENTRY_TEST(url_parsing_partial) { sentry_url_t url; TEST_CHECK_INT_EQUAL( - sentry__url_parse(&url, "http://username:password@example.com/foo/bar"), + sentry__url_parse( + &url, "http://username:password@example.com/foo/bar", true), 0); - TEST_CHECK_STRING_EQUAL(url.scheme, "http"); - TEST_CHECK_STRING_EQUAL(url.host, "example.com"); - TEST_CHECK_INT_EQUAL(url.port, 80); - TEST_CHECK_STRING_EQUAL(url.username, "username"); - TEST_CHECK_STRING_EQUAL(url.password, "password"); + check_url(&url); TEST_CHECK_STRING_EQUAL(url.path, "/foo/bar"); TEST_CHECK(url.query == NULL); TEST_CHECK(url.fragment == NULL); @@ -65,7 +68,25 @@ SENTRY_TEST(url_parsing_partial) SENTRY_TEST(url_parsing_invalid) { sentry_url_t url; - TEST_CHECK_INT_EQUAL(sentry__url_parse(&url, "http:"), 1); + TEST_CHECK_INT_EQUAL(sentry__url_parse(&url, "http:", true), 1); +} + +SENTRY_TEST(url_parsing_no_path) +{ + sentry_url_t url; + TEST_CHECK_INT_EQUAL( + sentry__url_parse(&url, "http://username:password@example.com", false), + 0); + check_url(&url); + sentry__url_cleanup(&url); +} + +SENTRY_TEST(url_parsing_with_path) +{ + sentry_url_t url; + TEST_CHECK_INT_EQUAL( + sentry__url_parse(&url, "http://username:password@example.com", true), + 1); } SENTRY_TEST(dsn_parsing_complete) From 95a08e3693a9bbeeb0389ebd35a56e430e935b70 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 21 Jan 2025 15:46:32 +0100 Subject: [PATCH 15/55] cleanup --- src/transports/sentry_transport_winhttp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index bdba408a4..70f856bc9 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -66,7 +66,6 @@ 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) { - state->proxy_username = sentry__string_to_wstr(url.username); // Convert user and pass to LPCWSTR int user_wlen = MultiByteToWideChar(CP_UTF8, 0, url.username, -1, NULL, 0); @@ -80,6 +79,7 @@ set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy) state->proxy_username = user_w; state->proxy_password = pass_w; } + sentry__url_cleanup(&url); } static int From 26966f54ef9148eb232d8cf62d0d801b6696de72 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:44:38 +0100 Subject: [PATCH 16/55] add first proxy from environment test --- src/sentry_core.c | 2 +- src/sentry_options.c | 1 + tests/test_integration_http.py | 23 ++++++++++++++++++++--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 0f49bf4bb..1c4ce7276 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -138,7 +138,7 @@ sentry_init(sentry_options_t *options) "the provided DSN \"%s\" is not valid", raw_dsn ? raw_dsn : ""); } - if (options->read_proxy_from_environment) { + if (options->read_proxy_from_environment == true) { sentry__set_proxy_from_environment(options); } diff --git a/src/sentry_options.c b/src/sentry_options.c index 887f24047..947abf968 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -59,6 +59,7 @@ sentry_options_new(void) opts->traces_sample_rate = 0.0; opts->max_spans = 0; opts->handler_strategy = SENTRY_HANDLER_STRATEGY_DEFAULT; + opts->read_proxy_from_environment = false; return opts; } diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 05aec9ad6..54356d217 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -626,7 +626,10 @@ def test_capture_minidump(cmake, httpserver): ) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) @pytest.mark.parametrize("proxy_auth", [(["off"]), (["on"])]) -def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): +@pytest.mark.parametrize("proxy_from_env", [(["proxy-from-env"]), ([""])]) +def test_capture_proxy( + cmake, httpserver, run_args, proxy_status, proxy_auth, proxy_from_env +): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -640,7 +643,7 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): if proxy_auth == ["on"]: proxy_command.append("--proxyauth=user:password") proxy_process = subprocess.Popen(proxy_command) - time.sleep(5) # Give mitmdump some time to start + time.sleep(10) # Give mitmdump some time to start if not is_proxy_running("localhost", 8080): pytest.fail("mitmdump (HTTP) did not start correctly") elif run_args == ["socks5-proxy"]: @@ -648,7 +651,7 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): if proxy_auth == ["on"]: proxy_command.append("--proxyauth=user:password") proxy_process = subprocess.Popen(proxy_command) - time.sleep(5) # Give mitmdump some time to start + time.sleep(10) # Give mitmdump some time to start if not is_proxy_running("localhost", 1080): pytest.fail("mitmdump (SOCKS5) did not start correctly") @@ -657,10 +660,22 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): # make sure we are isolated from previous runs shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + # if proxy_from_env is set, set the proxy environment variables + if proxy_from_env == ["proxy-from-env"]: + auth = "" + if proxy_auth == ["on"]: + auth = "user:password@" + if run_args == ["http-proxy"]: + os.environ["http_proxy"] = f"http://{auth}localhost:8080" + elif run_args == ["socks5-proxy"]: + os.environ["http_proxy"] = f"socks5://{auth}localhost:1080" + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") current_run_arg = run_args[0] if proxy_auth == ["on"]: current_run_arg += "-auth" + if proxy_from_env == ["proxy-from-env"]: + current_run_arg = "proxy-from-env" # overwrite args if proxy-from-env is set (e.g. don't manually set) run( tmp_path, "sentry_example", @@ -679,3 +694,5 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status, proxy_auth): if proxy_process: proxy_process.terminate() proxy_process.wait() + if proxy_from_env == ["proxy-from-env"]: + del os.environ["http_proxy"] # remove the proxy environment variables From dd78ce5aad45d54d6eceb2f598f99439698f882b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 23 Jan 2025 13:01:49 +0100 Subject: [PATCH 17/55] add IPv6 proxy from env test --- tests/test_integration_http.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 54356d217..5396cc45e 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -626,9 +626,10 @@ def test_capture_minidump(cmake, httpserver): ) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) @pytest.mark.parametrize("proxy_auth", [(["off"]), (["on"])]) +@pytest.mark.parametrize("proxy_IPv", [(["4"]), (["6"])]) @pytest.mark.parametrize("proxy_from_env", [(["proxy-from-env"]), ([""])]) def test_capture_proxy( - cmake, httpserver, run_args, proxy_status, proxy_auth, proxy_from_env + cmake, httpserver, run_args, proxy_status, proxy_auth, proxy_from_env, proxy_IPv ): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -643,7 +644,7 @@ def test_capture_proxy( if proxy_auth == ["on"]: proxy_command.append("--proxyauth=user:password") proxy_process = subprocess.Popen(proxy_command) - time.sleep(10) # Give mitmdump some time to start + 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 run_args == ["socks5-proxy"]: @@ -651,7 +652,7 @@ def test_capture_proxy( if proxy_auth == ["on"]: proxy_command.append("--proxyauth=user:password") proxy_process = subprocess.Popen(proxy_command) - time.sleep(10) # Give mitmdump some time to start + time.sleep(5) # Give mitmdump some time to start if not is_proxy_running("localhost", 1080): pytest.fail("mitmdump (SOCKS5) did not start correctly") @@ -663,12 +664,15 @@ def test_capture_proxy( # if proxy_from_env is set, set the proxy environment variables if proxy_from_env == ["proxy-from-env"]: auth = "" + host = "localhost" + if proxy_IPv == ["6"]: + host = "[::1]" if proxy_auth == ["on"]: auth = "user:password@" if run_args == ["http-proxy"]: - os.environ["http_proxy"] = f"http://{auth}localhost:8080" + os.environ["http_proxy"] = f"http://{auth}{host}:8080" elif run_args == ["socks5-proxy"]: - os.environ["http_proxy"] = f"socks5://{auth}localhost:1080" + os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") current_run_arg = run_args[0] From da3c91e6b3e3465b664325d93bb9a8fc117c03ae Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 24 Jan 2025 12:11:28 +0100 Subject: [PATCH 18/55] proxy-from-env cleanup --- examples/example.c | 4 ---- include/sentry.h | 19 ------------------- src/sentry_core.c | 4 ---- src/sentry_options.c | 24 ------------------------ src/sentry_options.h | 5 ----- tests/test_integration_http.py | 6 +++++- 6 files changed, 5 insertions(+), 57 deletions(-) diff --git a/examples/example.c b/examples/example.c index f609fc3d2..7f313fe94 100644 --- a/examples/example.c +++ b/examples/example.c @@ -284,10 +284,6 @@ main(int argc, char **argv) options, "socks5://user:password@127.0.0.1:1080"); } - if (has_arg(argc, argv, "proxy-from-env")) { - sentry_options_set_read_proxy_from_environment(options, true); - } - sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { diff --git a/include/sentry.h b/include/sentry.h index 7bfd847e5..baac5df8b 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1013,25 +1013,6 @@ SENTRY_API void sentry_options_set_http_proxy_n( SENTRY_API const char *sentry_options_get_http_proxy( const sentry_options_t *opts); -/** - * Sets whether to read the proxy settings from the environment. - * - * If `http_proxy` or `https_proxy` is a non-null environment variable, - * this overwrites the proxy set by `sentry_options_set_http_proxy` - * during `sentry_init`, even if the explicit set function is called after - * `sentry_options_set_read_proxy_from_environment`. - * - * To re-enable using the manually set proxy, one should call this function - * again with `0`. - */ -SENTRY_API void sentry_options_set_read_proxy_from_environment( - sentry_options_t *opts, int val); -/** - * Returns whether to read the proxy settings from the environment. - */ -SENTRY_API int sentry_options_get_read_proxy_from_environment( - const sentry_options_t *opts); - /** * Configures the path to a file containing ssl certificates for * verification. diff --git a/src/sentry_core.c b/src/sentry_core.c index 1c4ce7276..f37a281e1 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -138,10 +138,6 @@ sentry_init(sentry_options_t *options) "the provided DSN \"%s\" is not valid", raw_dsn ? raw_dsn : ""); } - if (options->read_proxy_from_environment == true) { - sentry__set_proxy_from_environment(options); - } - if (transport) { if (sentry__transport_startup(transport, options) != 0) { SENTRY_WARN("failed to initialize transport"); diff --git a/src/sentry_options.c b/src/sentry_options.c index 947abf968..60db87a28 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -59,7 +59,6 @@ sentry_options_new(void) opts->traces_sample_rate = 0.0; opts->max_spans = 0; opts->handler_strategy = SENTRY_HANDLER_STRATEGY_DEFAULT; - opts->read_proxy_from_environment = false; return opts; } @@ -277,29 +276,6 @@ sentry_options_get_http_proxy(const sentry_options_t *opts) return sentry_options_get_proxy(opts); } -void -sentry_options_set_read_proxy_from_environment(sentry_options_t *opts, int val) -{ - opts->read_proxy_from_environment = !!val; -} - -int -sentry_options_get_read_proxy_from_environment(const sentry_options_t *opts) -{ - return opts->read_proxy_from_environment; -} - -void -sentry__set_proxy_from_environment(sentry_options_t *opts) -{ - const char *https_proxy = getenv("https_proxy"); - if (https_proxy) { - sentry_options_set_proxy(opts, https_proxy); - } else { - sentry_options_set_proxy(opts, getenv("http_proxy")); - } -} - void sentry_options_set_ca_certs(sentry_options_t *opts, const char *path) { diff --git a/src/sentry_options.h b/src/sentry_options.h index 952df362d..521e9e5e1 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -49,7 +49,6 @@ typedef struct sentry_options_s { bool require_user_consent; bool symbolize_stacktraces; bool system_crash_reporter_enabled; - bool read_proxy_from_environment; sentry_attachment_t *attachments; sentry_run_t *run; @@ -81,8 +80,4 @@ typedef struct sentry_options_s { */ sentry_options_t *sentry__options_incref(sentry_options_t *options); -/** - * Sets the proxy value by reading it from the environment. - */ -void sentry__set_proxy_from_environment(sentry_options_t *opts); #endif diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 5396cc45e..2dd97acec 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -671,15 +671,19 @@ def test_capture_proxy( auth = "user:password@" if run_args == ["http-proxy"]: os.environ["http_proxy"] = f"http://{auth}{host}:8080" + os.environ["https_proxy"] = f"http://{auth}{host}:8080" elif run_args == ["socks5-proxy"]: os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" + os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") current_run_arg = run_args[0] if proxy_auth == ["on"]: current_run_arg += "-auth" if proxy_from_env == ["proxy-from-env"]: - current_run_arg = "proxy-from-env" # overwrite args if proxy-from-env is set (e.g. don't manually set) + current_run_arg = ( + "" # overwrite args if proxy-from-env is set (e.g. don't manually set) + ) run( tmp_path, "sentry_example", From 0259fe85b8f58e4cfbe3de7ddb1ac6d0a7b9da2a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 27 Jan 2025 17:21:56 +0100 Subject: [PATCH 19/55] rework integration tests structure --- CONTRIBUTING.md | 1 - tests/test_integration_http.py | 113 +++++++++++++++++++-------------- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5bc81c0d7..3be092865 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,7 +150,6 @@ The example currently supports the following commands: - `http-proxy-auth`: Uses a localhost `HTTP` proxy on port 8080 with `user:password` as authentication. - `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080. - `socks5-proxy-auth`: Uses a localhost `SOCKS5` proxy on port 1080 with `user:password` as authentication. -- `proxy-from-env`: Reads the proxy settings from the environment variables `https_proxy` and `http_proxy` (in this order of precedence). - `capture-transaction`: Captures a transaction. - `traces-sampler`: Installs a traces sampler callback function when used alongside `capture-transaction`. diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 2dd97acec..62cdb5135 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -611,6 +611,66 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) +def test_proxy_from_env(cmake): + # TODO parametrize to only set http_proxy/https_proxy/empty + # TODO if env. is "wrong" proxy, then no http traffic should be sent (equivalent to proxy server being "off" ???) + # TODO think about whether we need both http and socks5 proxy tests here + # if run_args == ["http-proxy"]: + # os.environ["http_proxy"] = f"http://{auth}{host}:8080" + # os.environ["https_proxy"] = f"http://{auth}{host}:8080" + # elif run_args == ["socks5-proxy"]: + # os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" + # os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" + pass + + +def test_proxy_crash(cmake): + # TODO run with run-arg "crash" and a proxy to see how much http traffic is sent + pass + + +def test_proxy_auth(cmake): + # TODO parametrize with auth_correct bool? + # run mitmdump with auth --proxyauth=user:password & "-auth" to "http-proxy" run arg + # "user:password@" {host:}{port} + # TODO think about whether we need both http and socks5 proxy tests here + # if run_args == ["http-proxy"]: + # os.environ["http_proxy"] = f"http://{auth}{host}:8080" + # os.environ["https_proxy"] = f"http://{auth}{host}:8080" + # elif run_args == ["socks5-proxy"]: + # os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" + # os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" + pass + + +def test_proxy_ipv6(cmake): + # TODO think about whether we need both http and socks5 proxy tests here + # if proxy_IPv == ["6"]: + # host = "[::1]" + pass + + +def start_mitmdump(proxy_type, proxy_auth: bool): + # start mitmdump from terminal + if proxy_type == "http-proxy": + proxy_command = ["mitmdump"] + if proxy_auth: + proxy_command += ["-q", "--proxyauth", "user:password"] + proxy_process = subprocess.Popen(proxy_command) + 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 += ["-q", "--proxyauth", "user:password"] + proxy_process = subprocess.Popen(proxy_command) + 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 + + @pytest.mark.parametrize( "run_args", [ @@ -625,12 +685,7 @@ def test_capture_minidump(cmake, httpserver): ], ) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) -@pytest.mark.parametrize("proxy_auth", [(["off"]), (["on"])]) -@pytest.mark.parametrize("proxy_IPv", [(["4"]), (["6"])]) -@pytest.mark.parametrize("proxy_from_env", [(["proxy-from-env"]), ([""])]) -def test_capture_proxy( - cmake, httpserver, run_args, proxy_status, proxy_auth, proxy_from_env, proxy_IPv -): +def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -639,68 +694,30 @@ def test_capture_proxy( try: if proxy_status == ["on"]: # start mitmdump from terminal - if run_args == ["http-proxy"]: - proxy_command = ["mitmdump"] - if proxy_auth == ["on"]: - proxy_command.append("--proxyauth=user:password") - proxy_process = subprocess.Popen(proxy_command) - 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 run_args == ["socks5-proxy"]: - proxy_command = ["mitmdump", "--mode", "socks5"] - if proxy_auth == ["on"]: - proxy_command.append("--proxyauth=user:password") - proxy_process = subprocess.Popen(proxy_command) - time.sleep(5) # Give mitmdump some time to start - if not is_proxy_running("localhost", 1080): - pytest.fail("mitmdump (SOCKS5) did not start correctly") + proxy_process = start_mitmdump(run_args[0], False) tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) # make sure we are isolated from previous runs shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - # if proxy_from_env is set, set the proxy environment variables - if proxy_from_env == ["proxy-from-env"]: - auth = "" - host = "localhost" - if proxy_IPv == ["6"]: - host = "[::1]" - if proxy_auth == ["on"]: - auth = "user:password@" - if run_args == ["http-proxy"]: - os.environ["http_proxy"] = f"http://{auth}{host}:8080" - os.environ["https_proxy"] = f"http://{auth}{host}:8080" - elif run_args == ["socks5-proxy"]: - os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" - os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") current_run_arg = run_args[0] - if proxy_auth == ["on"]: - current_run_arg += "-auth" - if proxy_from_env == ["proxy-from-env"]: - current_run_arg = ( - "" # overwrite args if proxy-from-env is set (e.g. don't manually set) - ) run( tmp_path, "sentry_example", - ["log", "start-session", "capture-event"] + ["log", "capture-event"] + [current_run_arg], # only passes if given proxy is running check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) if proxy_status == ["on"]: - assert len(httpserver.log) == 2 + assert len(httpserver.log) == 1 elif proxy_status == ["off"]: # Windows will send the request even if the proxy is not running # macOS/Linux will not send the request if the proxy is not running - assert len(httpserver.log) == (2 if (sys.platform == "win32") else 0) + assert len(httpserver.log) == (1 if (sys.platform == "win32") else 0) finally: if proxy_process: proxy_process.terminate() proxy_process.wait() - if proxy_from_env == ["proxy-from-env"]: - del os.environ["http_proxy"] # remove the proxy environment variables From 2ffd6d12de0627bf5212f038587f0cb37b73900b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 14:12:36 +0100 Subject: [PATCH 20/55] add tests --- CONTRIBUTING.md | 2 +- examples/example.c | 7 +- src/backends/sentry_backend_crashpad.cpp | 9 +- tests/test_integration_http.py | 200 ++++++++++++++++++----- 4 files changed, 168 insertions(+), 50 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3be092865..b1c4d33bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,8 +148,8 @@ The example currently supports the following commands: - `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. -- `socks5-proxy-auth`: Uses a localhost `SOCKS5` proxy on port 1080 with `user:password` as authentication. - `capture-transaction`: Captures a transaction. - `traces-sampler`: Installs a traces sampler callback function when used alongside `capture-transaction`. diff --git a/examples/example.c b/examples/example.c index 7f313fe94..da5546767 100644 --- a/examples/example.c +++ b/examples/example.c @@ -275,14 +275,13 @@ main(int argc, char **argv) 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"); } - if (has_arg(argc, argv, "socks5-proxy-auth")) { - sentry_options_set_proxy( - options, "socks5://user:password@127.0.0.1:1080"); - } sentry_init(options); diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 08f60d290..d749cafac 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,9 +439,14 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } + char *proxy_url; + ; +#ifdef SENTRY_PLATFORM_MACOS + proxy_url = getenv("https_proxy"); // other platforms do this already +#endif + proxy_url = options->proxy ? options->proxy : 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); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 62cdb5135..1905b9f81 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -25,7 +25,7 @@ assert_gzip_content_encoding, assert_gzip_file_header, ) -from .conditions import has_http, has_breakpad, has_files +from .conditions import has_http, has_breakpad, has_files, has_crashpad pytestmark = pytest.mark.skipif(not has_http, reason="tests need http") @@ -611,51 +611,165 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) -def test_proxy_from_env(cmake): - # TODO parametrize to only set http_proxy/https_proxy/empty +@pytest.mark.parametrize("port_correct", [(["yes"]), (["no"])]) +def test_proxy_from_env(cmake, httpserver, port_correct): + # TODO how can we test this? If it doesn't get read but it's there, we don't know (since it'll just fallback) + # can we listen to mitmdump's port and see if it gets traffic? + # TODO parametrize to only set http_proxy/https_proxy/empty? # TODO if env. is "wrong" proxy, then no http traffic should be sent (equivalent to proxy server being "off" ???) - # TODO think about whether we need both http and socks5 proxy tests here - # if run_args == ["http-proxy"]: - # os.environ["http_proxy"] = f"http://{auth}{host}:8080" - # os.environ["https_proxy"] = f"http://{auth}{host}:8080" - # elif run_args == ["socks5-proxy"]: - # os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" - # os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" - pass - - -def test_proxy_crash(cmake): - # TODO run with run-arg "crash" and a proxy to see how much http traffic is sent - pass - - -def test_proxy_auth(cmake): - # TODO parametrize with auth_correct bool? - # run mitmdump with auth --proxyauth=user:password & "-auth" to "http-proxy" run arg - # "user:password@" {host:}{port} - # TODO think about whether we need both http and socks5 proxy tests here - # if run_args == ["http-proxy"]: - # os.environ["http_proxy"] = f"http://{auth}{host}:8080" - # os.environ["https_proxy"] = f"http://{auth}{host}:8080" - # elif run_args == ["socks5-proxy"]: - # os.environ["http_proxy"] = f"socks5://{auth}{host}:1080" - # os.environ["https_proxy"] = f"socks5://{auth}{host}:1080" - pass - - -def test_proxy_ipv6(cmake): - # TODO think about whether we need both http and socks5 proxy tests here - # if proxy_IPv == ["6"]: - # host = "[::1]" - pass - - -def start_mitmdump(proxy_type, proxy_auth: bool): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + try: + proxy_process = start_mitmdump("http-proxy") + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + port = "8080" if port_correct == ["yes"] else "8081" + os.environ["http_proxy"] = f"http://localhost:{port}" + os.environ["https_proxy"] = f"http://localhost:{port}" + + run( + tmp_path, + "sentry_example", + ["log", "capture-event"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + + if port_correct == ["yes"]: + assert len(httpserver.log) == 1 + elif port_correct == ["no"]: + assert len(httpserver.log) == 0 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + + +@pytest.mark.skipif(not has_crashpad, reason="test needs crashpad backend") +def test_proxy_crash(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + os.environ["http_proxy"] = "http://localhost:8080" + os.environ["https_proxy"] = "http://localhost:8080" + + proxy_process = None # store the proxy process to terminate it later + try: + # proxy_process = start_mitmdump("http-proxy") # TODO uncomment after local testing + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + httpserver.expect_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + # TODO figure out why make_dsn returns 500 error where 'real' DSN works + DSN = "https://d545f4da00ff7b2fa7b6c8620c94a4e9@o447951.ingest.sentry.io/4506178389999616" + DSN = make_dsn(httpserver) + + try: + run( + tmp_path, + "sentry_example", + # ["log", "crash", "http-proxy"], + ["log", "crash"], + check=True, + env=dict(os.environ, SENTRY_DSN=DSN), + ) + except Exception as e: + print(f"An error occurred: {e}") + finally: + time.sleep(1) # give crashpad some time to write the minidump + assert len(httpserver.log) == 1 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + del os.environ["http_proxy"] + del os.environ["https_proxy"] + + +@pytest.mark.parametrize("auth_correct", [(["yes"]), (["no"])]) +def test_proxy_auth(cmake, httpserver, auth_correct): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + try: + proxy_auth = "user:password" if auth_correct == ["yes"] else "wrong:wrong" + proxy_process = start_mitmdump("http-proxy", proxy_auth) + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + ["log", "capture-event", "http-proxy-auth"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + if auth_correct == ["yes"]: + assert len(httpserver.log) == 1 + elif auth_correct == ["no"]: + assert len(httpserver.log) == 0 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + + +def test_proxy_ipv6(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + try: + proxy_process = start_mitmdump("http-proxy") + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + ["log", "capture-event", "http-proxy-ipv6"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + + assert len(httpserver.log) == 1 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + + +def start_mitmdump(proxy_type, proxy_auth: str = None): # start mitmdump from terminal if proxy_type == "http-proxy": proxy_command = ["mitmdump"] if proxy_auth: - proxy_command += ["-q", "--proxyauth", "user:password"] + proxy_command += ["-q", "--proxyauth", proxy_auth] proxy_process = subprocess.Popen(proxy_command) time.sleep(5) # Give mitmdump some time to start if not is_proxy_running("localhost", 8080): @@ -663,7 +777,7 @@ def start_mitmdump(proxy_type, proxy_auth: bool): elif proxy_type == "socks5-proxy": proxy_command = ["mitmdump", "--mode", "socks5"] if proxy_auth: - proxy_command += ["-q", "--proxyauth", "user:password"] + proxy_command += ["-q", "--proxyauth", proxy_auth] proxy_process = subprocess.Popen(proxy_command) time.sleep(5) # Give mitmdump some time to start if not is_proxy_running("localhost", 1080): @@ -694,7 +808,7 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): try: if proxy_status == ["on"]: # start mitmdump from terminal - proxy_process = start_mitmdump(run_args[0], False) + proxy_process = start_mitmdump(run_args[0]) tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) From 5f88d31cd7de401dea6095a9391531fe3c9118d4 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 14:51:08 +0100 Subject: [PATCH 21/55] move crashpad test & reusable start_mitmdump --- tests/__init__.py | 22 ++++++++++ tests/test_integration_crashpad.py | 49 ++++++++++++++++----- tests/test_integration_http.py | 70 +----------------------------- 3 files changed, 61 insertions(+), 80 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 98e6218d1..76ab3e64d 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -8,6 +8,7 @@ import pytest import pprint import textwrap +import time import socket sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) @@ -42,6 +43,27 @@ def is_proxy_running(host, port): return False +def start_mitmdump(proxy_type, proxy_auth: str = None): + # start mitmdump from terminal + if proxy_type == "http-proxy": + proxy_command = ["mitmdump"] + if proxy_auth: + proxy_command += ["-q", "--proxyauth", proxy_auth] + proxy_process = subprocess.Popen(proxy_command) + 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 += ["-q", "--proxyauth", proxy_auth] + proxy_process = subprocess.Popen(proxy_command) + 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 run(cwd, exe, args, env=dict(os.environ), **kwargs): __tracebackhide__ = True if os.environ.get("ANDROID_API"): diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index a0ffad812..1f99297ac 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -6,7 +6,7 @@ import pytest -from . import make_dsn, run, Envelope, is_proxy_running +from . import make_dsn, run, Envelope, start_mitmdump from .assertions import ( assert_crashpad_upload, assert_session, @@ -39,6 +39,42 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 +def test_crashpad_crash_proxy_env(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + os.environ["http_proxy"] = "http://localhost:8080" + os.environ["https_proxy"] = "http://localhost:8080" + try: + # proxy_process = start_mitmdump("http-proxy") + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( + "OK" + ) + + with httpserver.wait(timeout=10) as waiting: + child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) + assert child.returncode # well, it's a crash after all + + assert waiting.result + # TODO add listener to mitmproxy to check if the proxy was actually used + # (since fallback to direct connection is possible) + assert len(httpserver.log) == 1 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + del os.environ["http_proxy"] + del os.environ["https_proxy"] + + @pytest.mark.parametrize( "run_args", [ @@ -62,16 +98,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): try: if proxy_status == ["on"]: # start mitmdump from terminal - if run_args == ["http-proxy"]: - proxy_process = subprocess.Popen(["mitmdump"]) - 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 run_args == ["socks5-proxy"]: - proxy_process = subprocess.Popen(["mitmdump", "--mode", "socks5"]) - time.sleep(5) # Give mitmdump some time to start - if not is_proxy_running("localhost", 1080): - pytest.fail("mitmdump (SOCKS5) did not start correctly") + proxy_process = start_mitmdump("http") tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 1905b9f81..ba3b0c4a2 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -9,7 +9,7 @@ import pytest -from . import make_dsn, run, Envelope, is_proxy_running +from . import make_dsn, run, Envelope, start_mitmdump from .assertions import ( assert_attachment, assert_meta, @@ -653,53 +653,6 @@ def test_proxy_from_env(cmake, httpserver, port_correct): proxy_process.wait() -@pytest.mark.skipif(not has_crashpad, reason="test needs crashpad backend") -def test_proxy_crash(cmake, httpserver): - if not shutil.which("mitmdump"): - pytest.skip("mitmdump is not installed") - - os.environ["http_proxy"] = "http://localhost:8080" - os.environ["https_proxy"] = "http://localhost:8080" - - proxy_process = None # store the proxy process to terminate it later - try: - # proxy_process = start_mitmdump("http-proxy") # TODO uncomment after local testing - - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) - - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - - httpserver.expect_request( - "/api/123456/envelope/", - headers={"x-sentry-auth": auth_header}, - ).respond_with_data("OK") - # TODO figure out why make_dsn returns 500 error where 'real' DSN works - DSN = "https://d545f4da00ff7b2fa7b6c8620c94a4e9@o447951.ingest.sentry.io/4506178389999616" - DSN = make_dsn(httpserver) - - try: - run( - tmp_path, - "sentry_example", - # ["log", "crash", "http-proxy"], - ["log", "crash"], - check=True, - env=dict(os.environ, SENTRY_DSN=DSN), - ) - except Exception as e: - print(f"An error occurred: {e}") - finally: - time.sleep(1) # give crashpad some time to write the minidump - assert len(httpserver.log) == 1 - finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() - del os.environ["http_proxy"] - del os.environ["https_proxy"] - - @pytest.mark.parametrize("auth_correct", [(["yes"]), (["no"])]) def test_proxy_auth(cmake, httpserver, auth_correct): if not shutil.which("mitmdump"): @@ -764,27 +717,6 @@ def test_proxy_ipv6(cmake, httpserver): proxy_process.wait() -def start_mitmdump(proxy_type, proxy_auth: str = None): - # start mitmdump from terminal - if proxy_type == "http-proxy": - proxy_command = ["mitmdump"] - if proxy_auth: - proxy_command += ["-q", "--proxyauth", proxy_auth] - proxy_process = subprocess.Popen(proxy_command) - 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 += ["-q", "--proxyauth", proxy_auth] - proxy_process = subprocess.Popen(proxy_command) - 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 - - @pytest.mark.parametrize( "run_args", [ From 3cfefb3077ae67565414d1682ccc106f010f8290 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:07:13 +0100 Subject: [PATCH 22/55] test cleanup --- tests/test_integration_crashpad.py | 37 ++++++++++++++++++------------ tests/test_integration_http.py | 27 +++++++++++----------- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 1f99297ac..ea3e16efa 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -39,15 +39,17 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 -def test_crashpad_crash_proxy_env(cmake, httpserver): +@pytest.mark.parametrize("port_correct", [True, False]) +def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later - os.environ["http_proxy"] = "http://localhost:8080" - os.environ["https_proxy"] = "http://localhost:8080" + port = "8080" if port_correct else "8081" + os.environ["http_proxy"] = f"http://localhost:{port}" + os.environ["https_proxy"] = f"http://localhost:{port}" try: - # proxy_process = start_mitmdump("http-proxy") + proxy_process = start_mitmdump("http-proxy") tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) @@ -59,13 +61,18 @@ def test_crashpad_crash_proxy_env(cmake, httpserver): "OK" ) - with httpserver.wait(timeout=10) as waiting: - child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) - assert child.returncode # well, it's a crash after all + try: + with httpserver.wait(timeout=10) as waiting: + child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) + assert child.returncode # well, it's a crash after all + except AssertionError as e: + if port_correct: + raise e + else: + assert len(httpserver.log) == 0 + return assert waiting.result - # TODO add listener to mitmproxy to check if the proxy was actually used - # (since fallback to direct connection is possible) assert len(httpserver.log) == 1 finally: if proxy_process: @@ -88,17 +95,17 @@ def test_crashpad_crash_proxy_env(cmake, httpserver): ), ], ) -@pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) -def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): +@pytest.mark.parametrize("proxy_running", [True, False]) +def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later try: - if proxy_status == ["on"]: + if proxy_running: # start mitmdump from terminal - proxy_process = start_mitmdump("http") + proxy_process = start_mitmdump(run_args[0]) tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) @@ -118,11 +125,11 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): assert child.returncode # well, it's a crash after all except AssertionError: # we fail on macOS/Linux if the http proxy is not running - if run_args == ["http-proxy"] and proxy_status == ["off"]: + if run_args == ["http-proxy"] and not proxy_running: assert len(httpserver.log) == 0 return # we only fail on linux if the socks5 proxy is not running - elif run_args == ["socks5-proxy"] and proxy_status == ["off"]: + elif run_args == ["socks5-proxy"] and not proxy_running: assert len(httpserver.log) == 0 return diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index ba3b0c4a2..a08792004 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -611,12 +611,11 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) -@pytest.mark.parametrize("port_correct", [(["yes"]), (["no"])]) +@pytest.mark.parametrize("port_correct", [True, False]) def test_proxy_from_env(cmake, httpserver, port_correct): # TODO how can we test this? If it doesn't get read but it's there, we don't know (since it'll just fallback) # can we listen to mitmdump's port and see if it gets traffic? # TODO parametrize to only set http_proxy/https_proxy/empty? - # TODO if env. is "wrong" proxy, then no http traffic should be sent (equivalent to proxy server being "off" ???) if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -631,7 +630,7 @@ def test_proxy_from_env(cmake, httpserver, port_correct): httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - port = "8080" if port_correct == ["yes"] else "8081" + port = "8080" if port_correct else "8081" os.environ["http_proxy"] = f"http://localhost:{port}" os.environ["https_proxy"] = f"http://localhost:{port}" @@ -643,9 +642,9 @@ def test_proxy_from_env(cmake, httpserver, port_correct): env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) - if port_correct == ["yes"]: + if port_correct: assert len(httpserver.log) == 1 - elif port_correct == ["no"]: + else: assert len(httpserver.log) == 0 finally: if proxy_process: @@ -653,14 +652,14 @@ def test_proxy_from_env(cmake, httpserver, port_correct): proxy_process.wait() -@pytest.mark.parametrize("auth_correct", [(["yes"]), (["no"])]) +@pytest.mark.parametrize("auth_correct", [True, False]) def test_proxy_auth(cmake, httpserver, auth_correct): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later try: - proxy_auth = "user:password" if auth_correct == ["yes"] else "wrong:wrong" + proxy_auth = "user:password" if auth_correct else "wrong:wrong" proxy_process = start_mitmdump("http-proxy", proxy_auth) tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) @@ -677,9 +676,9 @@ def test_proxy_auth(cmake, httpserver, auth_correct): check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) - if auth_correct == ["yes"]: + if auth_correct: assert len(httpserver.log) == 1 - elif auth_correct == ["no"]: + else: assert len(httpserver.log) == 0 finally: if proxy_process: @@ -730,15 +729,15 @@ def test_proxy_ipv6(cmake, httpserver): ), ], ) -@pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) -def test_capture_proxy(cmake, httpserver, run_args, proxy_status): +@pytest.mark.parametrize("proxy_running", [True, False]) +def test_capture_proxy(cmake, httpserver, run_args, proxy_running): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later try: - if proxy_status == ["on"]: + if proxy_running: # start mitmdump from terminal proxy_process = start_mitmdump(run_args[0]) @@ -757,9 +756,9 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) - if proxy_status == ["on"]: + if proxy_running: assert len(httpserver.log) == 1 - elif proxy_status == ["off"]: + else: # Windows will send the request even if the proxy is not running # macOS/Linux will not send the request if the proxy is not running assert len(httpserver.log) == (1 if (sys.platform == "win32") else 0) From 718af591a780e9e51dcb5229e0875fbfe9a24375 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 16:16:44 +0100 Subject: [PATCH 23/55] fix null-string passing --- src/backends/sentry_backend_crashpad.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index d749cafac..198fb819a 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,12 +439,11 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } - char *proxy_url; - ; + const char *proxy_url; #ifdef SENTRY_PLATFORM_MACOS proxy_url = getenv("https_proxy"); // other platforms do this already #endif - proxy_url = options->proxy ? options->proxy : proxy_url; + proxy_url = options->proxy ? options->proxy : proxy_url ? proxy_url : ""; bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, From 4a974268b2e9e298267368d06343f698a00b60bc Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:07:36 +0100 Subject: [PATCH 24/55] clean up os.environ --- tests/test_integration_http.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index a08792004..9bba94b67 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -651,6 +651,9 @@ def test_proxy_from_env(cmake, httpserver, port_correct): proxy_process.terminate() proxy_process.wait() + del os.environ["http_proxy"] + del os.environ["https_proxy"] + @pytest.mark.parametrize("auth_correct", [True, False]) def test_proxy_auth(cmake, httpserver, auth_correct): From 8411d8bded7736881fe2c3d6cbf246851f36147a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:32:32 +0100 Subject: [PATCH 25/55] add check if request was proxied --- tests/__init__.py | 8 ++++++-- tests/test_integration_crashpad.py | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 76ab3e64d..a7a3e5492 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -49,7 +49,9 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): proxy_command = ["mitmdump"] if proxy_auth: proxy_command += ["-q", "--proxyauth", proxy_auth] - proxy_process = subprocess.Popen(proxy_command) + 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") @@ -57,7 +59,9 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): proxy_command = ["mitmdump", "--mode", "socks5"] if proxy_auth: proxy_command += ["-q", "--proxyauth", proxy_auth] - proxy_process = subprocess.Popen(proxy_command) + 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") diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index ea3e16efa..8bfea274b 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -73,7 +73,13 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): return assert waiting.result + + proxy_process.terminate() + proxy_process.wait() + stdout, stderr = proxy_process.communicate() assert len(httpserver.log) == 1 + # check if the request was proxied or just passed through + assert "POST" in stdout finally: if proxy_process: proxy_process.terminate() From 4d63147d70349cef3e4a506a42bf4c12da8d52b7 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:10:00 +0100 Subject: [PATCH 26/55] fix uninitialized proxy_url --- src/backends/sentry_backend_crashpad.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 198fb819a..a55cb9dcb 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,11 +439,13 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } - const char *proxy_url; + auto proxy_url = ""; #ifdef SENTRY_PLATFORM_MACOS proxy_url = getenv("https_proxy"); // other platforms do this already #endif - proxy_url = options->proxy ? options->proxy : proxy_url ? proxy_url : ""; + proxy_url = options->proxy ? options->proxy + : proxy_url != NULL ? proxy_url + : ""; bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, From 0c64f82c2d61f03a7c61058dfbc276f3860b1d78 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:10:35 +0100 Subject: [PATCH 27/55] add proxy_env finally check/cleanup function --- tests/test_integration_crashpad.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 8bfea274b..853e8a7e3 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -39,7 +39,7 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 -@pytest.mark.parametrize("port_correct", [True, False]) +@pytest.mark.parametrize("port_correct", [True, False]) # TODO separate tests def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -48,6 +48,7 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): port = "8080" if port_correct else "8081" os.environ["http_proxy"] = f"http://localhost:{port}" os.environ["https_proxy"] = f"http://localhost:{port}" + expected_logsize = 0 try: proxy_process = start_mitmdump("http-proxy") @@ -69,23 +70,28 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): if port_correct: raise e else: - assert len(httpserver.log) == 0 + expected_logsize = 0 return assert waiting.result + expected_logsize = 1 + finally: + proxy_env_finally(expected_logsize, httpserver, proxy_process) + + +def proxy_env_finally(expected_logsize, httpserver, proxy_process): + if proxy_process: proxy_process.terminate() proxy_process.wait() stdout, stderr = proxy_process.communicate() - assert len(httpserver.log) == 1 - # check if the request was proxied or just passed through - assert "POST" in stdout - finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() - del os.environ["http_proxy"] - del os.environ["https_proxy"] + if expected_logsize == 0: # don't expect any incoming requests at the proxy + assert "POST" not in stdout + else: + assert "POST" in stdout + assert len(httpserver.log) == expected_logsize + del os.environ["http_proxy"] + del os.environ["https_proxy"] @pytest.mark.parametrize( From b78aa9289ffb4b8936c01f3969eefc620dc06624 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 29 Jan 2025 12:14:25 +0100 Subject: [PATCH 28/55] extract proxy test cleanup/check function --- tests/__init__.py | 19 ++++++++++-- tests/test_integration_crashpad.py | 20 +++---------- tests/test_integration_http.py | 47 ++++++++++++------------------ 3 files changed, 40 insertions(+), 46 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index a7a3e5492..8fc1292d5 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -48,7 +48,7 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): if proxy_type == "http-proxy": proxy_command = ["mitmdump"] if proxy_auth: - proxy_command += ["-q", "--proxyauth", proxy_auth] + proxy_command += ["-v", "--proxyauth", proxy_auth] proxy_process = subprocess.Popen( proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True ) @@ -58,7 +58,7 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): elif proxy_type == "socks5-proxy": proxy_command = ["mitmdump", "--mode", "socks5"] if proxy_auth: - proxy_command += ["-q", "--proxyauth", proxy_auth] + proxy_command += ["-v", "--proxyauth", proxy_auth] proxy_process = subprocess.Popen( proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True ) @@ -68,6 +68,21 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): return proxy_process +def proxy_test_finally(expected_logsize, httpserver, proxy_process): + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + stdout, stderr = proxy_process.communicate() + if expected_logsize == 0: # don't expect any incoming requests at the proxy + # second case is for the case where we expect a POST request to be blocked (e.g. authentication failed) + assert ("POST" not in stdout) or ( + "POST" in stdout and "HTTP/1.0 200 OK" not in stdout + ) + else: + assert "POST" in stdout + assert len(httpserver.log) == expected_logsize + + def run(cwd, exe, args, env=dict(os.environ), **kwargs): __tracebackhide__ = True if os.environ.get("ANDROID_API"): diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 853e8a7e3..a2fc7db9e 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -6,7 +6,7 @@ import pytest -from . import make_dsn, run, Envelope, start_mitmdump +from . import make_dsn, run, Envelope, start_mitmdump, proxy_test_finally from .assertions import ( assert_crashpad_upload, assert_session, @@ -77,21 +77,9 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): expected_logsize = 1 finally: - proxy_env_finally(expected_logsize, httpserver, proxy_process) - - -def proxy_env_finally(expected_logsize, httpserver, proxy_process): - if proxy_process: - proxy_process.terminate() - proxy_process.wait() - stdout, stderr = proxy_process.communicate() - if expected_logsize == 0: # don't expect any incoming requests at the proxy - assert "POST" not in stdout - else: - assert "POST" in stdout - assert len(httpserver.log) == expected_logsize - del os.environ["http_proxy"] - del os.environ["https_proxy"] + proxy_test_finally(expected_logsize, httpserver, proxy_process) + del os.environ["http_proxy"] + del os.environ["https_proxy"] @pytest.mark.parametrize( diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 9bba94b67..ac5e11081 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -9,7 +9,7 @@ import pytest -from . import make_dsn, run, Envelope, start_mitmdump +from . import make_dsn, run, Envelope, start_mitmdump, proxy_test_finally from .assertions import ( assert_attachment, assert_meta, @@ -613,13 +613,15 @@ def test_capture_minidump(cmake, httpserver): @pytest.mark.parametrize("port_correct", [True, False]) def test_proxy_from_env(cmake, httpserver, port_correct): - # TODO how can we test this? If it doesn't get read but it's there, we don't know (since it'll just fallback) - # can we listen to mitmdump's port and see if it gets traffic? # TODO parametrize to only set http_proxy/https_proxy/empty? if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later + port = "8080" if port_correct else "8081" + os.environ["http_proxy"] = f"http://localhost:{port}" + os.environ["https_proxy"] = f"http://localhost:{port}" + expected_logsize = 0 try: proxy_process = start_mitmdump("http-proxy") @@ -630,10 +632,6 @@ def test_proxy_from_env(cmake, httpserver, port_correct): httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - port = "8080" if port_correct else "8081" - os.environ["http_proxy"] = f"http://localhost:{port}" - os.environ["https_proxy"] = f"http://localhost:{port}" - run( tmp_path, "sentry_example", @@ -643,14 +641,11 @@ def test_proxy_from_env(cmake, httpserver, port_correct): ) if port_correct: - assert len(httpserver.log) == 1 + expected_logsize = 1 else: - assert len(httpserver.log) == 0 + expected_logsize = 0 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() - + proxy_test_finally(expected_logsize, httpserver, proxy_process) del os.environ["http_proxy"] del os.environ["https_proxy"] @@ -661,10 +656,10 @@ def test_proxy_auth(cmake, httpserver, auth_correct): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later + expected_logsize = 0 try: proxy_auth = "user:password" if auth_correct else "wrong:wrong" proxy_process = start_mitmdump("http-proxy", proxy_auth) - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) # make sure we are isolated from previous runs @@ -680,13 +675,11 @@ def test_proxy_auth(cmake, httpserver, auth_correct): env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) if auth_correct: - assert len(httpserver.log) == 1 + expected_logsize = 1 else: - assert len(httpserver.log) == 0 + expected_logsize = 0 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() + proxy_test_finally(expected_logsize, httpserver, proxy_process) def test_proxy_ipv6(cmake, httpserver): @@ -694,6 +687,7 @@ def test_proxy_ipv6(cmake, httpserver): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later + expected_logsize = 0 try: proxy_process = start_mitmdump("http-proxy") @@ -712,11 +706,9 @@ def test_proxy_ipv6(cmake, httpserver): env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) - assert len(httpserver.log) == 1 + expected_logsize = 1 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() + proxy_test_finally(expected_logsize, httpserver, proxy_process) @pytest.mark.parametrize( @@ -738,6 +730,7 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_running): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later + expected_logsize = 0 try: if proxy_running: @@ -760,12 +753,10 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_running): env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) if proxy_running: - assert len(httpserver.log) == 1 + expected_logsize = 1 else: # Windows will send the request even if the proxy is not running # macOS/Linux will not send the request if the proxy is not running - assert len(httpserver.log) == (1 if (sys.platform == "win32") else 0) + expected_logsize = 1 if (sys.platform == "win32") else 0 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() + proxy_test_finally(expected_logsize, httpserver, proxy_process) From 75f7edf415825fda8f74878d1bb95088736768d5 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 29 Jan 2025 12:35:34 +0100 Subject: [PATCH 29/55] fix proxy_test_finally + update CHANGELOG.md --- CHANGELOG.md | 2 +- tests/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19ed3b5db..22e02bf69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ **Features**: -- Honor `http_proxy` environment variables. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) +- Honor `http(s)_proxy` environment variables on macOS crashpad transport. ([#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)) diff --git a/tests/__init__.py b/tests/__init__.py index 8fc1292d5..4d8580da1 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -76,10 +76,10 @@ def proxy_test_finally(expected_logsize, httpserver, proxy_process): if expected_logsize == 0: # don't expect any incoming requests at the proxy # second case is for the case where we expect a POST request to be blocked (e.g. authentication failed) assert ("POST" not in stdout) or ( - "POST" in stdout and "HTTP/1.0 200 OK" not in stdout + "POST" in stdout and "200 OK" not in stdout ) else: - assert "POST" in stdout + assert "POST" in stdout and "200 OK" in stdout assert len(httpserver.log) == expected_logsize From 576b8da639e440dfca0f58f1cab1de9dc3e782cf Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 29 Jan 2025 13:15:53 +0100 Subject: [PATCH 30/55] add assert functions --- tests/__init__.py | 20 ++++++++++++++------ tests/assertions.py | 12 ++++++++++++ tests/test_integration_http.py | 8 +++++++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 4d8580da1..be5630711 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -11,11 +11,13 @@ import time import socket + 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): @@ -68,17 +70,23 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): return proxy_process -def proxy_test_finally(expected_logsize, httpserver, 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 at the proxy - # second case is for the case where we expect a POST request to be blocked (e.g. authentication failed) - assert ("POST" not in stdout) or ( - "POST" in stdout and "200 OK" not in stdout - ) + if expected_logsize == 0: + # don't expect any incoming requests to go 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 diff --git a/tests/assertions.py b/tests/assertions.py index 8d640d372..ae6771c58 100644 --- a/tests/assertions.py +++ b/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 + ) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index ac5e11081..353934afb 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -24,6 +24,7 @@ assert_breakpad_crash, assert_gzip_content_encoding, assert_gzip_file_header, + assert_failed_proxy_auth_request, ) from .conditions import has_http, has_breakpad, has_files, has_crashpad @@ -679,7 +680,12 @@ def test_proxy_auth(cmake, httpserver, auth_correct): else: expected_logsize = 0 finally: - proxy_test_finally(expected_logsize, httpserver, proxy_process) + proxy_test_finally( + expected_logsize, + httpserver, + proxy_process, + assert_failed_proxy_auth_request, + ) def test_proxy_ipv6(cmake, httpserver): From 0a1a9d3d70ae19790d9e4a4a3b70a13ba455b421 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:08:13 +0100 Subject: [PATCH 31/55] add all-platform crashpad proxy from env reading --- src/backends/sentry_backend_crashpad.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index a55cb9dcb..b44f3381c 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -440,9 +440,7 @@ crashpad_backend_startup( SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } auto proxy_url = ""; -#ifdef SENTRY_PLATFORM_MACOS - proxy_url = getenv("https_proxy"); // other platforms do this already -#endif + proxy_url = getenv("https_proxy"); proxy_url = options->proxy ? options->proxy : proxy_url != NULL ? proxy_url : ""; From 429cf7c209396a6f546ece8b82a383ab5a6f0065 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 3 Feb 2025 16:56:21 +0100 Subject: [PATCH 32/55] fix crashpad proxy-from-env test --- tests/test_integration_crashpad.py | 14 ++++++-------- tests/test_integration_http.py | 4 +--- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index a2fc7db9e..db77143e6 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -75,7 +75,7 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): assert waiting.result - expected_logsize = 1 + expected_logsize = 1 if port_correct else 0 finally: proxy_test_finally(expected_logsize, httpserver, proxy_process) del os.environ["http_proxy"] @@ -101,6 +101,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later + expected_logsize = 0 try: if proxy_running: @@ -126,23 +127,20 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): except AssertionError: # we fail on macOS/Linux if the http proxy is not running if run_args == ["http-proxy"] and not proxy_running: - assert len(httpserver.log) == 0 + expected_logsize = 0 return # we only fail on linux if the socks5 proxy is not running elif run_args == ["socks5-proxy"] and not proxy_running: - assert len(httpserver.log) == 0 + expected_logsize = 0 return assert waiting.result # Apple's NSURLSession will send the request even if the socks proxy fails # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 - # Windows also provides fallback for http proxies - assert len(httpserver.log) == 1 + expected_logsize = 1 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() + proxy_test_finally(expected_logsize, httpserver, proxy_process) def test_crashpad_reinstall(cmake, httpserver): diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 353934afb..1ec767b57 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -761,8 +761,6 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_running): if proxy_running: expected_logsize = 1 else: - # Windows will send the request even if the proxy is not running - # macOS/Linux will not send the request if the proxy is not running - expected_logsize = 1 if (sys.platform == "win32") else 0 + expected_logsize = 0 finally: proxy_test_finally(expected_logsize, httpserver, proxy_process) From 40482c441361d9f056dd26a5bbfc6f54fca8cce3 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 3 Feb 2025 16:56:47 +0100 Subject: [PATCH 33/55] read proxy from env winHTTP transport --- src/transports/sentry_transport_winhttp.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 70f856bc9..9c3b6460f 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -93,16 +93,21 @@ 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 (getenv("https_proxy")) { + proxy = getenv("https_proxy"); + } else { + proxy = opts->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, opts->proxy); + set_proxy_credentials(state, proxy); } if (slash) { char *copy = sentry__string_clone_n(ptr, slash - ptr); From 391fba10d3a9950ee7daa8cb646b5a09cd8f4fab Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 12:36:16 +0100 Subject: [PATCH 34/55] add non-localhost hostname set/check/usage --- .github/workflows/ci.yml | 16 ++++++++++++++++ tests/__init__.py | 17 ++++++++++++++++- tests/test_integration_crashpad.py | 4 ++-- tests/test_integration_http.py | 10 +++++----- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3fb0e6079..58feafa39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -193,6 +193,22 @@ jobs: run: bash scripts/start-android.sh timeout-minutes: 20 + - name: Add sentry.native.test hostname + # 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: Add sentry.native.test hostname + run: | + echo "127.0.0.1 sentry.native.test" | sudo tee -a /etc/hosts + cat /etc/hosts + shell: bash + + - name: Print hosts file + run: type $env:SystemRoot\System32\drivers\etc\hosts + shell: powershell + - name: Test shell: bash run: | diff --git a/tests/__init__.py b/tests/__init__.py index be5630711..924d6cbcf 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -20,12 +20,19 @@ 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 = url.netloc.replace("127.0.0.1", "sentry.native.test") + check_sentry_native_resolves_to_localhost() + return urllib.parse.urlunsplit( ( url.scheme, @@ -37,6 +44,14 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456): ) +def check_sentry_native_resolves_to_localhost(): + try: + 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 is_proxy_running(host, port): try: with socket.create_connection((host, port), timeout=1): diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index db77143e6..0078ef491 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -57,7 +57,7 @@ def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): # make sure we are isolated from previous runs shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( "OK" ) @@ -113,7 +113,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): # make sure we are isolated from previous runs shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( "OK" ) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 1ec767b57..aa2e8a162 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -596,7 +596,7 @@ def test_capture_minidump(cmake, httpserver): "sentry_example", ["log", "attachment", "capture-minidump"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) assert len(httpserver.log) == 1 @@ -638,7 +638,7 @@ def test_proxy_from_env(cmake, httpserver, port_correct): "sentry_example", ["log", "capture-event"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) if port_correct: @@ -673,7 +673,7 @@ def test_proxy_auth(cmake, httpserver, auth_correct): "sentry_example", ["log", "capture-event", "http-proxy-auth"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) if auth_correct: expected_logsize = 1 @@ -709,7 +709,7 @@ def test_proxy_ipv6(cmake, httpserver): "sentry_example", ["log", "capture-event", "http-proxy-ipv6"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) expected_logsize = 1 @@ -756,7 +756,7 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_running): ["log", "capture-event"] + [current_run_arg], # only passes if given proxy is running check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) if proxy_running: expected_logsize = 1 From d44ae35f9fae8f3a349d0f1101dde330137dfdb5 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 12:38:49 +0100 Subject: [PATCH 35/55] fix CI to be platform specific --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58feafa39..40e231548 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,12 +194,14 @@ jobs: 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: 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 From 61f3ce2c0b3a18705d1ae85fc5622bdd560c959a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 12:41:06 +0100 Subject: [PATCH 36/55] third time's the charm --- .github/workflows/ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40e231548..376a37a17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -200,6 +200,11 @@ jobs: 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: | @@ -207,10 +212,6 @@ jobs: cat /etc/hosts shell: bash - - name: Print hosts file - run: type $env:SystemRoot\System32\drivers\etc\hosts - shell: powershell - - name: Test shell: bash run: | From f65939b0abb3e61d7592c602d0ef8e57704b3d14 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 13:01:28 +0100 Subject: [PATCH 37/55] replace the netloc replace with a string replace --- tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index 924d6cbcf..ba43f16da 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -30,7 +30,7 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False): # 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 = url.netloc.replace("127.0.0.1", "sentry.native.test") + host = host.replace("127.0.0.1", "sentry.native.test") check_sentry_native_resolves_to_localhost() return urllib.parse.urlunsplit( From 7b8a3e9ce910509e60efef0f4b1e94b7d99d1f81 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:01:25 +0100 Subject: [PATCH 38/55] remove accidental proxy host for non-proxy test --- tests/test_integration_http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index aa2e8a162..3df0f89de 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -596,7 +596,7 @@ def test_capture_minidump(cmake, httpserver): "sentry_example", ["log", "attachment", "capture-minidump"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) assert len(httpserver.log) == 1 From 234661c3e343fdfc211b3751ceb09cf080c269ff Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:16:13 +0100 Subject: [PATCH 39/55] add ignore Curl_getaddrinfo_ex to leaks.txt --- tests/leaks.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/leaks.txt b/tests/leaks.txt index 4772a4261..7e4d81e04 100644 --- a/tests/leaks.txt +++ b/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 \ No newline at end of file From b9f07cdf3d1591ec1e1df0f286a3b9ca88d0a038 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 11:27:35 +0100 Subject: [PATCH 40/55] refactor proxy tests --- CHANGELOG.md | 2 +- tests/__init__.py | 61 +----------- tests/proxy.py | 73 ++++++++++++++ tests/test_integration_crashpad.py | 94 ++++++++++-------- tests/test_integration_http.py | 152 ++++++++++++++++++----------- 5 files changed, 222 insertions(+), 160 deletions(-) create mode 100644 tests/proxy.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 22e02bf69..c6d3d4eeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ **Features**: -- Honor `http(s)_proxy` environment variables on macOS crashpad transport. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) +- Honor `https_proxy` environment variable on crashpad transport. ([#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)) diff --git a/tests/__init__.py b/tests/__init__.py index ba43f16da..a3e2aea4e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -8,13 +8,10 @@ import pytest import pprint import textwrap -import time import socket - 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 @@ -31,7 +28,7 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False): # 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() + _check_sentry_native_resolves_to_localhost() return urllib.parse.urlunsplit( ( @@ -44,7 +41,7 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False): ) -def check_sentry_native_resolves_to_localhost(): +def _check_sentry_native_resolves_to_localhost(): try: resolved_ip = socket.gethostbyname("sentry.native.test") assert resolved_ip == "127.0.0.1" @@ -52,60 +49,6 @@ def check_sentry_native_resolves_to_localhost(): pytest.skip("sentry.native.test does not resolve to localhost") -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 - 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 go 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 - - def run(cwd, exe, args, env=dict(os.environ), **kwargs): __tracebackhide__ = True if os.environ.get("ANDROID_API"): diff --git a/tests/proxy.py b/tests/proxy.py new file mode 100644 index 000000000..2ad82b772 --- /dev/null +++ b/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 diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 0078ef491..ddfb2343a 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -6,7 +6,17 @@ import pytest -from . import make_dsn, run, Envelope, start_mitmdump, proxy_test_finally +from . import ( + make_dsn, + run, + Envelope, +) +from .proxy import ( + setup_proxy_env_vars, + cleanup_proxy_env_vars, + start_mitmdump, + proxy_test_finally, +) from .assertions import ( assert_crashpad_upload, assert_session, @@ -39,47 +49,58 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 -@pytest.mark.parametrize("port_correct", [True, False]) # TODO separate tests -def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct): +def _setup_crashpad_proxy_test(cmake, httpserver, proxy): + proxy_process = start_mitmdump(proxy) if proxy else None + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) + httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") + + return env, proxy_process, tmp_path + + +def test_crashpad_crash_proxy_env(cmake, httpserver): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later - port = "8080" if port_correct else "8081" - os.environ["http_proxy"] = f"http://localhost:{port}" - os.environ["https_proxy"] = f"http://localhost:{port}" - expected_logsize = 0 + setup_proxy_env_vars(port=8080) try: - proxy_process = start_mitmdump("http-proxy") + env, proxy_process, tmp_path = _setup_crashpad_proxy_test( + cmake, httpserver, "http-proxy" + ) + + with httpserver.wait(timeout=10) as waiting: + child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) + assert child.returncode # well, it's a crash after all + assert waiting.result + finally: + proxy_test_finally(1, httpserver, proxy_process) + cleanup_proxy_env_vars() - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) +def test_crashpad_crash_proxy_env_port_incorrect(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") - env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) - httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( - "OK" + proxy_process = None # store the proxy process to terminate it later + setup_proxy_env_vars(port=8081) + try: + env, proxy_process, tmp_path = _setup_crashpad_proxy_test( + cmake, httpserver, "http-proxy" ) - try: - with httpserver.wait(timeout=10) as waiting: + with pytest.raises(AssertionError): + with httpserver.wait(timeout=10): child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) assert child.returncode # well, it's a crash after all - except AssertionError as e: - if port_correct: - raise e - else: - expected_logsize = 0 - return - - assert waiting.result - - expected_logsize = 1 if port_correct else 0 finally: - proxy_test_finally(expected_logsize, httpserver, proxy_process) - del os.environ["http_proxy"] - del os.environ["https_proxy"] + proxy_test_finally(0, httpserver, proxy_process) + cleanup_proxy_env_vars() @pytest.mark.parametrize( @@ -104,18 +125,9 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): expected_logsize = 0 try: - if proxy_running: - # start mitmdump from terminal - proxy_process = start_mitmdump(run_args[0]) - - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) - - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - - env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) - httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( - "OK" + proxy_to_start = run_args[0] if proxy_running else None + env, proxy_process, tmp_path = _setup_crashpad_proxy_test( + cmake, httpserver, proxy_to_start ) try: diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 3df0f89de..11f7e8e33 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -9,7 +9,17 @@ import pytest -from . import make_dsn, run, Envelope, start_mitmdump, proxy_test_finally +from . import ( + make_dsn, + run, + Envelope, +) +from .proxy import ( + setup_proxy_env_vars, + cleanup_proxy_env_vars, + start_mitmdump, + proxy_test_finally, +) from .assertions import ( assert_attachment, assert_meta, @@ -612,61 +622,77 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) -@pytest.mark.parametrize("port_correct", [True, False]) -def test_proxy_from_env(cmake, httpserver, port_correct): - # TODO parametrize to only set http_proxy/https_proxy/empty? +def _setup_http_proxy_test(cmake, httpserver, proxy, proxy_auth=None): + proxy_process = start_mitmdump(proxy, proxy_auth) if proxy else None + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)) + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + return env, proxy_process, tmp_path + + +def test_proxy_from_env(cmake, httpserver): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later - port = "8080" if port_correct else "8081" - os.environ["http_proxy"] = f"http://localhost:{port}" - os.environ["https_proxy"] = f"http://localhost:{port}" - expected_logsize = 0 + setup_proxy_env_vars(port=8080) try: - proxy_process = start_mitmdump("http-proxy") + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy" + ) - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + run( + tmp_path, + "sentry_example", + ["log", "capture-event"], + check=True, + env=env, + ) + + finally: + proxy_test_finally(1, httpserver, proxy_process) + cleanup_proxy_env_vars() - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") +def test_proxy_from_env_port_incorrect(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + setup_proxy_env_vars(port=8081) + try: + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy" + ) run( tmp_path, "sentry_example", ["log", "capture-event"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), + env=env, ) - if port_correct: - expected_logsize = 1 - else: - expected_logsize = 0 finally: - proxy_test_finally(expected_logsize, httpserver, proxy_process) - del os.environ["http_proxy"] - del os.environ["https_proxy"] + proxy_test_finally(0, httpserver, proxy_process) + cleanup_proxy_env_vars() -@pytest.mark.parametrize("auth_correct", [True, False]) -def test_proxy_auth(cmake, httpserver, auth_correct): +def test_proxy_auth(cmake, httpserver): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later - expected_logsize = 0 try: - proxy_auth = "user:password" if auth_correct else "wrong:wrong" - proxy_process = start_mitmdump("http-proxy", proxy_auth) - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) - - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy", proxy_auth="user:password" + ) run( tmp_path, @@ -675,46 +701,61 @@ def test_proxy_auth(cmake, httpserver, auth_correct): check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) - if auth_correct: - expected_logsize = 1 - else: - expected_logsize = 0 finally: proxy_test_finally( - expected_logsize, + 1, httpserver, proxy_process, assert_failed_proxy_auth_request, ) -def test_proxy_ipv6(cmake, httpserver): +def test_proxy_auth_incorrect(cmake, httpserver): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") proxy_process = None # store the proxy process to terminate it later - expected_logsize = 0 try: - proxy_process = start_mitmdump("http-proxy") + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy", proxy_auth="wrong:wrong" + ) + + run( + tmp_path, + "sentry_example", + ["log", "capture-event", "http-proxy-auth"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), + ) + finally: + proxy_test_finally( + 0, + httpserver, + proxy_process, + assert_failed_proxy_auth_request, + ) - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) +def test_proxy_ipv6(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + proxy_process = None # store the proxy process to terminate it later + try: + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy" + ) run( tmp_path, "sentry_example", ["log", "capture-event", "http-proxy-ipv6"], check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), + env=env, ) - expected_logsize = 1 finally: - proxy_test_finally(expected_logsize, httpserver, proxy_process) + proxy_test_finally(1, httpserver, proxy_process) @pytest.mark.parametrize( @@ -739,22 +780,15 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_running): expected_logsize = 0 try: - if proxy_running: - # start mitmdump from terminal - proxy_process = start_mitmdump(run_args[0]) - - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) - - # make sure we are isolated from previous runs - shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) - - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - current_run_arg = run_args[0] + proxy_to_start = run_args[0] if proxy_running else None + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, proxy_to_start + ) run( tmp_path, "sentry_example", ["log", "capture-event"] - + [current_run_arg], # only passes if given proxy is running + + run_args, # only passes if given proxy is running check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True)), ) From 3e54dc4aa2be763b604228d3ae075809aebf91f6 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 11:52:29 +0100 Subject: [PATCH 41/55] remove incorrect 'fallback' logic comments + update CONTRIBUTING.md - wrong bc of localhost-to-localhost proxy ignore in macOS --- CONTRIBUTING.md | 3 +++ tests/test_integration_crashpad.py | 12 ++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1c4d33bf..d06331af5 100644 --- a/CONTRIBUTING.md +++ b/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/ diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index ddfb2343a..5e8258b07 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -137,19 +137,11 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running): ) assert child.returncode # well, it's a crash after all except AssertionError: - # we fail on macOS/Linux if the http proxy is not running - if run_args == ["http-proxy"] and not proxy_running: - expected_logsize = 0 - return - # we only fail on linux if the socks5 proxy is not running - elif run_args == ["socks5-proxy"] and not proxy_running: - expected_logsize = 0 - return + expected_logsize = 0 + return assert waiting.result - # Apple's NSURLSession will send the request even if the socks proxy fails - # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 expected_logsize = 1 finally: proxy_test_finally(expected_logsize, httpserver, proxy_process) From 49c335dbbb2492eaf088b43575018735a960feba Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 13:06:02 +0100 Subject: [PATCH 42/55] add `http_proxy` reading --- src/backends/sentry_backend_crashpad.cpp | 6 +++++- src/transports/sentry_transport_winhttp.c | 7 ++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index b44f3381c..09c160eff 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -440,7 +440,11 @@ crashpad_backend_startup( SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } auto proxy_url = ""; - proxy_url = getenv("https_proxy"); + 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 : ""; diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 9c3b6460f..12eb67c0f 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -94,11 +94,12 @@ sentry__winhttp_transport_start( state->debug = opts->debug; char *proxy = ""; - if (getenv("https_proxy")) { + if (strncmp(opts->dsn->raw, "https", 5) == 0) { proxy = getenv("https_proxy"); - } else { - proxy = opts->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 (proxy && strstr(proxy, "http://") == proxy) { From c0cce33643e0416397940e4693b38970797e2562 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 13:07:06 +0100 Subject: [PATCH 43/55] update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6d3d4eeb..93150f913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ **Features**: -- Honor `https_proxy` environment variable on crashpad transport. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) +- 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)) From f877b67fdc0ee9ea7846c23e879cd85dedb8e177 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 13:28:41 +0100 Subject: [PATCH 44/55] read is_secure from dsn --- src/backends/sentry_backend_crashpad.cpp | 4 ++-- src/transports/sentry_transport_winhttp.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 09c160eff..8a0ea4aa8 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -440,9 +440,9 @@ crashpad_backend_startup( SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } auto proxy_url = ""; - if (strncmp(options->dsn->raw, "https", 5) == 0) { + if (options->dsn->is_secure) { proxy_url = getenv("https_proxy"); - } else if (strncmp(options->dsn->raw, "http", 4) == 0) { + } else { proxy_url = getenv("http_proxy"); } proxy_url = options->proxy ? options->proxy diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 12eb67c0f..c67cab32c 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -94,9 +94,9 @@ sentry__winhttp_transport_start( state->debug = opts->debug; char *proxy = ""; - if (strncmp(opts->dsn->raw, "https", 5) == 0) { + if (opts->dsn->is_secure) { proxy = getenv("https_proxy"); - } else if (strncmp(opts->dsn->raw, "http", 4) == 0) { + } else { proxy = getenv("http_proxy"); } proxy = opts->proxy ? opts->proxy : proxy != NULL ? proxy : ""; From 46ad5b0b120adb0dbc2f79f2de8e040c15fc6da8 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 14:07:09 +0100 Subject: [PATCH 45/55] cleaner proxy reading --- src/backends/sentry_backend_crashpad.cpp | 14 +++++--------- src/transports/sentry_transport_winhttp.c | 10 +++------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 8a0ea4aa8..3c0023e5a 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,15 +439,11 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } - auto proxy_url = ""; - if (options->dsn->is_secure) { - proxy_url = getenv("https_proxy"); - } else { - proxy_url = getenv("http_proxy"); - } - proxy_url = options->proxy ? options->proxy - : proxy_url != NULL ? proxy_url - : ""; + const char *env_proxy + = getenv(options->dsn->is_secure ? "https_proxy" : "http_proxy"); + const char *proxy_url = options->proxy ? options->proxy + : env_proxy ? env_proxy + : ""; bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index c67cab32c..ad3b003c9 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -93,13 +93,9 @@ sentry__winhttp_transport_start( state->user_agent = sentry__string_to_wstr(opts->user_agent); state->debug = opts->debug; - char *proxy = ""; - if (opts->dsn->is_secure) { - proxy = getenv("https_proxy"); - } else { - proxy = getenv("http_proxy"); - } - proxy = opts->proxy ? opts->proxy : proxy != NULL ? proxy : ""; + const char *env_proxy + = getenv(opts->dsn->is_secure ? "https_proxy" : "http_proxy"); + const char *proxy = opts->proxy ? opts->proxy : env_proxy ? env_proxy : ""; // ensure the proxy starts with `http://`, otherwise ignore it if (proxy && strstr(proxy, "http://") == proxy) { From a2508cde69377c479cd8484a347248c17a8a912c Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 14:51:56 +0100 Subject: [PATCH 46/55] check null dsn --- src/backends/sentry_backend_crashpad.cpp | 5 +++-- src/transports/sentry_transport_winhttp.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 3c0023e5a..afa0a6928 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,8 +439,9 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } - const char *env_proxy - = getenv(options->dsn->is_secure ? "https_proxy" : "http_proxy"); + const char *env_proxy = options->dsn + ? getenv(options->dsn->is_secure ? "https_proxy" : "http_proxy") + : nullptr; const char *proxy_url = options->proxy ? options->proxy : env_proxy ? env_proxy : ""; diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index ad3b003c9..fcb5cf878 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -93,8 +93,9 @@ sentry__winhttp_transport_start( state->user_agent = sentry__string_to_wstr(opts->user_agent); state->debug = opts->debug; - const char *env_proxy - = getenv(opts->dsn->is_secure ? "https_proxy" : "http_proxy"); + const char *env_proxy = opts->dsn + ? getenv(opts->dsn->is_secure ? "https_proxy" : "http_proxy") + : NULL; const char *proxy = opts->proxy ? opts->proxy : env_proxy ? env_proxy : ""; // ensure the proxy starts with `http://`, otherwise ignore it From 1b857ecd6944d78d0e4498e75ece8ffa7d1b1663 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 15:47:14 +0100 Subject: [PATCH 47/55] add tests --- examples/example.c | 3 ++ tests/proxy.py | 10 ++++-- tests/test_integration_crashpad.py | 45 +++++++++++++++++++++++++++ tests/test_integration_http.py | 49 ++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/examples/example.c b/examples/example.c index da5546767..6ac07fde6 100644 --- a/examples/example.c +++ b/examples/example.c @@ -278,6 +278,9 @@ main(int argc, char **argv) if (has_arg(argc, argv, "http-proxy-ipv6")) { sentry_options_set_proxy(options, "http://[::1]:8080"); } + if (has_arg(argc, argv, "proxy-empty")) { + sentry_options_set_proxy(options, ""); + } if (has_arg(argc, argv, "socks5-proxy")) { sentry_options_set_proxy(options, "socks5://127.0.0.1:1080"); diff --git a/tests/proxy.py b/tests/proxy.py index 2ad82b772..265027abe 100644 --- a/tests/proxy.py +++ b/tests/proxy.py @@ -53,21 +53,25 @@ def start_mitmdump(proxy_type, proxy_auth: str = None): def proxy_test_finally( - expected_logsize, + expected_httpserver_logsize, httpserver, proxy_process, proxy_log_assert=assert_no_proxy_request, + expected_proxy_logsize=None, ): + if expected_proxy_logsize is None: + expected_proxy_logsize = expected_httpserver_logsize + 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: + if expected_proxy_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 + assert len(httpserver.log) == expected_httpserver_logsize diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 5e8258b07..1169a435c 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -103,6 +103,51 @@ def test_crashpad_crash_proxy_env_port_incorrect(cmake, httpserver): cleanup_proxy_env_vars() +def test_crashpad_proxy_set_empty(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + setup_proxy_env_vars(port=8080) # we start the proxy but expect it to remain unused + try: + env, proxy_process, tmp_path = _setup_crashpad_proxy_test( + cmake, httpserver, "http-proxy" + ) + + with httpserver.wait(timeout=10) as waiting: + child = run( + tmp_path, "sentry_example", ["log", "crash", "proxy-empty"], env=env + ) + assert child.returncode # well, it's a crash after all + assert waiting.result + + finally: + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + cleanup_proxy_env_vars() + + +def test_crashpad_proxy_https_not_http(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + # we start the proxy but expect it to remain unused (dsn is http, so shouldn't use https proxy) + os.environ["https_proxy"] = f"http://localhost:8080" + try: + env, proxy_process, tmp_path = _setup_crashpad_proxy_test( + cmake, httpserver, "http-proxy" + ) + + with httpserver.wait(timeout=10) as waiting: + child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) + assert child.returncode # well, it's a crash after all + assert waiting.result + + finally: + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + del os.environ["https_proxy"] + + @pytest.mark.parametrize( "run_args", [ diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 11f7e8e33..bc5f453fd 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -758,6 +758,55 @@ def test_proxy_ipv6(cmake, httpserver): proxy_test_finally(1, httpserver, proxy_process) +def test_proxy_set_empty(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + setup_proxy_env_vars(port=8080) # we start the proxy but expect it to remain unused + try: + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy" + ) + + run( + tmp_path, + "sentry_example", + ["log", "capture-event", "proxy-empty"], + check=True, + env=env, + ) + + finally: + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + cleanup_proxy_env_vars() + + +def test_proxy_https_not_http(cmake, httpserver): + if not shutil.which("mitmdump"): + pytest.skip("mitmdump is not installed") + + proxy_process = None # store the proxy process to terminate it later + # we start the proxy but expect it to remain unused (dsn is http, so shouldn't use https proxy) + os.environ["https_proxy"] = f"http://localhost:8080" + try: + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy" + ) + + run( + tmp_path, + "sentry_example", + ["log", "capture-event"], + check=True, + env=env, + ) + + finally: + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + del os.environ["https_proxy"] + + @pytest.mark.parametrize( "run_args", [ From fedd7fe9963a4c18144358cf554fca1062ee011d Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 5 Feb 2025 17:04:13 +0100 Subject: [PATCH 48/55] change env clean order --- tests/test_integration_crashpad.py | 8 ++++---- tests/test_integration_http.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 1169a435c..4795c3aa2 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -79,8 +79,8 @@ def test_crashpad_crash_proxy_env(cmake, httpserver): assert child.returncode # well, it's a crash after all assert waiting.result finally: - proxy_test_finally(1, httpserver, proxy_process) cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process) def test_crashpad_crash_proxy_env_port_incorrect(cmake, httpserver): @@ -99,8 +99,8 @@ def test_crashpad_crash_proxy_env_port_incorrect(cmake, httpserver): child = run(tmp_path, "sentry_example", ["log", "crash"], env=env) assert child.returncode # well, it's a crash after all finally: - proxy_test_finally(0, httpserver, proxy_process) cleanup_proxy_env_vars() + proxy_test_finally(0, httpserver, proxy_process) def test_crashpad_proxy_set_empty(cmake, httpserver): @@ -122,8 +122,8 @@ def test_crashpad_proxy_set_empty(cmake, httpserver): assert waiting.result finally: - proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) def test_crashpad_proxy_https_not_http(cmake, httpserver): @@ -144,8 +144,8 @@ def test_crashpad_proxy_https_not_http(cmake, httpserver): assert waiting.result finally: - proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) del os.environ["https_proxy"] + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) @pytest.mark.parametrize( diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index bc5f453fd..de9403f57 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -656,8 +656,8 @@ def test_proxy_from_env(cmake, httpserver): ) finally: - proxy_test_finally(1, httpserver, proxy_process) cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process) def test_proxy_from_env_port_incorrect(cmake, httpserver): @@ -680,8 +680,8 @@ def test_proxy_from_env_port_incorrect(cmake, httpserver): ) finally: - proxy_test_finally(0, httpserver, proxy_process) cleanup_proxy_env_vars() + proxy_test_finally(0, httpserver, proxy_process) def test_proxy_auth(cmake, httpserver): @@ -778,8 +778,8 @@ def test_proxy_set_empty(cmake, httpserver): ) finally: - proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) def test_proxy_https_not_http(cmake, httpserver): @@ -803,8 +803,8 @@ def test_proxy_https_not_http(cmake, httpserver): ) finally: - proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) del os.environ["https_proxy"] + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) @pytest.mark.parametrize( From a526df18b73e359be33ccf05a96efb5c523eefd1 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:18:46 +0100 Subject: [PATCH 49/55] update crashpad to handle empty linux proxy --- external/crashpad | 2 +- src/backends/sentry_backend_crashpad.cpp | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 4cc763c2d..e09e9b2bd 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 4cc763c2d292c5581f065f88f1d5eebeb581f4ac +Subproject commit e09e9b2bdb51e78e6b3c9add5a72e7835d0090b3 diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index afa0a6928..c67fe0975 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -445,6 +445,12 @@ crashpad_backend_startup( const char *proxy_url = options->proxy ? options->proxy : env_proxy ? env_proxy : ""; +#ifdef SENTRY_PLATFORM_LINUX + // explicitly set an empty proxy to avoid reading from env. vars. on Linux + if (options->proxy == "") { + proxy_url = "" + } +#endif bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, From 142acef925d0072c6c0dcf362d1c58785472a2e5 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:23:15 +0100 Subject: [PATCH 50/55] forgot ; --- src/backends/sentry_backend_crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index c67fe0975..6f78306ca 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -448,7 +448,7 @@ crashpad_backend_startup( #ifdef SENTRY_PLATFORM_LINUX // explicitly set an empty proxy to avoid reading from env. vars. on Linux if (options->proxy == "") { - proxy_url = "" + proxy_url = ""; } #endif bool success = data->client->StartHandler(handler, database, database, From 882c64022851287afede1267af5076820a1655ad Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:39:13 +0100 Subject: [PATCH 51/55] linux fix with no_proxy + revert crashpad --- external/crashpad | 2 +- src/backends/sentry_backend_crashpad.cpp | 6 ------ tests/test_integration_crashpad.py | 5 +++++ 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/external/crashpad b/external/crashpad index e09e9b2bd..4cc763c2d 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit e09e9b2bdb51e78e6b3c9add5a72e7835d0090b3 +Subproject commit 4cc763c2d292c5581f065f88f1d5eebeb581f4ac diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 6f78306ca..afa0a6928 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -445,12 +445,6 @@ crashpad_backend_startup( const char *proxy_url = options->proxy ? options->proxy : env_proxy ? env_proxy : ""; -#ifdef SENTRY_PLATFORM_LINUX - // explicitly set an empty proxy to avoid reading from env. vars. on Linux - if (options->proxy == "") { - proxy_url = ""; - } -#endif bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 4795c3aa2..1fc2f4148 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -109,6 +109,9 @@ def test_crashpad_proxy_set_empty(cmake, httpserver): proxy_process = None # store the proxy process to terminate it later setup_proxy_env_vars(port=8080) # we start the proxy but expect it to remain unused + # on linux, set empty doesn't work; one needs to set no_proxy + if sys.platform == "linux": + os.environ["no_proxy"] = "sentry.native.test" try: env, proxy_process, tmp_path = _setup_crashpad_proxy_test( cmake, httpserver, "http-proxy" @@ -123,6 +126,8 @@ def test_crashpad_proxy_set_empty(cmake, httpserver): finally: cleanup_proxy_env_vars() + if sys.platform == "linux": + del os.environ["no_proxy"] proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) From 969ed4116ddd5f4f17d1de11e6fe948dca40376f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 13:11:11 +0100 Subject: [PATCH 52/55] return to proxy workaround for Linux crashpad transport --- external/crashpad | 2 +- src/backends/sentry_backend_crashpad.cpp | 6 ++++++ tests/test_integration_crashpad.py | 5 ----- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/external/crashpad b/external/crashpad index 4cc763c2d..8a447fc9d 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 4cc763c2d292c5581f065f88f1d5eebeb581f4ac +Subproject commit 8a447fc9d0ca921dd5654c3ce18779f455c218b1 diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index afa0a6928..6f78306ca 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -445,6 +445,12 @@ crashpad_backend_startup( const char *proxy_url = options->proxy ? options->proxy : env_proxy ? env_proxy : ""; +#ifdef SENTRY_PLATFORM_LINUX + // explicitly set an empty proxy to avoid reading from env. vars. on Linux + if (options->proxy == "") { + proxy_url = ""; + } +#endif bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", proxy_url, annotations, arguments, /* restartable */ true, diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 1fc2f4148..4795c3aa2 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -109,9 +109,6 @@ def test_crashpad_proxy_set_empty(cmake, httpserver): proxy_process = None # store the proxy process to terminate it later setup_proxy_env_vars(port=8080) # we start the proxy but expect it to remain unused - # on linux, set empty doesn't work; one needs to set no_proxy - if sys.platform == "linux": - os.environ["no_proxy"] = "sentry.native.test" try: env, proxy_process, tmp_path = _setup_crashpad_proxy_test( cmake, httpserver, "http-proxy" @@ -126,8 +123,6 @@ def test_crashpad_proxy_set_empty(cmake, httpserver): finally: cleanup_proxy_env_vars() - if sys.platform == "linux": - del os.environ["no_proxy"] proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) From 5bad4d4d383992e7eb3c218e8780a5114fddc9b6 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:08:31 +0100 Subject: [PATCH 53/55] strcmp --- src/backends/sentry_backend_crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 6f78306ca..484a49475 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -447,7 +447,7 @@ crashpad_backend_startup( : ""; #ifdef SENTRY_PLATFORM_LINUX // explicitly set an empty proxy to avoid reading from env. vars. on Linux - if (options->proxy == "") { + if (strcmp(options->proxy, "") == 0) { proxy_url = ""; } #endif From 45a9f356db1ca8720f0523e7dbdd55fc482a2580 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:46:33 +0100 Subject: [PATCH 54/55] pointer check --- src/backends/sentry_backend_crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 484a49475..85de1bf71 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -447,7 +447,7 @@ crashpad_backend_startup( : ""; #ifdef SENTRY_PLATFORM_LINUX // explicitly set an empty proxy to avoid reading from env. vars. on Linux - if (strcmp(options->proxy, "") == 0) { + if (options->proxy && strcmp(options->proxy, "") == 0) { proxy_url = ""; } #endif From 93678e063f9d6a2b68be912a624f6e9335e8b703 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 6 Feb 2025 15:40:43 +0100 Subject: [PATCH 55/55] update CONTRIBUTING.md --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d06331af5..c9bf5ac8b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -152,6 +152,7 @@ The example currently supports the following commands: - `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. +- `proxy-empty`: Sets the `proxy` option to the empty string `""`. - `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`.