[ZiadMarey]

---

Summary by Gitar

• Identifier URL export:
  • Added CleartextProtocolFilter.isKnownIdentifier and exportIdentifierUrls backed by an currently empty inventory
  • Added IdentifierUrlsExporter to write identifier-urls.json during prepare-package and packaged it in the npm assembly
• Documentation:
  • Documented consumer matching rules for identifier-urls.json in the npm README

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

ACOMMONS-237

[Luqmansonar]

Two test gaps before this matches the acceptance criteria.

1. Packaged npm import

IdentifierUrlsExporterTest checks toJson() and main(), but nothing opens the assembled npm package. The acceptance criteria also require that a Node consumer can import the packaged identifier-urls.json.

npm.xml and package.json files both list that file, and CI uploads sonar-analyzer-commons-configurations-*-npm.tar.gz. A wrong assembly entry would still leave these tests green.

Please add a check that unpacks package/identifier-urls.json from that tarball and imports it the way the README shows:

import identifierUrls from '@sonarsource/analyzer-commons-configurations/identifier-urls.json' with { type: 'json' };

Assert 50 entries and that the strings match CleartextProtocolFilter.exportIdentifierUrls(), including www.topografix.com -> ^/GPX.

2. Identifier URL cases

Two representative positives from the shared SonarJS suite are missing, and the whitespace/backslash rules now documented in the npm README are not locked in CleartextProtocolFilterTest.

Please add these positives:

• http://neverssl.com/
• http://osmand.net?mode=extensions#schema — this is the case that requires prepending / before ? or # on a ^/ domain
• " http://www.w3.org/2000/svg " — isKnownIdentifier strips surrounding whitespace before matching

And these negatives:

• http://www.w3.org/2000/svg extra (internal whitespace)
• a backslash in the path, for example http://www.w3.org/2000\svg

Java already rejects the last two. A test should keep that aligned with the consumer contract in the README.

[sonarqube-next]

Quality Gate failed

Failed conditions
75.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

[gitar-bot]

💡 Quality: Identifier matching semantics are now dead code with no tests

With IDENTIFIER_URLS = Map.of(), the lookup at line 246 always returns null, so the suffix normalisation and case-sensitive find() at lines 247-252 never run. This delta also deleted the tests that covered them: exact authority with port, the empty-suffix /, the / prefix for ?/#, and case-sensitive paths. The only test left checks one negative lookup. The javadoc (lines 238-239) and the npm README (lines 29-39) still promise these semantics to non-JVM consumers, but nothing checks them. A regression would only show up once the inventory is filled in, and it would spread to SonarJS through the exported JSON. To fix this, add a package-private overload isKnownIdentifier(String url, Map<String, Pattern> inventory) that the public method calls, and bring back the semantic tests against a test-only map such as example.invalid -> ^/schema.

Make the matcher testable independently of the (currently empty) production inventory.:

public static boolean isKnownIdentifier(String url) {
  return isKnownIdentifier(url, IDENTIFIER_URLS);
}

static boolean isKnownIdentifier(String url, Map<String, Pattern> inventory) {
  var matcher = IDENTIFIER_URL.matcher(url.strip());
  if (!matcher.matches()) {
    return false;
  }
  var pattern = inventory.get(matcher.group(1).toLowerCase(Locale.ROOT));
  var remainder = matcher.group(2);
  var suffix = remainder == null ? "/" : remainder;
  if (!suffix.startsWith("/")) {
    suffix = "/" + suffix;
  }
  return pattern != null && pattern.matcher(suffix).find();
}

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 2 closed / 3 findings

🟡 Medium risk · The new identifier-URL API and npm artifact create a consumer-facing matching contract; semantic mismatches could incorrectly suppress or retain analyzer findings.

Adds CleartextProtocolFilter.isKnownIdentifier and exportIdentifierUrls plus an IdentifierUrlsExporter that writes identifier-urls.json during prepare-package, with matching rules documented in the npm README. Approved with suggestions: the identifier matching logic is currently dead code because the inventory is empty, and its semantics (authority/port, suffix normalisation, case-sensitive paths) lost their tests in this change. Consider adding a package-private overload that accepts an inventory map and restoring those tests against a test-only entry such as example.invalid -> ^/schema. Resolved: the README now covers the whitespace and backslash rejection rules, and whitespace-padded URLs match consistently across Java and SonarJS.

💡 Quality: Identifier matching semantics are now dead code with no tests

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:228 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:238-252 📄 commons/src/test/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilterTest.java:30-34 📄 sonar-analyzer-commons-configurations/src/main/packaging/npm/README.md:29-39

With IDENTIFIER_URLS = Map.of(), the lookup at line 246 always returns null, so the suffix normalisation and case-sensitive find() at lines 247-252 never run. This delta also deleted the tests that covered them: exact authority with port, the empty-suffix /, the / prefix for ?/#, and case-sensitive paths. The only test left checks one negative lookup. The javadoc (lines 238-239) and the npm README (lines 29-39) still promise these semantics to non-JVM consumers, but nothing checks them. A regression would only show up once the inventory is filled in, and it would spread to SonarJS through the exported JSON. To fix this, add a package-private overload isKnownIdentifier(String url, Map<String, Pattern> inventory) that the public method calls, and bring back the semantic tests against a test-only map such as example.invalid -> ^/schema.

Make the matcher testable independently of the (currently empty) production inventory.

public static boolean isKnownIdentifier(String url) {
  return isKnownIdentifier(url, IDENTIFIER_URLS);
}

