Treat prefix literally in UrlTemplate.match(url, prefix)

[vasiliy-mikhailov]

Problem

UrlTemplate.match(url, prefix) strips the prefix with:

matchAgainst = matchAgainst.replaceFirst(prefix, "");

String.replaceFirst treats its first argument as a regular expression, so a prefix containing regex metacharacters is mis-handled. For example a prefix with an unmatched ( throws PatternSyntaxException; other metacharacters can silently strip the wrong text.

Fix

Quote the prefix with Pattern.quote(prefix) so it is matched literally.

Test

Adds shouldNotThrowWhenPrefixContainsRegexMetacharacters to UrlTemplateTest: matching with a prefix containing ( throws PatternSyntaxException on trunk and succeeds with this change.

[CLAassistant]

All committers have signed the CLA.

[qodo-code-review]

PR Summary by Qodo

Fix UrlTemplate.match prefix stripping to treat prefix literally
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Description

• Quote prefix with Pattern.quote before replaceFirst to avoid regex parsing.
• Prevent PatternSyntaxException and incorrect stripping when prefixes contain regex
 metacharacters.
• Add regression test covering a prefix with an unmatched ( character.

Diagram

flowchart TD
A["HTTP routing"] --> B["Template match"] --> C["Quote prefix"] --> D["Replace prefix"] --> E["Match target"]; T["Template test"] --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Avoid regex entirely (startsWith + substring)

• ➕ Eliminates regex overhead and any regex-related edge cases
• ➕ More explicit intent: remove a literal prefix only when present
• ➖ Requires additional conditional logic (e.g., only strip when matchAgainst starts with prefix)
• ➖ May subtly change behavior if the prior regex-based removal relied on regex semantics (unlikely, but possible)

Recommendation: The current approach (quoting via Pattern.quote(prefix)) is the best minimal fix: it preserves the existing replaceFirst behavior while making it safe for literal prefixes containing regex metacharacters. Consider the non-regex startsWith/substring alternative only if you want to remove regex usage entirely or optimize hot-path performance.

Files changed (2) +12 / -1

Bug fix (1) +1 / -1

UrlTemplate.javaQuote prefix before replaceFirst to prevent regex interpretation +1/-1

Quote prefix before replaceFirst to prevent regex interpretation

• Changes prefix stripping to use 'replaceFirst(Pattern.quote(prefix), "")' so the prefix is treated as a literal string. Prevents 'PatternSyntaxException' and incorrect matches when prefixes contain regex metacharacters.

java/src/org/openqa/selenium/remote/http/UrlTemplate.java

Tests (1) +11 / -0

UrlTemplateTest.javaAdd regression test for prefixes containing regex metacharacters +11/-0

Add regression test for prefixes containing regex metacharacters

• Adds a test that matches a URL with a prefix containing an unmatched '(' and asserts matching succeeds and parameters are extracted correctly. This guards against regressions where the prefix is accidentally parsed as a regex.

java/test/org/openqa/selenium/remote/http/UrlTemplateTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. match(matchAgainst, prefix) Javadoc incomplete ✓ Resolved 📘 Rule violation ✧ Quality

Description

The modified public method match(@Nullable String matchAgainst, @Nullable String prefix) lacks a
complete Javadoc block (no purpose sentence and no @param tags for matchAgainst and prefix).
This violates the requirement that all changed public API methods have complete Javadoc, reducing
API clarity and documentation quality.

Code

java/src/org/openqa/selenium/remote/http/UrlTemplate.java[152]

+      matchAgainst = matchAgainst.replaceFirst(Pattern.quote(prefix), "");

Evidence

PR Compliance ID 330201 requires complete Javadoc (purpose sentence, @param for each parameter,
and @return when non-void) for each changed public method. The changed method `match(@Nullable
String matchAgainst, @Nullable String prefix) has only an @return` line and omits the required
description and @param tags.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/remote/http/UrlTemplate.java[143-155]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The public method `match(@Nullable String matchAgainst, @Nullable String prefix)` was modified, but its Javadoc is incomplete: it lacks a free-text purpose description and is missing `@param` tags for both parameters.

## Issue Context
Compliance requires that any changed public API method in non-test code has complete Javadoc including a descriptive sentence and correct tags.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/http/UrlTemplate.java[143-155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Prefix stripping contract mismatch 🐞 Bug ≡ Correctness

Description

The new Javadoc says prefix is stripped from the start of matchAgainst, but the implementation
uses replaceFirst(Pattern.quote(prefix), ""), which will strip the first occurrence even when it
is not at the start. For example, matchAgainst="/x/prefix/y" with prefix="/prefix" becomes
"/x/y" even though the prefix was not leading.

Code

java/src/org/openqa/selenium/remote/http/UrlTemplate.java[R144-147]

+   * Matches a URL against this template after stripping a leading prefix.
+   *
+   * <p>The {@code prefix} is treated literally (not as a regex) and stripped from the start of
+   * {@code matchAgainst} before matching, unless it is empty or {@code "/"}.

Evidence

The Javadoc explicitly claims the prefix is stripped "from the start" of matchAgainst, but the
code performs an unanchored replaceFirst(...), which can remove the prefix even when it is not at
the beginning of the string.

java/src/org/openqa/selenium/remote/http/UrlTemplate.java[143-160]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The Javadoc for `UrlTemplate.match(matchAgainst, prefix)` states that the method strips a *leading* prefix (from the start of `matchAgainst`), but the current implementation removes the first occurrence of the prefix anywhere in the string.

### Issue Context
This is a public API contract mismatch: callers reading the new Javadoc may rely on start-anchored stripping semantics, while the implementation will also strip non-leading occurrences.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/http/UrlTemplate.java[143-160]

### Suggested fix
Pick one of the following and make Javadoc + behavior consistent:
- **If the intended behavior is “leading prefix only”**: change stripping to be start-anchored, e.g.:
 - `if (matchAgainst.startsWith(prefix)) { matchAgainst = matchAgainst.substring(prefix.length()); }`
 - or `matchAgainst = matchAgainst.replaceFirst("^" + Pattern.quote(prefix), "");`
- **If the intended behavior is “first occurrence anywhere”**: update the Javadoc wording to remove “leading”/“from the start” language and describe the actual behavior precisely (including the null/empty/"/" cases).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit db683a8

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0f0b54e

[diemol]

Thank you, @vasiliy-mikhailov!