From 521167c667485d926ed2d773503e87fd6e1e071e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 19 Jun 2020 16:19:21 +0200 Subject: [PATCH 1/3] fix: Avoid unsafety in the logger This also turns on the logger for the HTTP-based tests, and enables more testing of the crashpad backend. --- src/sentry_logger.c | 2 +- tests/test_integration_crashpad.py | 64 +++++++++++++++++++++++++--- tests/test_integration_http.py | 32 ++++++++------ tests/test_integration_ratelimits.py | 8 ++-- 4 files changed, 84 insertions(+), 22 deletions(-) diff --git a/src/sentry_logger.c b/src/sentry_logger.c index 7634a4831..ab6bd05c4 100644 --- a/src/sentry_logger.c +++ b/src/sentry_logger.c @@ -53,7 +53,7 @@ sentry__logger_defaultlogger( size_t len = strlen(prefix) + strlen(priority) + strlen(message) + 2; char *format = sentry_malloc(len); - snprintf("%s%s%s\n", len, prefix, priority, message); + snprintf(format, len, "%s%s%s\n", prefix, priority, message); vfprintf(stderr, format, args); diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index c280295fc..631afd3fb 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -1,5 +1,9 @@ import pytest -from .conditions import has_crashpad +import os +import time +from . import make_dsn, run, Envelope +from .conditions import has_crashpad, has_http +from .assertions import assert_session # TODO: # * with crashpad backend: @@ -8,8 +12,58 @@ # - expect report via http -@pytest.mark.skipif(not has_crashpad, reason="test needs crashpad backend") -def test_crashpad_build(cmake): - cmake( - ["sentry_example"], {"SENTRY_BACKEND": "crashpad", "SENTRY_TRANSPORT": "none"}, +@pytest.mark.skipif( + not has_http or not has_crashpad, reason="test needs crashpad backend" +) +def test_crashpad_capture(cmake, httpserver): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + ["log", "start-session", "capture-event"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + + assert len(httpserver.log) == 2 + + +@pytest.mark.skipif( + not has_http or not has_crashpad, reason="test needs crashpad backend" +) +def test_crashpad_crash(cmake, httpserver): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + httpserver.expect_request("/api/123456/minidump/").respond_with_data("OK") + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + child = run( + tmp_path, + "sentry_example", + ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"], + ) + assert child.returncode # well, its a crash after all + + run( + tmp_path, + "sentry_example", + ["log", "no-setup"], + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) + + assert len(httpserver.log) == 2 + outputs = (httpserver.log[0][0].get_data(), httpserver.log[1][0].get_data()) + session, multipart = ( + outputs if b'"type":"session"' in outputs[0] else (outputs[1], outputs[0]) + ) + + envelope = Envelope.deserialize(session) + assert_session(envelope, {"status": "abnormal", "errors": 0}) + + # TODO: crashpad actually sends a compressed multipart request, + # which we don’t parse / assert right now. + # Ideally, we would pull in a real relay to do all the http tests against. diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index a28cdfc6e..8e96b566a 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -35,7 +35,7 @@ def test_capture_http(cmake, httpserver): run( tmp_path, "sentry_example", - ["release-env", "capture-event", "add-stacktrace"], + ["log", "release-env", "capture-event", "add-stacktrace"], check=True, env=env, ) @@ -63,14 +63,14 @@ def test_session_http(cmake, httpserver): run( tmp_path, "sentry_example", - ["release-env", "start-session"], + ["log", "release-env", "start-session"], check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) run( tmp_path, "sentry_example", - ["start-session"], + ["log", "start-session"], check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) @@ -92,7 +92,7 @@ def test_capture_and_session_http(cmake, httpserver): run( tmp_path, "sentry_example", - ["start-session", "capture-event"], + ["log", "start-session", "capture-event"], check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) @@ -113,7 +113,9 @@ def test_capture_and_session_http(cmake, httpserver): def test_inproc_crash_http(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "inproc"}) - child = run(tmp_path, "sentry_example", ["start-session", "attachment", "crash"]) + child = run( + tmp_path, "sentry_example", ["log", "start-session", "attachment", "crash"] + ) assert child.returncode # well, its a crash after all httpserver.expect_request( @@ -123,7 +125,7 @@ def test_inproc_crash_http(cmake, httpserver): run( tmp_path, "sentry_example", - ["no-setup"], + ["log", "no-setup"], check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) @@ -154,10 +156,12 @@ def test_inproc_dump_inflight(cmake, httpserver): ).respond_with_data("OK") env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) - child = run(tmp_path, "sentry_example", ["capture-multiple", "crash"], env=env) + child = run( + tmp_path, "sentry_example", ["log", "capture-multiple", "crash"], env=env + ) assert child.returncode # well, its a crash after all - run(tmp_path, "sentry_example", ["no-setup"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "no-setup"], check=True, env=env) # we trigger 10 normal events, and 1 crash assert len(httpserver.log) >= 11 @@ -167,7 +171,9 @@ def test_inproc_dump_inflight(cmake, httpserver): def test_breakpad_crash_http(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "breakpad"}) - child = run(tmp_path, "sentry_example", ["start-session", "attachment", "crash"]) + child = run( + tmp_path, "sentry_example", ["log", "start-session", "attachment", "crash"] + ) assert child.returncode # well, its a crash after all httpserver.expect_request( @@ -177,7 +183,7 @@ def test_breakpad_crash_http(cmake, httpserver): run( tmp_path, "sentry_example", - ["no-setup"], + ["log", "no-setup"], check=True, env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) @@ -209,10 +215,12 @@ def test_breakpad_dump_inflight(cmake, httpserver): ).respond_with_data("OK") env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) - child = run(tmp_path, "sentry_example", ["capture-multiple", "crash"], env=env) + child = run( + tmp_path, "sentry_example", ["log", "capture-multiple", "crash"], env=env + ) assert child.returncode # well, its a crash after all - run(tmp_path, "sentry_example", ["no-setup"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "no-setup"], check=True, env=env) # we trigger 10 normal events, and 1 crash assert len(httpserver.log) >= 11 diff --git a/tests/test_integration_ratelimits.py b/tests/test_integration_ratelimits.py index a64cb2c77..e265d3c58 100644 --- a/tests/test_integration_ratelimits.py +++ b/tests/test_integration_ratelimits.py @@ -16,13 +16,13 @@ def test_retry_after(cmake, httpserver): httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data( "OK", 200, {"retry-after": "60"} ) - run(tmp_path, "sentry_example", ["capture-multiple"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "capture-multiple"], check=True, env=env) assert len(httpserver.log) == 1 httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data( "OK", 429, {"retry-after": "60"} ) - run(tmp_path, "sentry_example", ["capture-multiple"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "capture-multiple"], check=True, env=env) assert len(httpserver.log) == 2 @@ -35,11 +35,11 @@ def test_rate_limits(cmake, httpserver): httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data( "OK", 200, headers ) - run(tmp_path, "sentry_example", ["capture-multiple"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "capture-multiple"], check=True, env=env) assert len(httpserver.log) == 1 httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data( "OK", 429, headers ) - run(tmp_path, "sentry_example", ["capture-multiple"], check=True, env=env) + run(tmp_path, "sentry_example", ["log", "capture-multiple"], check=True, env=env) assert len(httpserver.log) == 2 From e966dd100374efded31f8538080a69d549631628 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 19 Jun 2020 16:34:40 +0200 Subject: [PATCH 2/3] add a delay --- tests/test_integration_crashpad.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 631afd3fb..96441be4e 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -55,6 +55,8 @@ def test_crashpad_crash(cmake, httpserver): env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), ) + time.sleep(2) # lets wait a bit for crashpad sending in the background + assert len(httpserver.log) == 2 outputs = (httpserver.log[0][0].get_data(), httpserver.log[1][0].get_data()) session, multipart = ( From e457e149521672d177e02d19548781909b334484 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Fri, 19 Jun 2020 20:25:07 +0200 Subject: [PATCH 3/3] ref: Back out crashpad integration tests --- tests/test_integration_crashpad.py | 66 +++--------------------------- 1 file changed, 5 insertions(+), 61 deletions(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 96441be4e..c280295fc 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -1,9 +1,5 @@ import pytest -import os -import time -from . import make_dsn, run, Envelope -from .conditions import has_crashpad, has_http -from .assertions import assert_session +from .conditions import has_crashpad # TODO: # * with crashpad backend: @@ -12,60 +8,8 @@ # - expect report via http -@pytest.mark.skipif( - not has_http or not has_crashpad, reason="test needs crashpad backend" -) -def test_crashpad_capture(cmake, httpserver): - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) - - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - - run( - tmp_path, - "sentry_example", - ["log", "start-session", "capture-event"], - check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), - ) - - assert len(httpserver.log) == 2 - - -@pytest.mark.skipif( - not has_http or not has_crashpad, reason="test needs crashpad backend" -) -def test_crashpad_crash(cmake, httpserver): - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) - - httpserver.expect_request("/api/123456/minidump/").respond_with_data("OK") - httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - - child = run( - tmp_path, - "sentry_example", - ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"], +@pytest.mark.skipif(not has_crashpad, reason="test needs crashpad backend") +def test_crashpad_build(cmake): + cmake( + ["sentry_example"], {"SENTRY_BACKEND": "crashpad", "SENTRY_TRANSPORT": "none"}, ) - assert child.returncode # well, its a crash after all - - run( - tmp_path, - "sentry_example", - ["log", "no-setup"], - check=True, - env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), - ) - - time.sleep(2) # lets wait a bit for crashpad sending in the background - - assert len(httpserver.log) == 2 - outputs = (httpserver.log[0][0].get_data(), httpserver.log[1][0].get_data()) - session, multipart = ( - outputs if b'"type":"session"' in outputs[0] else (outputs[1], outputs[0]) - ) - - envelope = Envelope.deserialize(session) - assert_session(envelope, {"status": "abnormal", "errors": 0}) - - # TODO: crashpad actually sends a compressed multipart request, - # which we don’t parse / assert right now. - # Ideally, we would pull in a real relay to do all the http tests against.