diff --git a/CHANGELOG.md b/CHANGELOG.md index c5cd5267b2..210cf4c4c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +**Fixes**: + +- Report on partial disk writes when streaming envelopes to file, which previously left truncated envelopes on disk and reported success. ([#1804](https://github.com/getsentry/sentry-native/pull/1804)) + ## 0.15.0 **Breaking**: diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index 6a15604541..d16df6141c 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -930,9 +930,13 @@ envelope_write_to_path(const sentry_envelope_t *envelope, return rv != 0; } + int failed = 1; sentry_jsonwriter_t *jw = sentry__jsonwriter_new_fw(fw); if (jw) { sentry__jsonwriter_write_value(jw, envelope->contents.items.headers); + if (sentry__jsonwriter_has_failed(jw)) { + goto done; + } sentry__jsonwriter_reset(jw); for (const sentry_envelope_item_t *item @@ -942,22 +946,36 @@ envelope_write_to_path(const sentry_envelope_t *envelope, continue; } const char newline = '\n'; - sentry__filewriter_write(fw, &newline, sizeof(char)); + if (sentry__filewriter_write(fw, &newline, sizeof(char)) != 0) { + goto done; + } sentry__jsonwriter_write_value(jw, item->headers); + if (sentry__jsonwriter_has_failed(jw)) { + goto done; + } sentry__jsonwriter_reset(jw); - sentry__filewriter_write(fw, &newline, sizeof(char)); + if (sentry__filewriter_write(fw, &newline, sizeof(char)) != 0) { + goto done; + } - sentry__filewriter_write(fw, item->payload, item->payload_len); + if (sentry__filewriter_write(fw, item->payload, item->payload_len) + != 0) { + goto done; + } } - sentry__jsonwriter_free(jw); + failed = sentry__filewriter_byte_count(fw) == 0; } - size_t rv = sentry__filewriter_byte_count(fw); +done: + if (failed) { + SENTRY_WARN("envelope write failed"); + } + sentry__jsonwriter_free(jw); sentry__filewriter_free(fw); - return rv == 0; + return failed; } MUST_USE int diff --git a/src/sentry_json.c b/src/sentry_json.c index b68d1a9ebd..079be8ad6a 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -196,7 +196,9 @@ sentry__jsonwriter_new_fw(sentry_filewriter_t *fw) void sentry__jsonwriter_free(sentry_jsonwriter_t *jw) { - jw->ops->free(jw); + if (jw) { + jw->ops->free(jw); + } } void @@ -208,6 +210,12 @@ sentry__jsonwriter_reset(sentry_jsonwriter_t *jw) jw->failed = false; } +bool +sentry__jsonwriter_has_failed(const sentry_jsonwriter_t *jw) +{ + return jw && jw->failed; +} + char * sentry__jsonwriter_into_string(sentry_jsonwriter_t *jw, size_t *len_out) { diff --git a/src/sentry_json.h b/src/sentry_json.h index 20f3025188..5aca6ed2e1 100644 --- a/src/sentry_json.h +++ b/src/sentry_json.h @@ -32,6 +32,11 @@ void sentry__jsonwriter_free(sentry_jsonwriter_t *jw); */ void sentry__jsonwriter_reset(sentry_jsonwriter_t *jw); +/** + * Returns true if any write operation on this JSON writer has failed. + */ +bool sentry__jsonwriter_has_failed(const sentry_jsonwriter_t *jw); + /** * This will consume and deallocate the JSON writer, returning the generated * JSON string, and writing its length into `len_out`. diff --git a/tests/unit/test_envelopes.c b/tests/unit/test_envelopes.c index 2f19c0450a..726b01ab86 100644 --- a/tests/unit/test_envelopes.c +++ b/tests/unit/test_envelopes.c @@ -12,6 +12,13 @@ #include "sentry_value.h" #include "transports/sentry_http_transport.h" +#if defined(SENTRY_PLATFORM_UNIX) && !defined(SENTRY_PLATFORM_ANDROID) \ + && !defined(SENTRY_PLATFORM_PS) && !defined(SENTRY_PLATFORM_NX) +# include +# include +# include +#endif + static char *const SERIALIZED_ENVELOPE_STR = "{\"dsn\":\"https://foo@sentry.invalid/42\"," "\"event_id\":\"c993afb6-b4ac-48a6-b61b-2558e601d65d\",\"trace\":{" @@ -1207,3 +1214,74 @@ SENTRY_TEST(envelope_can_add_client_report) sentry__rate_limiter_free(rl); sentry_close(); } + +#if defined(SENTRY_PLATFORM_UNIX) && !defined(SENTRY_PLATFORM_ANDROID) \ + && !defined(SENTRY_PLATFORM_PS) && !defined(SENTRY_PLATFORM_NX) +static void +sigxfsz_noop(int sig) +{ + (void)sig; +} +#endif + +SENTRY_TEST(write_envelope_partial_write_fails) +{ +#if !defined(SENTRY_PLATFORM_UNIX) || defined(SENTRY_PLATFORM_ANDROID) \ + || defined(SENTRY_PLATFORM_PS) || defined(SENTRY_PLATFORM_NX) + SKIP_TEST(); +#else + sentry_options_t *options = sentry_options_new(); + TEST_CHECK(options != NULL); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_backend(options, NULL); + sentry_init(options); + + sentry_envelope_t *envelope = sentry__envelope_new(); + TEST_CHECK(envelope != NULL); + + size_t big_len = 40000; + char *big_buf = sentry_malloc(big_len); + TEST_CHECK(big_buf != NULL); + memset(big_buf, 'X', big_len); + sentry__envelope_add_from_buffer(envelope, big_buf, big_len, "attachment"); + sentry_free(big_buf); + + const char *test_file_str + = SENTRY_TEST_PATH_PREFIX "sentry_test_partial_write"; + + // fork() isolates the RLIMIT_FSIZE setting from the test suite + pid_t pid = fork(); + TEST_CHECK(pid >= 0); + + if (pid == 0) { + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = sigxfsz_noop; + sigaction(SIGXFSZ, &sa, NULL); + + struct rlimit rl; + rl.rlim_cur = 512; + rl.rlim_max = 512; + if (setrlimit(RLIMIT_FSIZE, &rl) != 0) { + _exit(2); + } + + int rv = sentry_envelope_write_to_file(envelope, test_file_str); + sentry_envelope_free(envelope); + _exit(rv != 0 ? 0 : 1); + } + + int status = 0; + waitpid(pid, &status, 0); + TEST_CHECK(WIFEXITED(status)); + int child_exit = WEXITSTATUS(status); + TEST_CHECK_INT_EQUAL(child_exit, 0); + + sentry_path_t *test_path = sentry__path_from_str(test_file_str); + sentry__path_remove(test_path); + sentry__path_free(test_path); + + sentry_envelope_free(envelope); + sentry_close(); +#endif +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index ea810758b8..99d26a198a 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -406,5 +406,6 @@ XX(value_uint64) XX(value_unicode) XX(value_user) XX(value_wrong_type) +XX(write_envelope_partial_write_fails) XX(write_raw_envelope_to_file) XX(xmm_save_area_size)