decode SARIF URIs when filtering diff-informed alerts

src/upload-lib.test.ts

@@ -1043,3 +1043,61 @@ test.serial(
     t.deepEqual(actualOutput, expectedOutput);
   },
 );
+
+for (const [filePath, uri] of [
+  ["src/with space.js", "src/with%20space.js"],
+  ["src/caf\u00e9.js", "src/caf%C3%A9.js"],
+  ["src/caf\u00e9.js", "src/caf%c3%a9.js"],
+  ["src/!()'*.js", "src/%21%28%29%27%2A.js"],
+  ["src/literal%20.js", "src/literal%2520.js"],
+]) {
+  test.serial(`filterAlertsByDiffRange decodes URI ${uri}`, (t) => {
+    const input = sarif.readSarifFile(
+      `${__dirname}/../src/testdata/valid-sarif.sarif`,
+    );
+    input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri =
+      uri;
+
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: filePath, startLine: 1, endLine: 3 },
+    ]);
+
+    t.is(actualOutput.runs?.[0].results?.length, 1);
+  });
+}
+
+for (const uri of ["src/bad%.js", "src/bad%GG.js", "src/bad%C3%28.js"]) {
+  test.serial(
+    `filterAlertsByDiffRange falls back for malformed URI ${uri}`,
+    (t) => {
+      const input = sarif.readSarifFile(
+        `${__dirname}/../src/testdata/valid-sarif.sarif`,
+      );
+      input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri =
+        uri;
+
+      const actualOutput = runFilterAlertsByDiffRange(input, [
+        { path: uri, startLine: 0, endLine: 0 },
+      ]);
+
+      t.is(actualOutput.runs?.[0].results?.length, 1);
+    },
+  );
+}
+
+test.serial(
+  "filterAlertsByDiffRange filters malformed URI without a matching raw path",
+  (t) => {
+    const input = sarif.readSarifFile(
+      `${__dirname}/../src/testdata/valid-sarif.sarif`,
+    );
+    input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri =
+      "src/bad%GG.js";
+
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: "src/different.js", startLine: 0, endLine: 0 },
+    ]);
+
+    t.is(actualOutput.runs?.[0].results?.length, 0);
+  },
+);

src/upload-lib.ts

@@ -1099,17 +1099,29 @@ export function filterAlertsByDiffRange(
         return locations.some((physicalLocation) => {
           const locationUri = physicalLocation?.artifactLocation?.uri;
           const locationStartLine = physicalLocation?.region?.startLine;
-          if (!locationUri || locationStartLine === undefined) {
+          if (
+            typeof locationUri !== "string" ||
+            !locationUri ||
+            locationStartLine === undefined
+          ) {
             return false;
           }
+          let locationPath = locationUri;
+          try {
+            locationPath = decodeURIComponent(locationUri);
+          } catch {
+            logger.debug(
+              `Failed to decode location URI "${locationUri}", comparing it as a raw path`,
+            );
+          }
           // Alert filtering here replicates the same behavior as the restrictAlertsTo
           // extensible predicate in CodeQL. See the restrictAlertsTo documentation
           // https://codeql.github.com/codeql-standard-libraries/csharp/codeql/util/AlertFiltering.qll/predicate.AlertFiltering$restrictAlertsTo.3.html
           // for more details, such as why the filtering applies only to the first line
           // of an alert location.
           return diffRanges.some(
             (range) =>
-              range.path === locationUri &&
+              range.path === locationPath &&
               ((range.startLine <= locationStartLine &&
                 range.endLine >= locationStartLine) ||
                 (range.startLine === 0 && range.endLine === 0)),

unreleased-change-notes/fix-diff-informed-sarif-paths.md

@@ -0,0 +1 @@
+- Fixed diff-informed alert filtering incorrectly removing alerts in changed files whose paths contain spaces, special characters, or non-ASCII characters.