static boolean isKnownIdentifier(String url, Map<String, Pattern> inventory) {
  var matcher = IDENTIFIER_URL.matcher(url.strip());
  if (!matcher.matches()) {
    return false;
  }
  var pattern = inventory.get(matcher.group(1).toLowerCase(Locale.ROOT));
  var remainder = matcher.group(2);
  var suffix = remainder == null ? "/" : remainder;
  if (!suffix.startsWith("/")) {
    suffix = "/" + suffix;
  }
  return pattern != null && pattern.matcher(suffix).find();
}

✅ 2 closed

✅ Quality: npm README omits Java matcher's whitespace/backslash rejection rules

📄 sonar-analyzer-commons-configurations/src/main/packaging/npm/README.md:29-36 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:283-284 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:295-307
The README says consumers "must follow the Java matcher's semantics" and then lists those rules. It leaves out two things IDENTIFIER_URL (^http://([^\s/?#\]+)([/?#][^\s\]*)?$) actually does:

1. It rejects any URL that contains whitespace or a backslash anywhere, not only at the ends.
2. It defines the authority as the text up to the first /, ? or #.

So a JS consumer that follows the README treats http://www.w3.org/2000/svg extra as an identifier: it strips, sees authority www.w3.org, and ^/ matches the suffix. CleartextProtocolFilter.isKnownIdentifier returns false for the same string. The two analyzers then disagree, which is the drift this package is meant to prevent.

Fix: add both rules to the README's list of required semantics.

✅ Cross-PR: Whitespace-padded identifier URLs match in Java but not in SonarJS

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:295-297 📄 sonar-analyzer-commons-configurations/src/main/packaging/npm/README.md:29-31
Breaks when this PR and SonarSource/SonarJS#8279 both ship as implementations of the shared identifier matcher, and SonarJS gets a URL value with leading or trailing whitespace, such as the literal " http://www.w3.org/2000/svg ".

How

1. CleartextProtocolFilter.isKnownIdentifier calls url.strip() before matching (CleartextProtocolFilter.java:296), so the padded value is recognized as an identifier.
2. The npm README makes "Strip surrounding whitespace" a required consumer rule (README.md:31).
3. SonarJS isKnownIdentifierUrl runs /^http:\/\/([^\s/?#\]+)([/?#][^\s\]*)?$/i.exec(value) on the input as-is (identifier-urls.ts:74). The ^http:// anchor fails on a leading space, so it returns false.
4. The same string is suppressed by the JVM analyzers but reported by SonarJS. Avoiding that drift is the reason the inventory is shared.

Fix

• This PR: nothing needed. The Java behavior matches the documented contract.
• JS-2774 S5332: Add the identifier URL inventory and matcher SonarJS#8279: run the regex on value.trim() and add a test with a whitespace-padded identifier URL.

Compared b8a7f05 with SonarSource/SonarJS#8279 at 03f01fd

🤖 Prompt for agents

Code Review: Adds `CleartextProtocolFilter.isKnownIdentifier` and `exportIdentifierUrls` plus an `IdentifierUrlsExporter` that writes `identifier-urls.json` during `prepare-package`, with matching rules documented in the npm README. Approved with suggestions: the identifier matching logic is currently dead code because the inventory is empty, and its semantics (authority/port, suffix normalisation, case-sensitive paths) lost their tests in this change. Consider adding a package-private overload that accepts an inventory map and restoring those tests against a test-only entry such as `example.invalid -> ^/schema`. Resolved: the README now covers the whitespace and backslash rejection rules, and whitespace-padded URLs match consistently across Java and SonarJS.

1. 💡 Quality: Identifier matching semantics are now dead code with no tests
   Files: commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:228, commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:238-252, commons/src/test/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilterTest.java:30-34, sonar-analyzer-commons-configurations/src/main/packaging/npm/README.md:29-39

   With `IDENTIFIER_URLS = Map.of()`, the lookup at line 246 always returns null, so the suffix normalisation and case-sensitive `find()` at lines 247-252 never run. This delta also deleted the tests that covered them: exact authority with port, the empty-suffix `/`, the `/` prefix for `?`/`#`, and case-sensitive paths. The only test left checks one negative lookup. The javadoc (lines 238-239) and the npm README (lines 29-39) still promise these semantics to non-JVM consumers, but nothing checks them. A regression would only show up once the inventory is filled in, and it would spread to SonarJS through the exported JSON. To fix this, add a package-private overload `isKnownIdentifier(String url, Map<String, Pattern> inventory)` that the public method calls, and bring back the semantic tests against a test-only map such as `example.invalid -> ^/schema`.

   Fix (Make the matcher testable independently of the (currently empty) production inventory.):
   public static boolean isKnownIdentifier(String url) {
     return isKnownIdentifier(url, IDENTIFIER_URLS);
   }
   
   static boolean isKnownIdentifier(String url, Map<String, Pattern> inventory) {
     var matcher = IDENTIFIER_URL.matcher(url.strip());
     if (!matcher.matches()) {
       return false;
     }
     var pattern = inventory.get(matcher.group(1).toLowerCase(Locale.ROOT));
     var remainder = matcher.group(2);
     var suffix = remainder == null ? "/" : remainder;
     if (!suffix.startsWith("/")) {
       suffix = "/" + suffix;
     }
     return pattern != null && pattern.matcher(suffix).find();
   }

Verification Rules 1/1 met Knowledge ✅ 1 Repos 2 inspected · PRs 1 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

Knowledge ✅ 1 expectation met

SonarJS trims identifier URLs before helper calls expectation met

Broader impact Repos 2 inspected PRs 1 inspected

Cross-repo 2 repositories inspected 0 open impacts

SonarSource/SonarJS Inspected
SonarSource/sonar-xml Inspected

Cross-PR 1 PR inspected 0 open impacts

PR 8279: JS-2774 S5332: Add the identifier URL inventory and matcher Inspected

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}