Avoid throwing validation error on invalid URIs

The recent update of jsonschema inadvertently caused extra validation of
`uri-reference` formatted properties. This change ensures that these
errors are converted to warnings.

Note that we cannot revert the change to jsonschema since the old
version does not handle `uniqueItems` correctly.

lib/upload-lib.js

@@ -181,16 +181,23 @@ function validateSarifFileSchema(sarifFilePath, logger) {
     const sarif = JSON.parse(fs.readFileSync(sarifFilePath, "utf8"));
     const schema = require("../src/sarif-schema-2.1.0.json");
     const result = new jsonschema.Validator().validate(sarif, schema);
-    if (!result.valid) {
+    // Filter errors related to invalid URIs in the artifactLocation field as this
+    // is a breaking change. See https://github.com/github/codeql-action/issues/1703
+    const errors = (result.errors || []).filter((err) => err.argument !== "uri-reference");
+    const warnings = (result.errors || []).filter((err) => err.argument === "uri-reference");
+    for (const warning of warnings) {
+        logger.info(`Warning: '${warning.instance}' is not a valid URI in '${warning.property}'.`);
+    }
+    if (errors.length) {
         // Output the more verbose error messages in groups as these may be very large.
-        for (const error of result.errors) {
+        for (const error of errors) {
             logger.startGroup(`Error details: ${error.stack}`);
             logger.info(JSON.stringify(error, null, 2));
             logger.endGroup();
         }
         // Set the main error message to the stacks of all the errors.
         // This should be of a manageable size and may even give enough to fix the error.
-        const sarifErrors = result.errors.map((e) => `- ${e.stack}`);
+        const sarifErrors = errors.map((e) => `- ${e.stack}`);
         throw new Error(`Unable to upload "${sarifFilePath}" as it is not valid SARIF:\n${sarifErrors.join("\n")}`);
     }
 }

lib/upload-lib.test.js

@@ -233,6 +233,18 @@ ava_1.default.beforeEach(() => {
     t.deepEqual(loggedMessages.length, 1);
     t.assert(loggedMessages[0].includes("Pruned 2 results"));
 });
+(0, ava_1.default)("accept results with invalid artifactLocation.uri value", (t) => {
+    const loggedMessages = [];
+    const mockLogger = {
+        info: (message) => {
+            loggedMessages.push(message);
+        },
+    };
+    const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
+    uploadLib.validateSarifFileSchema(sarifFile, mockLogger);
+    t.deepEqual(loggedMessages.length, 1);
+    t.deepEqual(loggedMessages[0], "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.");
+});
 const affectedCodeQLVersion = {
     driver: {
         name: "CodeQL",

src/testdata/with-invalid-uri.sarif

@@ -0,0 +1,42 @@
+{
+  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
+  "version": "2.1.0",
+  "runs": [
+    {
+      "tool": {
+        "driver": {
+          "name": "LGTM.com",
+          "organization": "Semmle",
+          "version": "1.24.0-SNAPSHOT",
+          "rules": []
+        }
+      },
+      "results" : [ {
+        "ruleId" : "js/unused-local-variable",
+        "ruleIndex" : 0,
+        "message" : {
+          "text" : "Unused variable foo."
+        },
+        "locations" : [ {
+          "physicalLocation" : {
+            "artifactLocation" : {
+              "uri" : "not a valid URI",
+              "uriBaseId" : "%SRCROOT%",
+              "index" : 0
+            },
+            "region" : {
+              "startLine" : 2,
+              "startColumn" : 7,
+              "endColumn" : 10
+            }
+          }
+        } ]
+      } ],
+      "columnKind": "utf16CodeUnits",
+      "properties": {
+        "semmle.formatSpecifier": "2.1.0",
+        "semmle.sourceLanguage": "java"
+      }
+    }
+  ]
+}

src/upload-lib.test.ts

@@ -360,6 +360,23 @@ test("pruneInvalidResults", (t) => {
   t.assert(loggedMessages[0].includes("Pruned 2 results"));
 });
 
+test("accept results with invalid artifactLocation.uri value", (t) => {
+  const loggedMessages: string[] = [];
+  const mockLogger = {
+    info: (message: string) => {
+      loggedMessages.push(message);
+    },
+  } as Logger;
+
+  const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
+  uploadLib.validateSarifFileSchema(sarifFile, mockLogger);
+
+  t.deepEqual(loggedMessages.length, 1);
+  t.deepEqual(
+    loggedMessages[0],
+    "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'."
+  );
+});
 const affectedCodeQLVersion = {
   driver: {
     name: "CodeQL",

src/upload-lib.ts

@@ -228,9 +228,24 @@ export function validateSarifFileSchema(sarifFilePath: string, logger: Logger) {
   const schema = require("../src/sarif-schema-2.1.0.json") as jsonschema.Schema;
 
   const result = new jsonschema.Validator().validate(sarif, schema);
-  if (!result.valid) {
+  // Filter errors related to invalid URIs in the artifactLocation field as this
+  // is a breaking change. See https://github.com/github/codeql-action/issues/1703
+  const errors = (result.errors || []).filter(
+    (err) => err.argument !== "uri-reference"
+  );
+  const warnings = (result.errors || []).filter(
+    (err) => err.argument === "uri-reference"
+  );
+
+  for (const warning of warnings) {
+    logger.info(
+      `Warning: '${warning.instance}' is not a valid URI in '${warning.property}'.`
+    );
+  }
+
+  if (errors.length) {
     // Output the more verbose error messages in groups as these may be very large.
-    for (const error of result.errors) {
+    for (const error of errors) {
       logger.startGroup(`Error details: ${error.stack}`);
       logger.info(JSON.stringify(error, null, 2));
       logger.endGroup();
@@ -238,7 +253,7 @@ export function validateSarifFileSchema(sarifFilePath: string, logger: Logger) {
 
     // Set the main error message to the stacks of all the errors.
     // This should be of a manageable size and may even give enough to fix the error.
-    const sarifErrors = result.errors.map((e) => `- ${e.stack}`);
+    const sarifErrors = errors.map((e) => `- ${e.stack}`);
     throw new Error(
       `Unable to upload "${sarifFilePath}" as it is not valid SARIF:\n${sarifErrors.join(
         "\n"