diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3fb0e6079..376a37a17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -193,6 +193,25 @@ jobs: run: bash scripts/start-android.sh timeout-minutes: 20 + - name: Add sentry.native.test hostname + if: ${{ runner.os == 'Windows' }} + # The path is usually C:\Windows\System32\drivers\etc\hosts + run: | + Add-Content -Path $env:SystemRoot\System32\drivers\etc\hosts -Value "127.0.0.1 sentry.native.test" + shell: powershell + + - name: Print hosts file + if: ${{ runner.os == 'Windows' }} + run: type $env:SystemRoot\System32\drivers\etc\hosts + shell: powershell + + - name: Add sentry.native.test hostname + if: ${{ runner.os == 'macOS' || runner.os == 'Linux' }} + run: | + echo "127.0.0.1 sentry.native.test" | sudo tee -a /etc/hosts + cat /etc/hosts + shell: bash + - name: Test shell: bash run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index f35c5a0f1..93150f913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Features**: +- Ensure support for `http_proxy` and `https_proxy` environment variables across all transports. ([#1111](https://github.com/getsentry/sentry-native/pull/1111)) - Auto-detect the latest GDK and Windows SDK for the XBox build. ([#1124](https://github.com/getsentry/sentry-native/pull/1124)) - Enable debug-option by default when running in a debug-build. ([#1128](https://github.com/getsentry/sentry-native/pull/1128)) @@ -23,7 +24,7 @@ - Add support for Xbox Series X/S. ([#1100](https://github.com/getsentry/sentry-native/pull/1100)) - Add option to set debug log level. ([#1107](https://github.com/getsentry/sentry-native/pull/1107)) -- Add `traces_sampler` ([#1108](https://github.com/getsentry/sentry-native/pull/1108)) +- Add `traces_sampler`. ([#1108](https://github.com/getsentry/sentry-native/pull/1108)) - Provide support for C++17 compilers when using the `crashpad` backend. ([#1110](https://github.com/getsentry/sentry-native/pull/1110), [crashpad#116](https://github.com/getsentry/crashpad/pull/116), [mini_chromium#1](https://github.com/getsentry/mini_chromium/pull/1)) ## 0.7.17 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01585ad9f..c9bf5ac8b 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/ @@ -147,6 +150,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. +- `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`. diff --git a/examples/example.c b/examples/example.c index 6e48ed8a5..6ac07fde6 100644 --- a/examples/example.c +++ b/examples/example.c @@ -271,6 +271,16 @@ main(int argc, char **argv) if (has_arg(argc, argv, "http-proxy")) { sentry_options_set_proxy(options, "http://127.0.0.1:8080"); } + if (has_arg(argc, argv, "http-proxy-auth")) { + sentry_options_set_proxy( + options, "http://user:password@127.0.0.1:8080"); + } + if (has_arg(argc, argv, "http-proxy-ipv6")) { + sentry_options_set_proxy(options, "http://[::1]:8080"); + } + if (has_arg(argc, argv, "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/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 08f60d290..85de1bf71 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -439,9 +439,20 @@ crashpad_backend_startup( if (minidump_url) { SENTRY_DEBUGF("using minidump URL \"%s\"", minidump_url); } + 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 + : ""; +#ifdef SENTRY_PLATFORM_LINUX + // explicitly set an empty proxy to avoid reading from env. vars. on Linux + if (options->proxy && strcmp(options->proxy, "") == 0) { + proxy_url = ""; + } +#endif 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/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 b72751160..fcb5cf878 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; @@ -51,10 +53,35 @@ sentry__winhttp_bgworker_state_free(void *_state) sentry__dsn_decref(state->dsn); sentry__rate_limiter_free(state->ratelimiter); sentry_free(state->user_agent); + sentry_free(state->proxy_username); + sentry_free(state->proxy_password); sentry_free(state->proxy); sentry_free(state); } +// Function to extract and set credentials +static void +set_proxy_credentials(winhttp_bgworker_state_t *state, const char *proxy) +{ + sentry_url_t url; + sentry__url_parse(&url, proxy, false); + if (url.username && url.password) { + // Convert user and pass to LPCWSTR + int user_wlen + = MultiByteToWideChar(CP_UTF8, 0, url.username, -1, NULL, 0); + int pass_wlen + = MultiByteToWideChar(CP_UTF8, 0, url.password, -1, NULL, 0); + wchar_t *user_w = (wchar_t *)malloc(user_wlen * sizeof(wchar_t)); + wchar_t *pass_w = (wchar_t *)malloc(pass_wlen * sizeof(wchar_t)); + MultiByteToWideChar(CP_UTF8, 0, url.username, -1, user_w, user_wlen); + MultiByteToWideChar(CP_UTF8, 0, url.password, -1, pass_w, pass_wlen); + + state->proxy_username = user_w; + state->proxy_password = pass_w; + } + sentry__url_cleanup(&url); +} + static int sentry__winhttp_transport_start( const sentry_options_t *opts, void *transport_state) @@ -66,12 +93,20 @@ 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); + 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 - if (opts->proxy && strstr(opts->proxy, "http://") == opts->proxy) { - const char *ptr = opts->proxy + 7; + if (proxy && strstr(proxy, "http://") == proxy) { + const char *ptr = proxy + 7; + const char *at_sign = strchr(ptr, '@'); const char *slash = strchr(ptr, '/'); + if (at_sign && (!slash || at_sign < slash)) { + ptr = at_sign + 1; + set_proxy_credentials(state, proxy); + } if (slash) { char *copy = sentry__string_clone_n(ptr, slash - ptr); state->proxy = sentry__string_to_wstr(copy); @@ -103,6 +138,7 @@ sentry__winhttp_transport_start( SENTRY_WARN("`WinHttpOpen` failed"); return 1; } + return sentry__bgworker_start(bgworker); } @@ -209,6 +245,12 @@ sentry__winhttp_send_task(void *_envelope, void *_state) SENTRY_DEBUGF( "sending request using winhttp to \"%s\":\n%S", req->url, headers); + if (state->proxy_username && state->proxy_password) { + WinHttpSetCredentials(state->request, WINHTTP_AUTH_TARGET_PROXY, + WINHTTP_AUTH_SCHEME_BASIC, state->proxy_username, + state->proxy_password, 0); + } + if (WinHttpSendRequest(state->request, headers, (DWORD)-1, (LPVOID)req->body, (DWORD)req->body_len, (DWORD)req->body_len, 0)) { WinHttpReceiveResponse(state->request, NULL); diff --git a/tests/__init__.py b/tests/__init__.py index 98e6218d1..a3e2aea4e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -12,17 +12,24 @@ sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) - # https://docs.pytest.org/en/latest/assert.html#assert-details pytest.register_assert_rewrite("tests.assertions") +from tests.assertions import assert_no_proxy_request -def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456): +def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False): url = urllib.parse.urlsplit(httpserver.url_for("/{}".format(id))) # We explicitly use `127.0.0.1` here, because on Windows, `localhost` will # first try `::1` (the ipv6 loopback), retry a couple times and give up # after a timeout of 2 seconds, falling back to the ipv4 loopback instead. host = url.netloc.replace("localhost", "127.0.0.1") + if proxy_host: + # To avoid bypassing the proxy for requests to localhost, we need to add this mapping + # to the hosts file & make the DSN using this alternate hostname + # see https://learn.microsoft.com/en-us/windows/win32/wininet/enabling-internet-functionality#listing-the-proxy-bypass + host = host.replace("127.0.0.1", "sentry.native.test") + _check_sentry_native_resolves_to_localhost() + return urllib.parse.urlunsplit( ( url.scheme, @@ -34,12 +41,12 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456): ) -def is_proxy_running(host, port): +def _check_sentry_native_resolves_to_localhost(): try: - with socket.create_connection((host, port), timeout=1): - return True - except ConnectionRefusedError: - return False + resolved_ip = socket.gethostbyname("sentry.native.test") + assert resolved_ip == "127.0.0.1" + except socket.gaierror: + pytest.skip("sentry.native.test does not resolve to localhost") def run(cwd, exe, args, env=dict(os.environ), **kwargs): 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/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 diff --git a/tests/proxy.py b/tests/proxy.py new file mode 100644 index 000000000..265027abe --- /dev/null +++ b/tests/proxy.py @@ -0,0 +1,77 @@ +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_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_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_httpserver_logsize diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index a0ffad812..4795c3aa2 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, is_proxy_running +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,6 +49,105 @@ def test_crashpad_capture(cmake, httpserver): assert len(httpserver.log) == 2 +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 + setup_proxy_env_vars(port=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: + cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process) + + +def test_crashpad_crash_proxy_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_crashpad_proxy_test( + cmake, httpserver, "http-proxy" + ) + + 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 + finally: + cleanup_proxy_env_vars() + proxy_test_finally(0, httpserver, proxy_process) + + +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: + cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + + +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: + del os.environ["https_proxy"] + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + + @pytest.mark.parametrize( "run_args", [ @@ -52,35 +161,18 @@ def test_crashpad_capture(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 + expected_logsize = 0 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") - - 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" + 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: @@ -90,25 +182,14 @@ 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"]: - 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"]: - assert len(httpserver.log) == 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 - # 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 a89a102f6..de9403f57 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, is_proxy_running +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, @@ -24,8 +34,9 @@ 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 +from .conditions import has_http, has_breakpad, has_files, has_crashpad pytestmark = pytest.mark.skipif(not has_http, reason="tests need http") @@ -611,6 +622,191 @@ def test_capture_minidump(cmake, httpserver): assert_minidump(envelope) +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 + setup_proxy_env_vars(port=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: + cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process) + + +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=env, + ) + + finally: + cleanup_proxy_env_vars() + proxy_test_finally(0, httpserver, proxy_process) + + +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 + try: + env, proxy_process, tmp_path = _setup_http_proxy_test( + cmake, httpserver, "http-proxy", proxy_auth="user:password" + ) + + 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( + 1, + httpserver, + proxy_process, + assert_failed_proxy_auth_request, + ) + + +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 + try: + 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, + ) + + +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: + 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=env, + ) + + finally: + 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: + cleanup_proxy_env_vars() + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + + +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: + del os.environ["https_proxy"] + proxy_test_finally(1, httpserver, proxy_process, expected_proxy_logsize=0) + + @pytest.mark.parametrize( "run_args", [ @@ -624,49 +820,30 @@ 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_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 + expected_logsize = 0 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") - - 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") - + 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", "start-session", "capture-event"] + ["log", "capture-event"] + run_args, # 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_status == ["on"]: - assert len(httpserver.log) == 2 - 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) + if proxy_running: + expected_logsize = 1 + else: + expected_logsize = 0 finally: - if proxy_process: - proxy_process.terminate() - proxy_process.wait() + proxy_test_finally(expected_logsize, httpserver, proxy_process) 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)