From d65feb19d722db0b0d379a1652c2ee379af63440 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:47:17 +0100 Subject: [PATCH 01/37] Added socks5_proxy to options --- include/sentry.h | 16 ++++++++++++++++ src/sentry_options.c | 21 +++++++++++++++++++++ src/sentry_options.h | 1 + 3 files changed, 38 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index 0d3b8b83b..3d3b3dccf 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -979,6 +979,22 @@ SENTRY_API void sentry_options_set_http_proxy_n( SENTRY_API const char *sentry_options_get_http_proxy( const sentry_options_t *opts); +/** + * Configures the socks5 proxy. + * + * The given proxy has to include the full scheme, eg. `socks5://some.proxy/`. + */ +SENTRY_API void sentry_options_set_socks5_proxy( + sentry_options_t *opts, const char *proxy); +SENTRY_API void sentry_options_set_socks5_proxy_n( + sentry_options_t *opts, const char *proxy, size_t proxy_len); + +/** + * Returns the configured socks5 proxy. + */ +SENTRY_API const char *sentry_options_get_socks5_proxy( + const sentry_options_t *opts); + /** * Configures the path to a file containing ssl certificates for * verification. diff --git a/src/sentry_options.c b/src/sentry_options.c index 014ec491f..7ddc11c02 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -255,6 +255,27 @@ sentry_options_get_http_proxy(const sentry_options_t *opts) return opts->http_proxy; } +void +sentry_options_set_socks5_proxy_n( + sentry_options_t *opts, const char *proxy, size_t proxy_len) +{ + sentry_free(opts->http_proxy); + opts->socks5_proxy = sentry__string_clone_n(proxy, proxy_len); +} + +void +sentry_options_set_socks5_proxy(sentry_options_t *opts, const char *proxy) +{ + sentry_free(opts->http_proxy); + opts->socks5_proxy = sentry__string_clone(proxy); +} + +const char * +sentry_options_get_socks5_proxy(const sentry_options_t *opts) +{ + return opts->socks5_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 060b17f70..717a6fd02 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -36,6 +36,7 @@ typedef struct sentry_options_s { char *environment; char *dist; char *http_proxy; + char *socks5_proxy; char *ca_certs; char *transport_thread_name; char *sdk_name; From e9b61a6c7c31546c7862ab274e695b9ae078e249 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:57:10 +0100 Subject: [PATCH 02/37] correct options freeing --- src/sentry_options.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry_options.c b/src/sentry_options.c index 7ddc11c02..bc924c606 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -259,14 +259,14 @@ void sentry_options_set_socks5_proxy_n( sentry_options_t *opts, const char *proxy, size_t proxy_len) { - sentry_free(opts->http_proxy); + sentry_free(opts->socks5_proxy); opts->socks5_proxy = sentry__string_clone_n(proxy, proxy_len); } void sentry_options_set_socks5_proxy(sentry_options_t *opts, const char *proxy) { - sentry_free(opts->http_proxy); + sentry_free(opts->socks5_proxy); opts->socks5_proxy = sentry__string_clone(proxy); } From b3f8c769e51684c1e6f8473c0f066b7c764779c9 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 10:37:12 +0100 Subject: [PATCH 03/37] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8def114fe..2b4d587b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +**Features**: +- Add SOCKS5 proxy support ([#1063](https://github.com/getsentry/sentry-native/pull/1063)) + ## 0.7.11 **Fixes**: From ca446211908fa1ea360d246529fc0c5471a412fc Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 10:37:36 +0100 Subject: [PATCH 04/37] Add socks5_proxy attribute to curl_transport_state --- src/transports/sentry_transport_curl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index 11f87e5d2..80b3a5014 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -18,6 +18,7 @@ typedef struct curl_transport_state_s { CURL *curl_handle; char *user_agent; char *http_proxy; + char *socks5_proxy; char *ca_certs; sentry_rate_limiter_t *ratelimiter; bool debug; @@ -55,6 +56,7 @@ sentry__curl_bgworker_state_free(void *_state) sentry_free(state->ca_certs); sentry_free(state->user_agent); sentry_free(state->http_proxy); + sentry_free(state->socks5_proxy); sentry_free(state); } @@ -102,6 +104,7 @@ sentry__curl_transport_start( state->dsn = sentry__dsn_incref(options->dsn); state->http_proxy = sentry__string_clone(options->http_proxy); + state->socks5_proxy = sentry__string_clone(options->socks5_proxy); state->user_agent = sentry__string_clone(options->user_agent); state->ca_certs = sentry__string_clone(options->ca_certs); state->curl_handle = curl_easy_init(); @@ -218,6 +221,9 @@ sentry__curl_send_task(void *_envelope, void *_state) if (state->http_proxy) { curl_easy_setopt(curl, CURLOPT_PROXY, state->http_proxy); } + if (state->socks5_proxy) { + curl_easy_setopt(curl ,CURLOPT_PROXY, state->socks5_proxy); + } if (state->ca_certs) { curl_easy_setopt(curl, CURLOPT_CAINFO, state->ca_certs); } From 3287b6465dbd5b3296af39fb9eccc3793ab71d03 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 10:50:24 +0100 Subject: [PATCH 05/37] formatting --- src/transports/sentry_transport_curl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index 80b3a5014..3d276c268 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -222,7 +222,7 @@ sentry__curl_send_task(void *_envelope, void *_state) curl_easy_setopt(curl, CURLOPT_PROXY, state->http_proxy); } if (state->socks5_proxy) { - curl_easy_setopt(curl ,CURLOPT_PROXY, state->socks5_proxy); + curl_easy_setopt(curl, CURLOPT_PROXY, state->socks5_proxy); } if (state->ca_certs) { curl_easy_setopt(curl, CURLOPT_CAINFO, state->ca_certs); From ad6ef46f86e1500142bbe0016aeca30c1ac73eeb Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:48:06 +0100 Subject: [PATCH 06/37] refactor to use single proxy attribute --- include/sentry.h | 29 +++++------------- src/backends/sentry_backend_crashpad.cpp | 4 +-- src/sentry_options.c | 39 ++++++------------------ src/sentry_options.h | 3 +- src/transports/sentry_transport_curl.c | 16 +++------- tests/unit/test_uninit.c | 2 +- 6 files changed, 25 insertions(+), 68 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 3d3b3dccf..acd052338 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -964,35 +964,20 @@ SENTRY_API void sentry_options_set_dist_n( SENTRY_API const char *sentry_options_get_dist(const sentry_options_t *opts); /** - * Configures the http proxy. + * Configures the proxy. * - * The given proxy has to include the full scheme, eg. `http://some.proxy/`. + * The given proxy has to include the full scheme, + * eg. `http://some.proxy/`or 'socks5://some.proxy/'. */ -SENTRY_API void sentry_options_set_http_proxy( +SENTRY_API void sentry_options_set_proxy( sentry_options_t *opts, const char *proxy); -SENTRY_API void sentry_options_set_http_proxy_n( +SENTRY_API void sentry_options_set_proxy_n( sentry_options_t *opts, const char *proxy, size_t proxy_len); /** - * Returns the configured http proxy. + * Returns the configured proxy. */ -SENTRY_API const char *sentry_options_get_http_proxy( - const sentry_options_t *opts); - -/** - * Configures the socks5 proxy. - * - * The given proxy has to include the full scheme, eg. `socks5://some.proxy/`. - */ -SENTRY_API void sentry_options_set_socks5_proxy( - sentry_options_t *opts, const char *proxy); -SENTRY_API void sentry_options_set_socks5_proxy_n( - sentry_options_t *opts, const char *proxy, size_t proxy_len); - -/** - * Returns the configured socks5 proxy. - */ -SENTRY_API const char *sentry_options_get_socks5_proxy( +SENTRY_API const char *sentry_options_get_proxy( const sentry_options_t *opts); /** diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 49e93a160..2a98f12b4 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,9 +439,9 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_TRACEF("using minidump URL \"%s\"", minidump_url); } - bool success = data->client->StartHandler(handler, database, database, + bool success = data->client->StartHandler(handler, database, database, //TODO update to not only use http_proxy minidump_url ? minidump_url : "", - options->http_proxy ? options->http_proxy : "", annotations, arguments, + options->proxy ? options->proxy : "", annotations, arguments, /* restartable */ true, /* asynchronous_start */ false, attachments); sentry_free(minidump_url); diff --git a/src/sentry_options.c b/src/sentry_options.c index bc924c606..dc464dd1a 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -89,7 +89,7 @@ sentry_options_free(sentry_options_t *opts) sentry_free(opts->user_agent); sentry_free(opts->environment); sentry_free(opts->dist); - sentry_free(opts->http_proxy); + sentry_free(opts->proxy); sentry_free(opts->ca_certs); sentry_free(opts->transport_thread_name); sentry__path_free(opts->database_path); @@ -235,45 +235,24 @@ sentry_options_get_dist(const sentry_options_t *opts) } void -sentry_options_set_http_proxy_n( +sentry_options_set_proxy_n( sentry_options_t *opts, const char *proxy, size_t proxy_len) { - sentry_free(opts->http_proxy); - opts->http_proxy = sentry__string_clone_n(proxy, proxy_len); + sentry_free(opts->proxy); + opts->proxy = sentry__string_clone_n(proxy, proxy_len); } void -sentry_options_set_http_proxy(sentry_options_t *opts, const char *proxy) +sentry_options_set_proxy(sentry_options_t *opts, const char *proxy) { - sentry_free(opts->http_proxy); - opts->http_proxy = sentry__string_clone(proxy); + sentry_free(opts->proxy); + opts->proxy = sentry__string_clone(proxy); } const char * -sentry_options_get_http_proxy(const sentry_options_t *opts) +sentry_options_get_proxy(const sentry_options_t *opts) { - return opts->http_proxy; -} - -void -sentry_options_set_socks5_proxy_n( - sentry_options_t *opts, const char *proxy, size_t proxy_len) -{ - sentry_free(opts->socks5_proxy); - opts->socks5_proxy = sentry__string_clone_n(proxy, proxy_len); -} - -void -sentry_options_set_socks5_proxy(sentry_options_t *opts, const char *proxy) -{ - sentry_free(opts->socks5_proxy); - opts->socks5_proxy = sentry__string_clone(proxy); -} - -const char * -sentry_options_get_socks5_proxy(const sentry_options_t *opts) -{ - return opts->socks5_proxy; + return opts->proxy; } void diff --git a/src/sentry_options.h b/src/sentry_options.h index 717a6fd02..b077fc0bc 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -35,8 +35,7 @@ typedef struct sentry_options_s { char *release; char *environment; char *dist; - char *http_proxy; - char *socks5_proxy; + char *proxy; char *ca_certs; char *transport_thread_name; char *sdk_name; diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index 3d276c268..f38986ec2 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -17,8 +17,7 @@ typedef struct curl_transport_state_s { sentry_dsn_t *dsn; CURL *curl_handle; char *user_agent; - char *http_proxy; - char *socks5_proxy; + char *proxy; char *ca_certs; sentry_rate_limiter_t *ratelimiter; bool debug; @@ -55,8 +54,7 @@ sentry__curl_bgworker_state_free(void *_state) sentry__rate_limiter_free(state->ratelimiter); sentry_free(state->ca_certs); sentry_free(state->user_agent); - sentry_free(state->http_proxy); - sentry_free(state->socks5_proxy); + sentry_free(state->proxy); sentry_free(state); } @@ -103,8 +101,7 @@ sentry__curl_transport_start( curl_bgworker_state_t *state = sentry__bgworker_get_state(bgworker); state->dsn = sentry__dsn_incref(options->dsn); - state->http_proxy = sentry__string_clone(options->http_proxy); - state->socks5_proxy = sentry__string_clone(options->socks5_proxy); + state->proxy = sentry__string_clone(options->proxy); state->user_agent = sentry__string_clone(options->user_agent); state->ca_certs = sentry__string_clone(options->ca_certs); state->curl_handle = curl_easy_init(); @@ -218,11 +215,8 @@ sentry__curl_send_task(void *_envelope, void *_state) curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&info); curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, header_callback); - if (state->http_proxy) { - curl_easy_setopt(curl, CURLOPT_PROXY, state->http_proxy); - } - if (state->socks5_proxy) { - curl_easy_setopt(curl, CURLOPT_PROXY, state->socks5_proxy); + if (state->proxy) { //TODO do we need to process the proxy here? + curl_easy_setopt(curl, CURLOPT_PROXY, state->proxy); } if (state->ca_certs) { curl_easy_setopt(curl, CURLOPT_CAINFO, state->ca_certs); diff --git a/tests/unit/test_uninit.c b/tests/unit/test_uninit.c index bf9577ce7..5251344f1 100644 --- a/tests/unit/test_uninit.c +++ b/tests/unit/test_uninit.c @@ -63,7 +63,7 @@ SENTRY_TEST(invalid_dsn) SENTRY_TEST(invalid_proxy) { sentry_options_t *options = sentry_options_new(); - sentry_options_set_http_proxy(options, "invalid"); + sentry_options_set_proxy(options, "invalid"); TEST_CHECK(sentry_init(options) == 0); From 50b57fbcbb4355ddc0510fec062f46ad8c9a55ce Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:49:35 +0100 Subject: [PATCH 07/37] format --- include/sentry.h | 3 +-- src/backends/sentry_backend_crashpad.cpp | 7 ++++--- src/transports/sentry_transport_curl.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index acd052338..e98d14a61 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -977,8 +977,7 @@ SENTRY_API void sentry_options_set_proxy_n( /** * Returns the configured proxy. */ -SENTRY_API const char *sentry_options_get_proxy( - const sentry_options_t *opts); +SENTRY_API const char *sentry_options_get_proxy(const sentry_options_t *opts); /** * Configures the path to a file containing ssl certificates for diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 2a98f12b4..a51fd2441 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,9 +439,10 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_TRACEF("using minidump URL \"%s\"", minidump_url); } - bool success = data->client->StartHandler(handler, database, database, //TODO update to not only use http_proxy - minidump_url ? minidump_url : "", - options->proxy ? options->proxy : "", annotations, arguments, + bool success = data->client->StartHandler(handler, database, + database, // TODO update to not only use http_proxy + minidump_url ? minidump_url : "", options->proxy ? options->proxy : "", + annotations, arguments, /* restartable */ true, /* asynchronous_start */ false, attachments); sentry_free(minidump_url); diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index f38986ec2..c51f4c0f5 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -215,7 +215,7 @@ sentry__curl_send_task(void *_envelope, void *_state) curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&info); curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, header_callback); - if (state->proxy) { //TODO do we need to process the proxy here? + if (state->proxy) { // TODO do we need to process the proxy here? curl_easy_setopt(curl, CURLOPT_PROXY, state->proxy); } if (state->ca_certs) { From f9fea04d1893a05742c4d593fcce6164f6fb421e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:53:15 +0100 Subject: [PATCH 08/37] Added back deprecated http_proxy API --- include/sentry.h | 16 ++++++++++++++++ src/sentry_options.c | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index e98d14a61..8ab416b24 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -979,6 +979,22 @@ SENTRY_API void sentry_options_set_proxy_n( */ SENTRY_API const char *sentry_options_get_proxy(const sentry_options_t *opts); +/** + * Configures the proxy. + * + * The given proxy has to include the full scheme, + * eg. `http://some.proxy/`or 'socks5://some.proxy/'. + */ +SENTRY_API void sentry_options_set_http_proxy( + sentry_options_t *opts, const char *proxy); +SENTRY_API void sentry_options_set_http_proxy_n( + sentry_options_t *opts, const char *proxy, size_t proxy_len); + +/** + * Returns the configured proxy. + */ +SENTRY_API const char *sentry_options_get_http_proxy(const sentry_options_t *opts); + /** * Configures the path to a file containing ssl certificates for * verification. diff --git a/src/sentry_options.c b/src/sentry_options.c index dc464dd1a..731039d25 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -255,6 +255,26 @@ sentry_options_get_proxy(const sentry_options_t *opts) return opts->proxy; } +void +sentry_options_set_http_proxy_n( + sentry_options_t *opts, const char *proxy, size_t proxy_len) +{ + sentry_options_set_proxy_n(opts, proxy, proxy_len); +} + +void +sentry_options_set_http_proxy(sentry_options_t *opts, const char *proxy) +{ + sentry_options_set_proxy(opts, proxy); + +} + +const char * +sentry_options_get_http_proxy(const sentry_options_t *opts) +{ + return sentry_options_get_proxy(opts); +} + void sentry_options_set_ca_certs(sentry_options_t *opts, const char *path) { From 5ce42fb9b587c48641a1412bc11fa183d8b6593a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:59:56 +0100 Subject: [PATCH 09/37] formatting --- include/sentry.h | 3 ++- src/sentry_options.c | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 8ab416b24..4c120256f 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -993,7 +993,8 @@ SENTRY_API void sentry_options_set_http_proxy_n( /** * Returns the configured proxy. */ -SENTRY_API const char *sentry_options_get_http_proxy(const sentry_options_t *opts); +SENTRY_API const char *sentry_options_get_http_proxy( + const sentry_options_t *opts); /** * Configures the path to a file containing ssl certificates for diff --git a/src/sentry_options.c b/src/sentry_options.c index 731039d25..e1d807bd1 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -266,7 +266,6 @@ void sentry_options_set_http_proxy(sentry_options_t *opts, const char *proxy) { sentry_options_set_proxy(opts, proxy); - } const char * From 11bedada0ac4ccb0e4cb755411a99820cecb9a91 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:14:39 +0100 Subject: [PATCH 10/37] refactor sentry_transport_winhttp.c to use opts->proxy --- src/transports/sentry_transport_winhttp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index ffb114eb9..8b8e9d367 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -69,9 +69,9 @@ sentry__winhttp_transport_start( sentry__bgworker_setname(bgworker, opts->transport_thread_name); // ensure the proxy starts with `http://`, otherwise ignore it - if (opts->http_proxy - && strstr(opts->http_proxy, "http://") == opts->http_proxy) { - const char *ptr = opts->http_proxy + 7; + if (opts->proxy + && strstr(opts->proxy, "http://") == opts->proxy) { + const char *ptr = opts->proxy + 7; const char *slash = strchr(ptr, '/'); if (slash) { char *copy = sentry__string_clone_n(ptr, slash - ptr); From 2e45f63cdcc3a1d67de7f96e4c46b846238a943e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:25:56 +0100 Subject: [PATCH 11/37] formatting --- src/transports/sentry_transport_winhttp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 8b8e9d367..c15eb5899 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -69,8 +69,7 @@ sentry__winhttp_transport_start( sentry__bgworker_setname(bgworker, opts->transport_thread_name); // ensure the proxy starts with `http://`, otherwise ignore it - if (opts->proxy - && strstr(opts->proxy, "http://") == opts->proxy) { + if (opts->proxy && strstr(opts->proxy, "http://") == opts->proxy) { const char *ptr = opts->proxy + 7; const char *slash = strchr(ptr, '/'); if (slash) { From ac09ba789e5c5ffa10b15aa0b2fdab1d5e084ecb Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:49:18 +0100 Subject: [PATCH 12/37] update crashpad dependency --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index b95f5c6c4..d5e828d31 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit b95f5c6c47ebd0511970df45f42cd866840454cc +Subproject commit d5e828d31c952c0e5d58808853ccd391cfcb5d3e From 99228158ca2e1b4377c5da60fa6607daf140b808 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:41:42 +0100 Subject: [PATCH 13/37] fix: update minimum libcurl version check to 7.21.7 --- src/transports/sentry_transport_curl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index c51f4c0f5..965fcccea 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -85,7 +85,7 @@ sentry__curl_transport_start( }; if (!sentry__check_min_version( - curl_version, (sentry_version_t) { 7, 10, 7 })) { + curl_version, (sentry_version_t) { 7, 21, 7 })) { SENTRY_WARNF("`libcurl` is at unsupported version `%u.%u.%u`", curl_version.major, curl_version.minor, curl_version.patch); return 1; From 498dc940dc58702fcd9c7822b670773134a83675 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:53:30 +0100 Subject: [PATCH 14/37] update crashpad dependency --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index d5e828d31..81c5c5a97 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit d5e828d31c952c0e5d58808853ccd391cfcb5d3e +Subproject commit 81c5c5a9725da5869f7c2e7fd084ceaf337f180e From f44dad641e9c1b35976448901c2d7d0c1995f9c8 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:07:33 +0100 Subject: [PATCH 15/37] test: add proxy integration tests --- tests/__init__.py | 7 ++++ tests/test_integration_crashpad.py | 52 +++++++++++++++++++++++++++++- tests/test_integration_http.py | 48 ++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 61380f90b..29436bbde 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -8,6 +8,7 @@ import pytest import pprint import textwrap +import socket sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) @@ -32,6 +33,12 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456): ) ) +def is_proxy_running(host, port): + try: + with socket.create_connection((host, port), timeout=1): + return True + except ConnectionRefusedError: + return False def run(cwd, exe, args, env=dict(os.environ), **kwargs): __tracebackhide__ = True diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 0340cfbb2..59888f1a7 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -1,11 +1,12 @@ import os import shutil +import subprocess import sys import time import pytest -from . import make_dsn, run, Envelope +from . import make_dsn, run, Envelope, is_proxy_running from .assertions import ( assert_crashpad_upload, assert_session, @@ -37,6 +38,55 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 +@pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) +@pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) +def test_crashpad_crash_socks5_proxy(cmake, httpserver, run_args, proxy_status): + 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"]: + # start mitmdump from terminal + if run_args == ["http-proxy"]: + proxy_process = subprocess.Popen(["mitmdump"]) + time.sleep(1) # 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(1) # Give mitmdump some time to start + if not is_proxy_running('localhost', 1080): + pytest.fail("mitmdump (SOCKS5) did not start correctly") + + 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") + + try: + with httpserver.wait(timeout=10) as waiting: + child = run(tmp_path, "sentry_example", ["log", "crash"] + run_args, env=env) + assert child.returncode # well, it's a crash after all + except AssertionError: + # only want to end up here on (http-proxy, off) tuple (we expect the request to fail) + if run_args == ["http-proxy"] and proxy_status == ["off"]: + 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 + # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above + assert len(httpserver.log) == 1 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() + def test_crashpad_reinstall(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 1ef549f16..abb80d36f 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -4,10 +4,11 @@ import shutil import time import uuid +import subprocess import pytest -from . import make_dsn, run, Envelope +from . import make_dsn, run, Envelope, is_proxy_running from .assertions import ( assert_attachment, assert_meta, @@ -606,3 +607,48 @@ def test_capture_minidump(cmake, httpserver): assert_attachment(envelope) assert_minidump(envelope) + +@pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) +@pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) +def test_capture_proxy(cmake, httpserver, run_args, proxy_status): + 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"]: + # start mitmdump from terminal + if run_args == ["http-proxy"]: + proxy_process = subprocess.Popen(["mitmdump"]) + time.sleep(1) # 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(1) # Give mitmdump some time to start + if not is_proxy_running('localhost', 1080): + pytest.fail("mitmdump (SOCKS5) did not start correctly") + + 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", "start-session", "capture-event"] + run_args, # 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 + elif proxy_status == ["off"]: + assert len(httpserver.log) == 0 + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() From df77a143c17ab4f43482b6b3b326a3ecf862751a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:07:51 +0100 Subject: [PATCH 16/37] get latest crashpad changes --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 81c5c5a97..3224a1cf8 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 81c5c5a9725da5869f7c2e7fd084ceaf337f180e +Subproject commit 3224a1cf80f66c95b11f446bd7a4dae3cddc6be5 From eb0a2b88937ae6d4aabaaf0b57f13f5ceba1279f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:08:40 +0100 Subject: [PATCH 17/37] feat: add example.c run-args for http and socks5 proxy --- examples/example.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/examples/example.c b/examples/example.c index 750dffec1..4065cd40b 100644 --- a/examples/example.c +++ b/examples/example.c @@ -236,6 +236,14 @@ main(int argc, char **argv) sentry_options_set_sdk_name(options, "sentry.native.android.flutter"); } + if (has_arg(argc, argv, "http-proxy")) { + sentry_options_set_proxy(options, "http://127.0.0.1:8080"); + } + + if (has_arg(argc, argv, "socks5-proxy")) { + sentry_options_set_proxy(options, "socks5://127.0.0.1:1080"); + } + sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { From 56df66e0876600401e0b037a4bad7fb0bcc42495 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:25:12 +0100 Subject: [PATCH 18/37] test: changed test name + added more mitmdump startup time --- tests/test_integration_crashpad.py | 7 ++++--- tests/test_integration_http.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 59888f1a7..896e7a89f 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -40,7 +40,7 @@ def test_crashpad_capture(cmake, httpserver): @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) -def test_crashpad_crash_socks5_proxy(cmake, httpserver, run_args, proxy_status): +def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") @@ -51,12 +51,12 @@ def test_crashpad_crash_socks5_proxy(cmake, httpserver, run_args, proxy_status): # start mitmdump from terminal if run_args == ["http-proxy"]: proxy_process = subprocess.Popen(["mitmdump"]) - time.sleep(1) # 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"]: proxy_process = subprocess.Popen(["mitmdump", "--mode", "socks5"]) - time.sleep(1) # 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") @@ -75,6 +75,7 @@ def test_crashpad_crash_socks5_proxy(cmake, httpserver, run_args, proxy_status): except AssertionError: # only want to end up here on (http-proxy, off) tuple (we expect the request to fail) if run_args == ["http-proxy"] and proxy_status == ["off"]: + assert len(httpserver.log) == 0 return assert waiting.result diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index abb80d36f..0dd13c8e6 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -621,12 +621,12 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): # start mitmdump from terminal if run_args == ["http-proxy"]: proxy_process = subprocess.Popen(["mitmdump"]) - time.sleep(1) # 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"]: proxy_process = subprocess.Popen(["mitmdump", "--mode", "socks5"]) - time.sleep(1) # 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") From 0ab5c684c3c8b6f2b172d762002d15833c299c6e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:25:45 +0100 Subject: [PATCH 19/37] chore: format --- tests/__init__.py | 2 ++ tests/test_integration_crashpad.py | 15 ++++++++++----- tests/test_integration_http.py | 12 +++++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 29436bbde..98e6218d1 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -33,6 +33,7 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456): ) ) + def is_proxy_running(host, port): try: with socket.create_connection((host, port), timeout=1): @@ -40,6 +41,7 @@ def is_proxy_running(host, port): except ConnectionRefusedError: return False + 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 896e7a89f..dd347a717 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -38,13 +38,14 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 + @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") - proxy_process = None # store the proxy process to terminate it later + proxy_process = None # store the proxy process to terminate it later try: if proxy_status == ["on"]: @@ -52,12 +53,12 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): 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): + 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): + if not is_proxy_running("localhost", 1080): pytest.fail("mitmdump (SOCKS5) did not start correctly") tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) @@ -66,11 +67,15 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): 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") + httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data( + "OK" + ) try: with httpserver.wait(timeout=10) as waiting: - child = run(tmp_path, "sentry_example", ["log", "crash"] + run_args, env=env) + child = run( + tmp_path, "sentry_example", ["log", "crash"] + run_args, env=env + ) assert child.returncode # well, it's a crash after all except AssertionError: # only want to end up here on (http-proxy, off) tuple (we expect the request to fail) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 0dd13c8e6..a35704bd6 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -608,13 +608,14 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) + @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): pytest.skip("mitmdump is not installed") - proxy_process = None # store the proxy process to terminate it later + proxy_process = None # store the proxy process to terminate it later try: if proxy_status == ["on"]: @@ -622,12 +623,12 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): 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): + 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): + if not is_proxy_running("localhost", 1080): pytest.fail("mitmdump (SOCKS5) did not start correctly") tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) @@ -640,10 +641,11 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): run( tmp_path, "sentry_example", - ["log", "start-session", "capture-event"] + run_args, # only passes if given proxy is running + ["log", "start-session", "capture-event"] + + run_args, # 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 elif proxy_status == ["off"]: From 20e611c34b7ad1c62b5e94a12b52dffd43d8ed2e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:16:42 +0100 Subject: [PATCH 20/37] test: add mitmproxy to requirements.txt --- tests/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/requirements.txt b/tests/requirements.txt index 306be91e8..50002c903 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -5,3 +5,4 @@ msgpack==1.0.8 pytest-xdist==3.5.0 clang-format==19.1.3 pywin32==308; sys_platform == "win32" +mitmproxy==11.0.0 From 1d1f3d1dd062efd73cc1c6ef2576a38b7c54d368 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:40:11 +0100 Subject: [PATCH 21/37] test: add skip for proxy test on non-MacOS platforms --- tests/test_integration_crashpad.py | 3 +++ tests/test_integration_http.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index dd347a717..735d018d1 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -39,6 +39,9 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 +@pytest.mark.skipif( + sys.platform != "darwin", reason="currently proxy tests are only supported on macOS" +) @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index a35704bd6..ae07416ed 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -2,6 +2,7 @@ import json import os import shutil +import sys import time import uuid import subprocess @@ -609,6 +610,9 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) +@pytest.mark.skipif( + sys.platform != "darwin", reason="currently proxy tests are only supported on macOS" +) @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_capture_proxy(cmake, httpserver, run_args, proxy_status): From 43805fd35acccbe7dcffb1f4fc66c40705c69964 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:42:41 +0100 Subject: [PATCH 22/37] test: move socksproxy-off assert into AssertionError check for Linux test --- tests/test_integration_crashpad.py | 13 +++++++++---- tests/test_integration_http.py | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 735d018d1..40bdbbd2f 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -40,7 +40,8 @@ def test_crashpad_capture(cmake, httpserver): @pytest.mark.skipif( - sys.platform != "darwin", reason="currently proxy tests are only supported on macOS" + sys.platform != "darwin" and sys.platform != "linux", + reason="currently proxy tests are only supported on macOS and Linux", ) @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) @@ -85,11 +86,15 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): if run_args == ["http-proxy"] and proxy_status == ["off"]: assert len(httpserver.log) == 0 return + elif run_args == ["socks5-proxy"] and proxy_status == ["off"]: + # Apple's NSURLSession will send the request even if the socks proxy fails + # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 + # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above + assert len(httpserver.log) == 1 + 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 - # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above + assert len(httpserver.log) == 1 finally: if proxy_process: diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index ae07416ed..b318916b8 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -611,7 +611,8 @@ def test_capture_minidump(cmake, httpserver): @pytest.mark.skipif( - sys.platform != "darwin", reason="currently proxy tests are only supported on macOS" + sys.platform != "darwin" and sys.platform != "linux", + reason="currently proxy tests are only supported on macOS and Linux", ) @pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) From e6bc54c870626b79836586a6270859218de7466e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:00:32 +0100 Subject: [PATCH 23/37] test: change expected http log length to match platform --- tests/test_integration_crashpad.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 40bdbbd2f..c822afb57 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -90,7 +90,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): # Apple's NSURLSession will send the request even if the socks proxy fails # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above - assert len(httpserver.log) == 1 + assert len(httpserver.log) == 1 if (sys.platform == "darwin") else 0 return assert waiting.result From ae3ca77e9d9e96c4fc936c9e47a92b974936b14d Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:15:40 +0100 Subject: [PATCH 24/37] test: order of operations --- tests/test_integration_crashpad.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index c822afb57..8f0d91992 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -90,7 +90,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): # Apple's NSURLSession will send the request even if the socks proxy fails # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above - assert len(httpserver.log) == 1 if (sys.platform == "darwin") else 0 + assert len(httpserver.log) == (1 if (sys.platform == "darwin") else 0) return assert waiting.result From 63f2f580716e876424ee8c63f0378f1355c1bef0 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:30:13 +0100 Subject: [PATCH 25/37] test: run http proxy tests on all platforms --- tests/test_integration_crashpad.py | 16 ++++++++++++---- tests/test_integration_http.py | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 8f0d91992..74b9cbf62 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -39,11 +39,19 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 -@pytest.mark.skipif( - sys.platform != "darwin" and sys.platform != "linux", - reason="currently proxy tests are only supported on macOS and Linux", +@pytest.mark.parametrize( + "run_args", + [ + pytest.param(["http-proxy"]), # HTTP proxy test runs on all platforms + pytest.param( + ["socks5-proxy"], + marks=pytest.mark.skipif( + sys.platform not in ["darwin", "linux"], + reason="SOCKS5 proxy tests are only supported on macOS and Linux", + ), + ), + ], ) -@pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index b318916b8..880093e79 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -610,11 +610,19 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) -@pytest.mark.skipif( - sys.platform != "darwin" and sys.platform != "linux", - reason="currently proxy tests are only supported on macOS and Linux", +@pytest.mark.parametrize( + "run_args", + [ + pytest.param(["http-proxy"]), # HTTP proxy test runs on all platforms + pytest.param( + ["socks5-proxy"], + marks=pytest.mark.skipif( + sys.platform not in ["darwin", "linux"], + reason="SOCKS5 proxy tests are only supported on macOS and Linux", + ), + ), + ], ) -@pytest.mark.parametrize("run_args", [(["http-proxy"]), (["socks5-proxy"])]) @pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])]) def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if not shutil.which("mitmdump"): From 1e0f1c8dc3d121aa6ec7a9d7ee2a0f047935d073 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:30:42 +0100 Subject: [PATCH 26/37] test: add special case for Windows Autoproxy --- tests/test_integration_crashpad.py | 1 - tests/test_integration_http.py | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 74b9cbf62..45b0efcdd 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -97,7 +97,6 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): elif run_args == ["socks5-proxy"] and proxy_status == ["off"]: # Apple's NSURLSession will send the request even if the socks proxy fails # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 - # hence there is no (socks5-proxy, off) tuple in the AssertionError catch above assert len(httpserver.log) == (1 if (sys.platform == "darwin") else 0) return diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 880093e79..639c11bad 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -662,7 +662,9 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if proxy_status == ["on"]: assert len(httpserver.log) == 2 elif proxy_status == ["off"]: - assert len(httpserver.log) == 0 + # Window's Autoproxy will send the request even if the http proxy fails + # https://learn.microsoft.com/en-us/windows/win32/winhttp/winhttp-autoproxy-support + assert len(httpserver.log) == (2 if (sys.platform == "win32") else 0) finally: if proxy_process: proxy_process.terminate() From 7233e4442c5379b5e186b29792f60932052a7417 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:47:34 +0100 Subject: [PATCH 27/37] apply suggestions from code review --- CHANGELOG.md | 2 +- CONTRIBUTING.md | 2 ++ include/sentry.h | 4 +++- src/backends/sentry_backend_crashpad.cpp | 3 +-- src/transports/sentry_transport_curl.c | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98bee966a..ae4f443e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ **Features**: - Provide version information for non-static Windows binaries. ([#1076](https://github.com/getsentry/sentry-native/pull/1076), [crashpad#110](https://github.com/getsentry/crashpad/pull/110)) -- Add SOCKS5 proxy support. ([#1063](https://github.com/getsentry/sentry-native/pull/1063)) +- Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063)) **Fixes**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2bdf6e097..7e718aea9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,6 +146,8 @@ The example currently supports the following commands: - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event. - `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. +- `socks-proxy`: Uses a localhost SOCKS proxy on port 1080. Only on Windows using crashpad with its WER handler module: diff --git a/include/sentry.h b/include/sentry.h index 1a2e12b81..c02bd53fc 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -982,8 +982,10 @@ SENTRY_API const char *sentry_options_get_proxy(const sentry_options_t *opts); /** * Configures the proxy. * + * This is a **deprecated** alias for `sentry_options_set_proxy(_n)`. + * * The given proxy has to include the full scheme, - * eg. `http://some.proxy/`or 'socks5://some.proxy/'. + * eg. `http://some.proxy/. */ SENTRY_API void sentry_options_set_http_proxy( sentry_options_t *opts, const char *proxy); diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 66b80494b..e906fb4d3 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,8 +439,7 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_TRACEF("using minidump URL \"%s\"", minidump_url); } - bool success = data->client->StartHandler(handler, database, - database, // TODO update to not only use http_proxy + bool success = data->client->StartHandler(handler, database, database, minidump_url ? minidump_url : "", options->proxy ? options->proxy : "", annotations, arguments, /* restartable */ true, diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index 965fcccea..1912c4bbd 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -215,7 +215,7 @@ sentry__curl_send_task(void *_envelope, void *_state) curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&info); curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, header_callback); - if (state->proxy) { // TODO do we need to process the proxy here? + if (state->proxy) { curl_easy_setopt(curl, CURLOPT_PROXY, state->proxy); } if (state->ca_certs) { From 22982f83fb56c62de1043fccdc16d52ece2f3d4d Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:32:13 +0100 Subject: [PATCH 28/37] chore: added comments about proxy behaviour --- tests/test_integration_crashpad.py | 2 ++ tests/test_integration_http.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 45b0efcdd..c310972cd 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -102,6 +102,8 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): assert waiting.result + # there is a fallback to direct connection if the proxy fails, so we always expect a request to come through + # regardless of whether proxy_status is on or off assert len(httpserver.log) == 1 finally: if proxy_process: diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 67aa160ca..3e39b53d0 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -663,8 +663,8 @@ def test_capture_proxy(cmake, httpserver, run_args, proxy_status): if proxy_status == ["on"]: assert len(httpserver.log) == 2 elif proxy_status == ["off"]: - # Window's Autoproxy will send the request even if the http proxy fails - # https://learn.microsoft.com/en-us/windows/win32/winhttp/winhttp-autoproxy-support + # 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) finally: if proxy_process: From 8f024446c1cf1e9c2f8a29b357abd4f54ab670e5 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:11:26 +0100 Subject: [PATCH 29/37] cleaned up test and moved comments to proper place (let's hope I didn't mess up CI) --- tests/test_integration_crashpad.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index c310972cd..65cb77f43 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -90,20 +90,20 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): ) assert child.returncode # well, it's a crash after all except AssertionError: - # only want to end up here on (http-proxy, off) tuple (we expect the request to fail) + # we fail on macOS/Linux if the http proxy is not running if run_args == ["http-proxy"] and proxy_status == ["off"]: 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"]: - # Apple's NSURLSession will send the request even if the socks proxy fails - # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022 - assert len(httpserver.log) == (1 if (sys.platform == "darwin") else 0) + assert len(httpserver.log) == 0 return assert waiting.result - # there is a fallback to direct connection if the proxy fails, so we always expect a request to come through - # regardless of whether proxy_status is on or off + # 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 provides fallback for both http and socks5 proxies assert len(httpserver.log) == 1 finally: if proxy_process: From a8877f7a9777b784c72ded4aad7bafd7fc4d1baa Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:44:29 +0100 Subject: [PATCH 30/37] chore: fix minor comment typo --- tests/test_integration_crashpad.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 65cb77f43..a0ffad812 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -103,7 +103,7 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status): # 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 provides fallback for both http and socks5 proxies + # Windows also provides fallback for http proxies assert len(httpserver.log) == 1 finally: if proxy_process: From 20c37edad4d9cea522e193c06be01b73296e8181 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:41:44 +0100 Subject: [PATCH 31/37] chore: added api documentation --- include/sentry.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index e07c4bc4e..460738f47 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -976,6 +976,13 @@ SENTRY_API const char *sentry_options_get_dist(const sentry_options_t *opts); * * The given proxy has to include the full scheme, * eg. `http://some.proxy/`or 'socks5://some.proxy/'. + * + * Not every transport behaves the same way when configuring a proxy. + * On Windows if a transport can't connect to the proxy it will fall back on a + * connection without proxy. This is also true for the crashpad_handler + * transport on macOS for a socks proxy, but not for a http proxy. + * All transports that use libcurl (Linux and the Native SDK transport on macOS) + * will honor the proxy settings and not fall back. */ SENTRY_API void sentry_options_set_proxy( sentry_options_t *opts, const char *proxy); From f5e6259b648ece145cfb7a3efa4e28bee40d3609 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:03:13 +0100 Subject: [PATCH 32/37] Update include/sentry.h Co-authored-by: Mischan Toosarani-Hausberger --- include/sentry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sentry.h b/include/sentry.h index 460738f47..d025f41c6 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -975,7 +975,7 @@ SENTRY_API const char *sentry_options_get_dist(const sentry_options_t *opts); * Configures the proxy. * * The given proxy has to include the full scheme, - * eg. `http://some.proxy/`or 'socks5://some.proxy/'. + * eg. `http://some.proxy/` or `socks5://some.proxy/`. * * Not every transport behaves the same way when configuring a proxy. * On Windows if a transport can't connect to the proxy it will fall back on a From a7c1af3519c7744c1ef8fa286151ad222cab86c1 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:03:29 +0100 Subject: [PATCH 33/37] Update CONTRIBUTING.md Co-authored-by: Mischan Toosarani-Hausberger --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7e718aea9..041d99d24 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,8 +146,8 @@ The example currently supports the following commands: - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event. - `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. -- `socks-proxy`: Uses a localhost SOCKS proxy on port 1080. +- `http-proxy`: Uses a localhost `HTTP` proxy on port 8080. +- `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080. Only on Windows using crashpad with its WER handler module: From 7f50f6e45e375ac2ab464e251e1f8f86af402c39 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:17:13 +0100 Subject: [PATCH 34/37] update crashpad submodule --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 3224a1cf8..b95f5c6c4 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 3224a1cf80f66c95b11f446bd7a4dae3cddc6be5 +Subproject commit b95f5c6c47ebd0511970df45f42cd866840454cc From 881c017885af97ca791c83b25dd7c1a0168ff188 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:27:20 +0100 Subject: [PATCH 35/37] update crashpad submodule (again) --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index b95f5c6c4..852f8fea9 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit b95f5c6c47ebd0511970df45f42cd866840454cc +Subproject commit 852f8fea9cbea7a63610b8c698f301bd1ba4b3cd From 140be40fe51bfaf693efec2c72d539afedfa9e0f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:39:09 +0100 Subject: [PATCH 36/37] updated CHANGELOG.md --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5547541a..012c759a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +**Unreleased**: + +**Features**: +- Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063)) + ## 0.7.15 **Fixes**: @@ -23,7 +28,6 @@ - Provide version information for non-static Windows binaries. ([#1076](https://github.com/getsentry/sentry-native/pull/1076), [crashpad#110](https://github.com/getsentry/crashpad/pull/110)) - Add an alternative handler strategy to `inproc` to support `.NET` on Linux and `Mono` on Android (specifically, [.NET MAUI](https://github.com/dotnet/android/issues/9055#issuecomment-2261347912)). ([#1027](https://github.com/getsentry/sentry-native/pull/1027)) -- Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063)) **Fixes**: From 37d075c9ab422820bc1c4cb1a541f87fac509375 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:39:33 +0100 Subject: [PATCH 37/37] updated CHANGELOG.md again --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 012c759a8..20afa9699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -**Unreleased**: +## Unreleased **Features**: - Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063))