From b27e220d2014e6d0ee8dabe69c76189a14bb02ac Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Thu, 30 Nov 2023 10:09:48 -0500 Subject: [PATCH 1/2] fix(sourcemaps): don't attempt to treat remote URL as a local file path When a sourceMappingURL is a remote URL (e.g. static storage in an S3 bucket), `sentry-cli sourcemaps inject` was treating it like a normal file path, attempting to "normalize" it with the source path, and producing a bogus url such as `path/to/source/dir/https://some-static-host.example.com/path/to/foo.js.map`, which of course doesn't exist, and thus the sourcemap isn't found and doesn't have the debug id injected - even if it's sitting right there in the local directory, next to its corresponding bundled source file. With this change, the sourcemap discovery step does not attempt to use the discovered sourcemap URL if it is a remote URL. Instead, it falls back to guessing the sourcemap location based on the source filename and its location. I also changed a couple trycmd tests to remove the `+` before the timezone offset, as it causes the tests to fail when the local timezone is west of UTC. Fixes #1846. --- src/utils/sourcemaps.rs | 17 ++++++++++++++++- .../sourcemaps-inject-split-ambiguous.trycmd | 6 +++--- .../sourcemaps-upload-some-debugids.trycmd | 16 ++++++++-------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/utils/sourcemaps.rs b/src/utils/sourcemaps.rs index ebd546622d..f6cf0cb524 100644 --- a/src/utils/sourcemaps.rs +++ b/src/utils/sourcemaps.rs @@ -239,6 +239,14 @@ fn url_matches_extension(url: &str, extensions: &[&str]) -> bool { .unwrap_or(false) } +/// Return true iff url is a remote url (not a local path or embedded sourcemap). +fn is_remote_url(url: &str) -> bool { + return match Url::parse(url) { + Ok(url) => url.scheme() != "data", + Err(_) => false, + }; +} + impl SourceMapProcessor { /// Creates a new sourcemap validator. pub fn new() -> SourceMapProcessor { @@ -364,7 +372,14 @@ impl SourceMapProcessor { continue; }; - let sourcemap_reference = match discover_sourcemaps_location(contents) { + // If this is a full external URL, the code below is going to attempt + // to "normalize" it with the source path, resulting in a bogus path + // like "path/to/source/dir/https://some-static-host.example.com/path/to/foo.js.map" + // that can't be resolved to a source map file. + // Instead, we pretend we failed to discover the location, and we fall back to + // guessing the source map location based on the source location. + let location = discover_sourcemaps_location(contents).filter(|loc| !is_remote_url(loc)); + let sourcemap_reference = match location { Some(url) => SourceMapReference::from_url(url.to_string()), None => match guess_sourcemap_reference(&sourcemaps, &source.url) { Ok(target) => target, diff --git a/tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.trycmd b/tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.trycmd index 06e044e168..4d9cd2fd61 100644 --- a/tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.trycmd +++ b/tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.trycmd @@ -9,9 +9,9 @@ $ sentry-cli sourcemaps inject ./code ./maps ./maps2 --log-level warn > Found 1 file > Analyzing 5 sources > Injecting debug ids - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] Ambiguous matches for sourcemap path ./code/foo/index.js.map: - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] ./maps/foo/index.js.map - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] ./maps2/foo/index.js.map + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] Ambiguous matches for sourcemap path ./code/foo/index.js.map: + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] ./maps/foo/index.js.map + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] ./maps2/foo/index.js.map Source Map Debug ID Injection Report Modified: The following source files have been modified to have debug ids diff --git a/tests/integration/_cases/sourcemaps/sourcemaps-upload-some-debugids.trycmd b/tests/integration/_cases/sourcemaps/sourcemaps-upload-some-debugids.trycmd index 126fbef4f3..4206f2e412 100644 --- a/tests/integration/_cases/sourcemaps/sourcemaps-upload-some-debugids.trycmd +++ b/tests/integration/_cases/sourcemaps/sourcemaps-upload-some-debugids.trycmd @@ -5,14 +5,14 @@ $ sentry-cli sourcemaps upload tests/integration/_fixtures/upload_some_debugids > Analyzing 20 sources > Rewriting sources > Adding source map references - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] Some source files don't have debug ids: - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/server/app/page.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/server/chunks/1.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/server/dummy_embedded.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/server/pages/_document.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/server/pages/api/hello.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/static/chunks/575-bb7d7e0e6de8d623.js - WARN [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] - ~/static/chunks/pages/asdf-05b39167abbe433b.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] Some source files don't have debug ids: + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/server/app/page.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/server/chunks/1.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/server/dummy_embedded.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/server/pages/_document.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/server/pages/api/hello.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/static/chunks/575-bb7d7e0e6de8d623.js + WARN [..]-[..]-[..] [..]:[..]:[..].[..] [..]:[..] - ~/static/chunks/pages/asdf-05b39167abbe433b.js > Bundled 20 files for upload > Bundle ID: [..]-[..]-[..]-[..]-[..] > Uploaded files to Sentry From 7374c4ff7cdb441fd71726b476a535da35989a2e Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Thu, 30 Nov 2023 12:07:16 -0500 Subject: [PATCH 2/2] test(sourcemaps): add integration test for external, full sourcemap URL --- .../_cases/sourcemaps/sourcemaps-inject.trycmd | 6 ++++-- .../integration/_fixtures/inject/server/dummy_hosted.js | 1 + .../_fixtures/inject/server/dummy_hosted.js.map | 9 +++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 tests/integration/_fixtures/inject/server/dummy_hosted.js create mode 100644 tests/integration/_fixtures/inject/server/dummy_hosted.js.map diff --git a/tests/integration/_cases/sourcemaps/sourcemaps-inject.trycmd b/tests/integration/_cases/sourcemaps/sourcemaps-inject.trycmd index c08beae665..ee8bb6d681 100644 --- a/tests/integration/_cases/sourcemaps/sourcemaps-inject.trycmd +++ b/tests/integration/_cases/sourcemaps/sourcemaps-inject.trycmd @@ -2,10 +2,10 @@ $ sentry-cli sourcemaps inject ./server ./static ? success > Searching ./server -> Found 12 files +> Found 14 files > Searching ./static > Found 8 files -> Analyzing 20 sources +> Analyzing 22 sources > Injecting debug ids Source Map Debug ID Injection Report @@ -13,6 +13,7 @@ Source Map Debug ID Injection Report [..]-[..]-[..]-[..]-[..] - ./server/app/page.js [..]-[..]-[..]-[..]-[..] - ./server/chunks/1.js [..]-[..]-[..]-[..]-[..] - ./server/dummy_embedded.js + [..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted.js [..]-[..]-[..]-[..]-[..] - ./server/flight-manifest.js [..]-[..]-[..]-[..]-[..] - ./server/pages/_document.js [..]-[..]-[..]-[..]-[..] - ./server/pages/api/hello.js @@ -21,6 +22,7 @@ Source Map Debug ID Injection Report [..]-[..]-[..]-[..]-[..] - ./static/chunks/app/head-172ad45600676c06.js [..]-[..]-[..]-[..]-[..] - ./static/chunks/pages/asdf-05b39167abbe433b.js Modified: The following sourcemap files have been modified to have debug ids + [..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted.js.map [..]-[..]-[..]-[..]-[..] - ./server/pages/_document.js.map [..]-[..]-[..]-[..]-[..] - ./static/chunks/575-bb7d7e0e6de8d623.js.map Ignored: The following source files already have debug ids diff --git a/tests/integration/_fixtures/inject/server/dummy_hosted.js b/tests/integration/_fixtures/inject/server/dummy_hosted.js new file mode 100644 index 0000000000..352cc4e62c --- /dev/null +++ b/tests/integration/_fixtures/inject/server/dummy_hosted.js @@ -0,0 +1 @@ +//# sourceMappingURL=https://some-static-hosting.example.com/path/to/dummy_embedded.js.map diff --git a/tests/integration/_fixtures/inject/server/dummy_hosted.js.map b/tests/integration/_fixtures/inject/server/dummy_hosted.js.map new file mode 100644 index 0000000000..648aa29a7b --- /dev/null +++ b/tests/integration/_fixtures/inject/server/dummy_hosted.js.map @@ -0,0 +1,9 @@ +{ + "version": 3, + "file": "1.js", + "mappings": ";;;", + "sources": [], + "sourcesContent": [], + "names": [], + "sourceRoot": "" +}