fix(sourcemaps): Fix double association bug

Fixes a bug where the same sourcemap can be associated with mulitple source files.

Fixes #2445
Fixes [CLI-5](https://linear.app/getsentry/issue/CLI-5/ensure-collect-sourcemap-references-does-not-associate-the-same)

Author: szokeasaurusrex

src/utils/file_upload.rs

@@ -211,7 +211,7 @@ impl<'a> TryFrom<&'a UploadContext<'_>> for LegacyUploadContext<'a> {
     }
 }
 
-#[derive(Eq, PartialEq, Debug, Copy, Clone)]
+#[derive(Eq, PartialEq, Debug, Copy, Clone, Hash)]
 pub enum LogLevel {
     Warning,
     Error,
@@ -226,7 +226,7 @@ impl fmt::Display for LogLevel {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct SourceFile {
     pub url: String,
     pub path: PathBuf,

src/utils/sourcemaps.rs

@@ -287,53 +287,78 @@ impl SourceMapProcessor {
     /// Collect references to sourcemaps in minified source files
     /// and saves them in `self.sourcemap_references`.
     fn collect_sourcemap_references(&mut self) {
-        let sourcemaps = self
+        // Collect available sourcemaps
+        let sourcemaps: HashSet<_> = self
             .sources
             .iter()
             .map(|x| x.1)
             .filter(|x| x.ty == SourceFileType::SourceMap)
             .map(|x| x.url.clone())
             .collect();
 
-        for source in self.sources.values_mut() {
-            // Skip everything but minified JS files.
-            if source.ty != SourceFileType::MinifiedSource {
-                continue;
-            }
+        let mut already_associated_sourcemaps = HashSet::new();
 
-            if self.sourcemap_references.contains_key(&source.url) {
-                continue;
-            }
-
-            let Ok(contents) = std::str::from_utf8(&source.contents) else {
-                continue;
-            };
+        let unassociated_js_source_locations: HashMap<_, _> = self
+            .sources
+            .values_mut()
+            .filter(|source| source.ty == SourceFileType::MinifiedSource)
+            .filter(|source| !self.sourcemap_references.contains_key(&source.url))
+            .filter_map(|source| {
+                str::from_utf8(&source.contents.clone())
+                    .map(|contents| {
+                        (
+                            source,
+                            discover_sourcemaps_location(contents)
+                                .filter(|loc| !is_remote_sourcemap(loc))
+                                .map(String::from),
+                        )
+                    })
+                    .ok()
+            })
+            .collect();
 
-            // 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_sourcemap(loc));
-            let sourcemap_reference = match location {
-                Some(url) => SourceMapReference::from_url(url.to_owned()),
-                None => match guess_sourcemap_reference(&sourcemaps, &source.url) {
-                    Ok(target) => target,
-                    Err(err) => {
+        // First pass: if location discovered, add to sourcemap_references
+        unassociated_js_source_locations
+            .iter()
+            .filter_map(|(source, location)| location.as_ref().map(|location| (source, location)))
+            .for_each(|(source, location)| {
+                // Add location to already associated sourcemaps, so we cannot guess it again.
+                already_associated_sourcemaps.insert(location.to_owned());
+
+                self.sourcemap_references.insert(
+                    source.url.clone(),
+                    Some(SourceMapReference::from_url(location.to_owned())),
+                );
+            });
+
+        // Second pass: for remaining sourcemaps, try to guess the location
+        unassociated_js_source_locations
+            .into_iter()
+            .filter(|(_, location)| location.is_none())
+            .for_each(|(source, _)| {
+                let sourcemap_reference = guess_sourcemap_reference(&sourcemaps, &source.url)
+                    .inspect_err(|err| {
                         source.warn(format!(
                             "could not determine a source map reference ({err})"
                         ));
-                        self.sourcemap_references.insert(source.url.clone(), None);
-                        continue;
-                    }
-                },
-            };
+                    })
+                    .ok()
+                    .filter(|sourcemap_reference| {
+                        !already_associated_sourcemaps.contains(&sourcemap_reference.url)
+                    })
+                    .inspect(|sourcemap_reference| {
+                        // In practice, original_url is always set in guess_sourcemap_reference
+                        if let Some(original_url) =
+                            sourcemap_reference.original_url.as_ref().cloned()
+                        {
+                            // Add original url to already associated sourcemaps, so we cannot guess it again.
+                            already_associated_sourcemaps.insert(original_url);
+                        }
+                    });
 
-            self.sourcemap_references
-                .insert(source.url.clone(), Some(sourcemap_reference));
-        }
+                self.sourcemap_references
+                    .insert(source.url.clone(), sourcemap_reference);
+            });
     }
 
     pub fn dump_log(&self, title: &str) {

tests/integration/_cases/sourcemaps/sourcemaps-inject-double-association.trycmd

@@ -0,0 +1,17 @@
+```
+$ sentry-cli sourcemaps inject ./
+? success
+> Searching ./
+> Found 3 files
+> Analyzing 3 sources
+> Injecting debug ids
+
+Source Map Debug ID Injection Report
+  Modified: The following source files have been modified to have debug ids
+    d577e31d-376c-5043-b751-add9ec84b7bc - ./app.min.js
+    f3aa1477-a45c-540a-9905-f6d9f6d1d7e1 - ./references_app.min.js
+  Modified: The following sourcemap files have been modified to have debug ids
+    f3aa1477-a45c-540a-9905-f6d9f6d1d7e1 - ./app.min.js.map
+
+
+```

tests/integration/_expected_outputs/sourcemaps/sourcemaps-inject-double-association/app.min.js

@@ -0,0 +1,6 @@
+
+!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="f3aa1477-a45c-540a-9905-f6d9f6d1d7e1")}catch(e){}}();
+!function(e,t){"use strict";function n(e){return document.querySelector(e)}function r(e,t){e.textContent=t}var o=n("#message");r(o,"Hello World!")}(window,document);
+//# sourceMappingURL=app.min.js.map
+
+//# debugId=f3aa1477-a45c-540a-9905-f6d9f6d1d7e1

tests/integration/_expected_outputs/sourcemaps/sourcemaps-inject-double-association/app.min.js.map

@@ -0,0 +1 @@
+!function(e,t){"use strict";function n(e){return document.getElementById(e)}function r(e,t){e.addEventListener("click",t)}var o=n("btn");r(o,function(){console.log("Button clicked!")})}(window,document);

tests/integration/_expected_outputs/sourcemaps/sourcemaps-inject-double-association/references_app.min.js

@@ -194,6 +194,28 @@ fn command_sourcemaps_inject_indexed() {
     assert_directories_equal(TESTCASE_PATH, EXPECTED_OUTPUT_PATH);
 }
 
+#[test]
+fn command_sourcemaps_inject_double_association() {
+    let testcase_cwd_path =
+        "tests/integration/_cases/sourcemaps/sourcemaps-inject-double-association.in/";
+    if std::path::Path::new(testcase_cwd_path).exists() {
+        remove_dir_all(testcase_cwd_path).unwrap();
+    }
+    copy_recursively(
+        "tests/integration/_fixtures/inject_double_association/",
+        testcase_cwd_path,
+    )
+    .unwrap();
+
+    TestManager::new()
+        .register_trycmd_test("sourcemaps/sourcemaps-inject-double-association.trycmd");
+
+    assert_directories_equal(
+        testcase_cwd_path,
+        "tests/integration/_expected_outputs/sourcemaps/sourcemaps-inject-double-association",
+    );
+}
+
 /// Recursively assert that the contents of two directories are equal.
 ///
 /// We only support directories that contain exclusively text files.