diff --git a/CHANGELOG.md b/CHANGELOG.md index caa497b98..ca1c444f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +**Features**: + +- Cache consent-revoked envelopes to disk when `cache_keep` is enabled, so they can be sent when consent is later given. ([#1542](https://github.com/getsentry/sentry-native/pull/1542)) + **Fixes**: - macOS: cache VM regions for FP validation in the new unwinder. ([#1634](https://github.com/getsentry/sentry-native/pull/1634)) diff --git a/examples/example.c b/examples/example.c index aadceb6ed..fdaacbdfe 100644 --- a/examples/example.c +++ b/examples/example.c @@ -694,6 +694,9 @@ main(int argc, char **argv) if (has_arg(argc, argv, "log-attributes")) { sentry_options_set_logs_with_attributes(options, true); } + if (has_arg(argc, argv, "require-user-consent")) { + sentry_options_set_require_user_consent(options, true); + } if (has_arg(argc, argv, "cache-keep")) { sentry_options_set_cache_keep(options, true); sentry_options_set_cache_max_size(options, 4 * 1024 * 1024); // 4 MB @@ -758,6 +761,10 @@ main(int argc, char **argv) return EXIT_FAILURE; } + if (has_arg(argc, argv, "user-consent-revoke")) { + sentry_user_consent_revoke(); + } + if (has_arg(argc, argv, "set-global-attribute")) { sentry_set_attribute("global.attribute.bool", sentry_value_new_attribute(sentry_value_new_bool(true), NULL)); @@ -1049,6 +1056,9 @@ main(int argc, char **argv) } sentry_capture_event(event); } + if (has_arg(argc, argv, "user-consent-give")) { + sentry_user_consent_give(); + } if (has_arg(argc, argv, "capture-exception")) { sentry_value_t exc = sentry_value_new_exception( "ParseIntError", "invalid digit found in string"); diff --git a/include/sentry.h b/include/sentry.h index 26f7adbd3..35c1e604b 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1470,6 +1470,11 @@ SENTRY_API int sentry_options_get_auto_session_tracking( * This disables uploads until the user has given the consent to the SDK. * Consent itself is given with `sentry_user_consent_give` and * `sentry_user_consent_revoke`. + * + * When combined with `cache_keep` or `http_retry`, envelopes captured + * while consent is revoked are written to the cache directory instead + * of being discarded. With `http_retry` enabled, cached envelopes are + * sent automatically once consent is given. */ SENTRY_API void sentry_options_set_require_user_consent( sentry_options_t *opts, int val); @@ -1505,6 +1510,10 @@ SENTRY_API int sentry_options_get_symbolize_stacktraces( * subdirectory within the database directory. The cache is cleared on startup * based on the cache_max_items, cache_max_size, and cache_max_age options. * + * When combined with `sentry_options_set_require_user_consent`, envelopes + * captured while consent is revoked are also written to the cache. With + * `http_retry` enabled, they are sent once consent is given. + * * Only applicable for HTTP transports. * * Disabled by default. @@ -1920,6 +1929,9 @@ SENTRY_EXPERIMENTAL_API int sentry_reinstall_backend(void); /** * Gives user consent. + * + * Schedules a retry of any envelopes cached while consent was revoked, + * provided that `http_retry` is enabled. */ SENTRY_API void sentry_user_consent_give(void); diff --git a/src/sentry_batcher.c b/src/sentry_batcher.c index a459239e0..4f2e6e3ca 100644 --- a/src/sentry_batcher.c +++ b/src/sentry_batcher.c @@ -226,9 +226,7 @@ sentry__batcher_flush(sentry_batcher_t *batcher, bool crash_safe) // crash sentry__run_write_envelope(batcher->run, envelope); sentry_envelope_free(envelope); - } else if (!batcher->user_consent - || sentry__atomic_fetch(batcher->user_consent) - == SENTRY_USER_CONSENT_GIVEN) { + } else if (!sentry__run_should_skip_upload(batcher->run)) { // Normal operation: use transport for HTTP transmission sentry__transport_send_envelope(batcher->transport, envelope); } else { @@ -376,12 +374,10 @@ sentry__batcher_startup( { // dsn is incref'd because release() decref's it and may outlive options. batcher->dsn = sentry__dsn_incref(options->dsn); - // transport, run, and user_consent are non-owning refs, safe because they + // transport and run are non-owning refs, safe because they // are only accessed in flush() which is bound by the options lifetime. batcher->transport = options->transport; batcher->run = options->run; - batcher->user_consent - = options->require_user_consent ? (long *)&options->user_consent : NULL; // Mark thread as starting before actually spawning so thread can transition // to RUNNING. This prevents shutdown from thinking the thread was never diff --git a/src/sentry_batcher.h b/src/sentry_batcher.h index 29563bf74..53a0e4bb0 100644 --- a/src/sentry_batcher.h +++ b/src/sentry_batcher.h @@ -49,7 +49,6 @@ typedef struct { sentry_dsn_t *dsn; sentry_transport_t *transport; sentry_run_t *run; - long *user_consent; // (atomic) NULL if consent not required } sentry_batcher_t; typedef struct { diff --git a/src/sentry_core.c b/src/sentry_core.c index b3c214868..4ce4987d3 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -80,13 +80,13 @@ load_user_consent(sentry_options_t *opts) sentry__path_free(consent_path); switch (contents ? contents[0] : 0) { case '1': - opts->user_consent = SENTRY_USER_CONSENT_GIVEN; + opts->run->user_consent = SENTRY_USER_CONSENT_GIVEN; break; case '0': - opts->user_consent = SENTRY_USER_CONSENT_REVOKED; + opts->run->user_consent = SENTRY_USER_CONSENT_REVOKED; break; default: - opts->user_consent = SENTRY_USER_CONSENT_UNKNOWN; + opts->run->user_consent = SENTRY_USER_CONSENT_UNKNOWN; break; } sentry_free(contents); @@ -97,9 +97,7 @@ sentry__should_skip_upload(void) { bool skip = true; SENTRY_WITH_OPTIONS (options) { - skip = options->require_user_consent - && sentry__atomic_fetch((long *)&options->user_consent) - != SENTRY_USER_CONSENT_GIVEN; + skip = sentry__run_should_skip_upload(options->run); } return skip; } @@ -205,6 +203,7 @@ sentry_init(sentry_options_t *options) SENTRY_WARN("failed to initialize run directory"); goto fail; } + options->run->require_user_consent = options->require_user_consent; load_user_consent(options); @@ -435,7 +434,7 @@ static void set_user_consent(sentry_user_consent_t new_val) { SENTRY_WITH_OPTIONS (options) { - if (sentry__atomic_store((long *)&options->user_consent, new_val) + if (sentry__atomic_store(&options->run->user_consent, new_val) != new_val) { if (options->backend && options->backend->user_consent_changed_func) { @@ -447,6 +446,8 @@ set_user_consent(sentry_user_consent_t new_val) switch (new_val) { case SENTRY_USER_CONSENT_GIVEN: sentry__path_write_buffer(consent_path, "1\n", 2); + // flush any envelopes cached while consent was revoked + sentry_transport_retry(options->transport); break; case SENTRY_USER_CONSENT_REVOKED: sentry__path_write_buffer(consent_path, "0\n", 2); @@ -484,7 +485,7 @@ sentry_user_consent_get(void) sentry_user_consent_t rv = SENTRY_USER_CONSENT_UNKNOWN; SENTRY_WITH_OPTIONS (options) { rv = (sentry_user_consent_t)(int)sentry__atomic_fetch( - (long *)&options->user_consent); + &options->run->user_consent); } return rv; } @@ -503,13 +504,19 @@ void sentry__capture_envelope( sentry_transport_t *transport, sentry_envelope_t *envelope) { - bool has_consent = !sentry__should_skip_upload(); - if (!has_consent) { - SENTRY_INFO("discarding envelope due to missing user consent"); - sentry_envelope_free(envelope); + if (!sentry__should_skip_upload()) { + sentry__transport_send_envelope(transport, envelope); return; } - sentry__transport_send_envelope(transport, envelope); + bool cached = false; + SENTRY_WITH_OPTIONS (options) { + if (options->cache_keep || options->http_retry) { + cached = sentry__run_write_cache(options->run, envelope, 0); + } + } + SENTRY_INFO(cached ? "caching envelope due to missing user consent" + : "discarding envelope due to missing user consent"); + sentry_envelope_free(envelope); } void diff --git a/src/sentry_core.h b/src/sentry_core.h index d78688b41..0c0b03e87 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -34,6 +34,10 @@ /** * This function will check the user consent, and return `true` if uploads * should *not* be sent to the sentry server, and be discarded instead. + * + * Note: This function acquires the options lock internally. Use + * `sentry__run_should_skip_upload` from worker threads that may run while + * the options are locked during SDK shutdown. */ bool sentry__should_skip_upload(void); diff --git a/src/sentry_database.c b/src/sentry_database.c index fd4f2f14c..bac8e818e 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -74,6 +74,8 @@ sentry__run_new(const sentry_path_t *database_path) } run->refcount = 1; + run->require_user_consent = 0; + run->user_consent = SENTRY_USER_CONSENT_UNKNOWN; run->uuid = uuid; run->run_path = run_path; run->session_path = session_path; @@ -96,6 +98,14 @@ sentry__run_new(const sentry_path_t *database_path) return NULL; } +bool +sentry__run_should_skip_upload(sentry_run_t *run) +{ + return sentry__atomic_fetch(&run->require_user_consent) + && (sentry__atomic_fetch(&run->user_consent) + != SENTRY_USER_CONSENT_GIVEN); +} + sentry_run_t * sentry__run_incref(sentry_run_t *run) { @@ -426,7 +436,9 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) } } else if (sentry__path_ends_with(file, ".envelope")) { sentry_envelope_t *envelope = sentry__envelope_from_path(file); - sentry__capture_envelope(options->transport, envelope); + if (envelope) { + sentry__capture_envelope(options->transport, envelope); + } } sentry__path_remove(file); @@ -438,7 +450,9 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) } sentry__pathiter_free(db_iter); - sentry__capture_envelope(options->transport, session_envelope); + if (session_envelope) { + sentry__capture_envelope(options->transport, session_envelope); + } } // Cache Pruning below is based on prune_crash_reports.cc from Crashpad diff --git a/src/sentry_database.h b/src/sentry_database.h index 33e6735ce..561874f8a 100644 --- a/src/sentry_database.h +++ b/src/sentry_database.h @@ -14,8 +14,19 @@ typedef struct sentry_run_s { sentry_path_t *cache_path; sentry_filelock_t *lock; long refcount; + long require_user_consent; // (atomic) bool + long user_consent; // (atomic) sentry_user_consent_t } sentry_run_t; +/** + * This function will check the user consent, and return `true` if uploads + * should *not* be sent to the sentry server, and be discarded instead. + * + * This is a lock-free variant of `sentry__should_skip_upload`, safe to call + * from worker threads while the options are locked during SDK shutdown. + */ +bool sentry__run_should_skip_upload(sentry_run_t *run); + /** * This creates a new application run including its associated directory and * lockfile: diff --git a/src/sentry_options.c b/src/sentry_options.c index cdb497484..51c8cca6c 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -46,7 +46,6 @@ sentry_options_new(void) } sentry_options_set_sdk_name(opts, SENTRY_SDK_NAME); opts->max_breadcrumbs = SENTRY_BREADCRUMBS_MAX; - opts->user_consent = SENTRY_USER_CONSENT_UNKNOWN; opts->auto_session_tracking = true; opts->system_crash_reporter_enabled = false; opts->attach_screenshot = false; diff --git a/src/sentry_options.h b/src/sentry_options.h index 23bae517e..8c99a58d8 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -86,7 +86,6 @@ struct sentry_options_s { struct sentry_backend_s *backend; sentry_session_t *session; - long user_consent; long refcount; uint64_t shutdown_timeout; sentry_handler_strategy_t handler_strategy; diff --git a/src/sentry_retry.c b/src/sentry_retry.c index 0c22e880e..89fc9d1a5 100644 --- a/src/sentry_retry.c +++ b/src/sentry_retry.c @@ -142,6 +142,10 @@ size_t sentry__retry_send(sentry_retry_t *retry, uint64_t before, sentry_retry_send_func_t send_cb, void *data) { + if (sentry__run_should_skip_upload(retry->run)) { + return 1; // keep the poll alive until consent is given + } + sentry_pathiter_t *piter = sentry__path_iter_directory(retry->run->cache_path); if (!piter) { diff --git a/tests/test_integration_cache.py b/tests/test_integration_cache.py index ba10a31e5..3d099a81f 100644 --- a/tests/test_integration_cache.py +++ b/tests/test_integration_cache.py @@ -263,3 +263,79 @@ def test_cache_max_items_with_retry(cmake, backend, unreachable_dsn): assert cache_dir.exists() cache_files = list(cache_dir.glob("*.envelope")) assert len(cache_files) <= 5 + + +def test_cache_consent_revoke(cmake, unreachable_dsn): + """With consent revoked and cache_keep, envelopes are cached to disk.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=unreachable_dsn) + + run( + tmp_path, + "sentry_example", + [ + "log", + "cache-keep", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "flush", + ], + env=env, + ) + + assert cache_dir.exists() + cache_files = list(cache_dir.glob("*.envelope")) + assert len(cache_files) == 1 + + +def test_cache_consent_discard(cmake, unreachable_dsn): + """With consent revoked but no cache_keep, envelopes are discarded.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=unreachable_dsn) + + run( + tmp_path, + "sentry_example", + [ + "log", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "flush", + ], + env=env, + ) + + assert not cache_dir.exists() or len(list(cache_dir.glob("*.envelope"))) == 0 + + +def test_cache_consent_flush(cmake, httpserver): + """Giving consent after capturing flushes cached envelopes immediately.""" + from . import make_dsn + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + [ + "log", + "http-retry", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "user-consent-give", + "flush", + ], + env=env, + ) + + assert len(httpserver.log) >= 1 + assert not cache_dir.exists() or len(list(cache_dir.glob("*.envelope"))) == 0 diff --git a/tests/unit/test_cache.c b/tests/unit/test_cache.c index f8e0083f7..a5b806718 100644 --- a/tests/unit/test_cache.c +++ b/tests/unit/test_cache.c @@ -3,6 +3,7 @@ #include "sentry_envelope.h" #include "sentry_options.h" #include "sentry_path.h" +#include "sentry_retry.h" #include "sentry_string.h" #include "sentry_testsupport.h" #include "sentry_uuid.h" @@ -310,6 +311,83 @@ SENTRY_TEST(cache_max_items_with_retry) sentry_close(); } +SENTRY_TEST(cache_consent_revoked) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_cache_keep(options, true); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_path_t *cache_path + = sentry__path_join_str(options->database_path, "cache"); + TEST_ASSERT(!!cache_path); + sentry__path_remove_all(cache_path); + + sentry_capture_event( + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "test", "revoked")); + + int count = 0; + bool is_retry_format = false; + sentry_pathiter_t *iter = sentry__path_iter_directory(cache_path); + const sentry_path_t *entry; + while (iter && (entry = sentry__pathiter_next(iter)) != NULL) { + if (sentry__path_ends_with(entry, ".envelope")) { + count++; + uint64_t ts; + int attempt; + const char *uuid; + is_retry_format = sentry__parse_cache_filename( + sentry__path_filename(entry), &ts, &attempt, &uuid); + } + } + sentry__pathiter_free(iter); + TEST_CHECK_INT_EQUAL(count, 1); + TEST_CHECK(is_retry_format); + + sentry__path_free(cache_path); + sentry_close(); +} + +SENTRY_TEST(cache_consent_revoked_nocache) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_cache_keep(options, false); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_path_t *cache_path + = sentry__path_join_str(options->database_path, "cache"); + TEST_ASSERT(!!cache_path); + sentry__path_remove_all(cache_path); + + sentry_capture_event( + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "test", "revoked")); + + int count = 0; + sentry_pathiter_t *iter = sentry__path_iter_directory(cache_path); + const sentry_path_t *entry; + while (iter && (entry = sentry__pathiter_next(iter)) != NULL) { + if (sentry__path_ends_with(entry, ".envelope")) { + count++; + } + } + sentry__pathiter_free(iter); + TEST_CHECK_INT_EQUAL(count, 0); + + sentry__path_free(cache_path); + sentry_close(); +} + SENTRY_TEST(cache_max_size_and_age) { #if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) diff --git a/tests/unit/test_retry.c b/tests/unit/test_retry.c index 77cc1a993..4dab3026e 100644 --- a/tests/unit/test_retry.c +++ b/tests/unit/test_retry.c @@ -489,3 +489,49 @@ SENTRY_TEST(retry_trigger) sentry__retry_free(retry); sentry_close(); } + +SENTRY_TEST(retry_consent) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_http_retry(options, true); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_retry_t *retry = sentry__retry_new(options); + TEST_ASSERT(!!retry); + + const sentry_path_t *cache_path = options->run->cache_path; + sentry__path_remove_all(cache_path); + sentry__path_create_dir_all(cache_path); + + uint64_t old_ts + = sentry__usec_time() / 1000 - 10 * sentry__retry_backoff(0); + sentry_uuid_t event_id = sentry_uuid_new_v4(); + write_retry_file(options->run, old_ts, 0, &event_id); + + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 1); + + // consent revoked: retry_send skips the round without calling send_cb, + // but returns non-zero to keep the poll alive until consent is given + retry_test_ctx_t ctx = { 200, 0 }; + size_t remaining = sentry__retry_send(retry, 0, test_send_cb, &ctx); + TEST_CHECK_INT_EQUAL(ctx.count, 0); + TEST_CHECK(remaining != 0); + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 1); + + // give consent: retry_send sends and removes the file + sentry_user_consent_give(); + ctx = (retry_test_ctx_t) { 200, 0 }; + remaining = sentry__retry_send(retry, UINT64_MAX, test_send_cb, &ctx); + TEST_CHECK_INT_EQUAL(ctx.count, 1); + TEST_CHECK_INT_EQUAL(remaining, 0); + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 0); + + sentry__retry_free(retry); + sentry_close(); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index a1df4cafa..f072d73f8 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -40,6 +40,8 @@ XX(bgworker_flush) XX(bgworker_task_delay) XX(breadcrumb_without_type_or_message_still_valid) XX(build_id_parser) +XX(cache_consent_revoked) +XX(cache_consent_revoked_nocache) XX(cache_keep) XX(cache_max_age) XX(cache_max_items) @@ -207,6 +209,7 @@ XX(read_write_envelope_to_invalid_path) XX(recursive_paths) XX(retry_backoff) XX(retry_cache) +XX(retry_consent) XX(retry_filename) XX(retry_make_cache_path) XX(retry_result)