Skip to content

Commit

Permalink
fix: Avoid unsafety in the logger
Browse files Browse the repository at this point in the history
This also turns on the logger for the HTTP-based tests, and enables
more testing of the crashpad backend.
  • Loading branch information
Swatinem committed Jun 19, 2020
1 parent 205e38f commit 521167c
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/sentry_logger.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
64 changes: 59 additions & 5 deletions tests/test_integration_crashpad.py
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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.
32 changes: 20 additions & 12 deletions tests/test_integration_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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)),
)
Expand All @@ -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)),
)
Expand All @@ -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(
Expand All @@ -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)),
)
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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)),
)
Expand Down Expand Up @@ -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
8 changes: 4 additions & 4 deletions tests/test_integration_ratelimits.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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

0 comments on commit 521167c

Please sign in to comment.