From f6eda12d7180e3801dc3ac9def8c3c61048137c4 Mon Sep 17 00:00:00 2001 From: relaxolotl <5597345+relaxolotl@users.noreply.github.com> Date: Wed, 30 Mar 2022 17:59:30 -0400 Subject: [PATCH] ref: Switch the internal representation of project IDs from ints over to char*s (#690) This tries to future-proof project IDs as they aren't guaranteed to be numbers forever. Project IDs are now treated as opaque strings, and minimal validation is now applied to them to reflect that fact. --- CHANGELOG.md | 3 +++ src/sentry_utils.c | 18 ++++++++---------- src/sentry_utils.h | 2 +- tests/unit/test_utils.c | 30 ++++++++++++++++-------------- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index facc746b3..f8437350e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ - New API to check whether the application has crashed in the previous run: `sentry_get_crashed_last_run()` and `sentry_clear_crashed_last_run()` ([#685](https://github.com/getsentry/sentry-native/pull/685)). - Allow overriding the SDK name at build time - set the `SENTRY_SDK_NAME` CMake cache variable. +**Internal**: +- Project IDs are now treated as opaque strings instead of integer values. ([#690](https://github.com/getsentry/sentry-native/pull/690)) + **Fixes**: - Updated CI as well as list of supported platforms to reflect Windows Server 2016, and therefore MSVC 2017 losing active support. diff --git a/src/sentry_utils.c b/src/sentry_utils.c index 65f2a2415..569fa0833 100644 --- a/src/sentry_utils.c +++ b/src/sentry_utils.c @@ -218,8 +218,7 @@ sentry__dsn_new(const char *raw_dsn) sentry_url_t url; memset(&url, 0, sizeof(sentry_url_t)); size_t path_len; - char *tmp; - char *end; + char *project_id; sentry_dsn_t *dsn = SENTRY_MAKE(sentry_dsn_t); if (!dsn) { @@ -255,16 +254,14 @@ sentry__dsn_new(const char *raw_dsn) path_len--; } - tmp = strrchr(url.path, '/'); - if (!tmp) { + project_id = strrchr(url.path, '/'); + if (!project_id || strlen(project_id + 1) == 0) { goto exit; } - dsn->project_id = (uint64_t)strtoll(tmp + 1, &end, 10); - if (end != tmp + strlen(tmp)) { - goto exit; - } - *tmp = 0; + dsn->project_id = sentry__string_clone(project_id + 1); + *project_id = 0; + dsn->path = url.path; url.path = NULL; @@ -299,6 +296,7 @@ sentry__dsn_decref(sentry_dsn_t *dsn) sentry_free(dsn->path); sentry_free(dsn->public_key); sentry_free(dsn->secret_key); + sentry_free(dsn->project_id); sentry_free(dsn); } } @@ -329,7 +327,7 @@ init_string_builder_for_url(sentry_stringbuilder_t *sb, const sentry_dsn_t *dsn) sentry__stringbuilder_append_int64(sb, (int64_t)dsn->port); sentry__stringbuilder_append(sb, dsn->path); sentry__stringbuilder_append(sb, "/api/"); - sentry__stringbuilder_append_int64(sb, (int64_t)dsn->project_id); + sentry__stringbuilder_append(sb, dsn->project_id); } char * diff --git a/src/sentry_utils.h b/src/sentry_utils.h index cb79c08f1..8b19e65c2 100644 --- a/src/sentry_utils.h +++ b/src/sentry_utils.h @@ -49,7 +49,7 @@ typedef struct sentry_dsn_s { char *path; char *secret_key; char *public_key; - uint64_t project_id; + char *project_id; int port; long refcount; bool is_valid; diff --git a/tests/unit/test_utils.c b/tests/unit/test_utils.c index a8a6f26bf..f280f59f7 100644 --- a/tests/unit/test_utils.c +++ b/tests/unit/test_utils.c @@ -71,7 +71,7 @@ SENTRY_TEST(url_parsing_invalid) SENTRY_TEST(dsn_parsing_complete) { sentry_dsn_t *dsn = sentry__dsn_new( - "http://username:password@example.com/foo/bar/42?x=y#z"); + "http://username:password@example.com/foo/bar/42%21?x=y#z"); TEST_CHECK(!!dsn); if (!dsn) { return; @@ -83,10 +83,10 @@ SENTRY_TEST(dsn_parsing_complete) TEST_CHECK_STRING_EQUAL(dsn->public_key, "username"); TEST_CHECK_STRING_EQUAL(dsn->secret_key, "password"); TEST_CHECK_STRING_EQUAL(dsn->path, "/foo/bar"); - TEST_CHECK_INT_EQUAL((int)dsn->project_id, 42); + TEST_CHECK_STRING_EQUAL(dsn->project_id, "42%21"); sentry__dsn_decref(dsn); - dsn = sentry__dsn_new("https://username@example.com/42"); + dsn = sentry__dsn_new("https://username@example.com/42%21"); TEST_CHECK(!!dsn); if (!dsn) { return; @@ -97,22 +97,24 @@ SENTRY_TEST(dsn_parsing_complete) TEST_CHECK_STRING_EQUAL(dsn->public_key, "username"); TEST_CHECK(!dsn->secret_key); TEST_CHECK_STRING_EQUAL(dsn->path, ""); - TEST_CHECK_INT_EQUAL((int)dsn->project_id, 42); + TEST_CHECK_STRING_EQUAL(dsn->project_id, "42%21"); sentry__dsn_decref(dsn); -} -SENTRY_TEST(dsn_parsing_invalid) -{ - sentry_dsn_t *dsn - = sentry__dsn_new("http://username:password@example.com/foo/bar?x=y#z"); + dsn = sentry__dsn_new("https://username@example.com/pathone/pathtwo/42%21"); TEST_CHECK(!!dsn); - if (dsn) { - TEST_CHECK(!dsn->is_valid); - sentry__dsn_decref(dsn); + if (!dsn) { + return; } + TEST_CHECK(dsn->is_valid); + TEST_CHECK_STRING_EQUAL(dsn->path, "/pathone/pathtwo"); + TEST_CHECK_STRING_EQUAL(dsn->project_id, "42%21"); + sentry__dsn_decref(dsn); +} - dsn = sentry__dsn_new("=https://foo@bar.ingest.sentry.io/" - "1234567"); +SENTRY_TEST(dsn_parsing_invalid) +{ + sentry_dsn_t *dsn = sentry__dsn_new("=https://foo@bar.ingest.sentry.io/" + "1234567"); TEST_CHECK(!!dsn); if (dsn) { TEST_CHECK(!dsn->is_valid);