Fix #20456: Handle bracket style delimiters in Html::escapeJsRegularExpression()

[veksa]

Q	A
Is bugfix?	✔️
New feature?	❌
Breaks BC?	❌
Fixed issues	#20322, #20456

Two separate defects in escapeJsRegularExpression(). They're in adjacent lines of the same short method, so I put them in one PR rather than have the second one conflict with the first.

Hex escapes (#20322)

A JavaScript \u escape takes exactly four hex digits. The replacement didn't pad, so \xFF came out as \uFF, which JS reads as \u00 followed by a literal F. Two more problems in the same line: [0-9a-fA-F]+ is greedy while PCRE reads at most two digits without braces, so \x41abc swallowed the abc; and code points above the BMP need the \u{...} form, which the old code never produced.

pattern	before	after
/^[\x00-\xFF]{8,72}$/	/^[\u00-\uFF]{8,72}$/	/^[\u0000-\u00FF]{8,72}$/
/^[\x{A1}-\x{FE}]{2}$/u	/^[\uA1-\uFE]{2}$/u	/^[\u00A1-\u00FE]{2}$/u
/\x41abc/	/\u41abc/	/\u0041abc/
/[\x{1F600}-\x{1F64F}]/u	/[\u1F600-\u1F64F]/u	/[\u{1F600}-\u{1F64F}]/u

The \u{...} form needs the u flag, which the existing modifier filter already keeps. The second row is the case raised in #20330.

Bracket style delimiters (#20456)

The end of the pattern is found by searching for the last occurrence of the delimiter. That's fine for /, # or ~, but PHP also allows the pairs (), {}, [] and <>, which are closed by the matching bracket. So the search lands in the middle of the pattern and the rest is cut off:

pattern	before	after
{^\d{3}$}	/^\d/	/^\d{3}$/
(^(\d+)(\.\d+)?$)	/^(\d+)/	/^(\d+)(\.\d+)?$/
<^[a-z]+$>i	/^[a-z]+$>/i	/^[a-z]+$/i
[^[a-z]+$]	/^/	/^[a-z]+$/

Either way the result is a regex that no longer means what the server-side rule means, so client-side validation quietly accepts values the server rejects.

One note on #20456

The report gives ([a-z_])([a-z0-9_])* as input and expects /([a-z_])([a-z0-9_])*/. That input isn't a valid PCRE pattern: with ( as the delimiter the matching ) closes it right after [a-z_], and the rest is parsed as modifiers, so preg_match() errors out. Producing the expected output would mean treating a delimiter-less string as the pattern body, which is not what any caller passes in and would change the existing ('([a-z0-9-]+)') assertion. Happy to look at that separately if you want the helper to accept those too.

I also added a sentence to the PHPDoc saying the argument is expected to be a valid PCRE pattern, since that was never written down.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 22f3797d-3910-4dd2-aa28-c867f0f07798

📥 Commits

Reviewing files that changed from the base of the PR and between 66f00d1 and 176accc.

📒 Files selected for processing (3)

• framework/CHANGELOG.md
• framework/helpers/BaseHtml.php
• tests/framework/helpers/HtmlTest.php

📜 Recent review details

🧰 Additional context used

🧠 Learnings (7)

📚 Learning: 2026-04-21T21:24:32.138Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20829
File: framework/db/mssql/QueryBuilder.php:678-683
Timestamp: 2026-04-21T21:24:32.138Z
Learning: In yiisoft/yii2, follow the existing PHPStan configuration: `empty()` is explicitly prohibited. When reviewing PHP code, do not recommend replacing strict checks like `$var === null || $var === []` (or similar null/empty-array logic) with `empty($var)`, since that would conflict with the codebase’s `empty()` policy.

Applied to files:

• framework/helpers/BaseHtml.php
• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-06-05T13:48:39.340Z

Learnt from: WarLikeLaux
Repo: yiisoft/yii2 PR: 20774
File: tests/framework/filters/VerbFilterTest.php:107-110
Timestamp: 2026-06-05T13:48:39.340Z
Learning: This codebase targets PHP 7.4. Do not suggest PHP 8+ non-capturing catch syntax (e.g., `catch (SomeException) { ... }` without a variable). If an exception is intentionally suppressed, use a typed catch with a (possibly unused) variable, e.g. `catch (SomeException $e) { }`, so the code remains compatible with PHP 7.4.

Applied to files:

• framework/helpers/BaseHtml.php
• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-06-07T20:07:07.417Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20930
File: tests/framework/db/oci/SchemaConstraintsTest.php:70-73
Timestamp: 2026-06-07T20:07:07.417Z
Learning: When reviewing code intended for the yiisoft/yii2 branch 22.0 (PHP 8.3+), treat PHP 8.0+ non-capturing catch clauses as valid. Do not flag code like `catch (SomeException) { ... }` (no exception variable) as incompatible with the target PHP version. Only apply incompatibility warnings for older target branches/versions that don’t support this syntax.

Applied to files:

• framework/helpers/BaseHtml.php
• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-07-06T14:24:20.463Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20982
File: framework/db/mysql/QueryBuilder.php:80-84
Timestamp: 2026-07-06T14:24:20.463Z
Learning: For yiisoft/yii2 targeting branch 22.0 (PHP requirement: 8.3+), do not flag PHP 8.1+ syntax as incompatible when reviewing code. In particular, allow constructs introduced in PHP 8.1/8.2 such as string-key array unpacking (e.g., `...parent::someMethod()`) and non-capturing catch clauses. Only emit PHP compatibility/incompatibility warnings when the target PHP version/branch is older than the feature’s introduction (e.g., PHP 7.4), where these syntaxes would not be supported.

Applied to files:

• framework/helpers/BaseHtml.php
• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-04-11T18:58:31.942Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20817
File: tests/framework/grid/CheckboxColumnTest.php:45-49
Timestamp: 2026-04-11T18:58:31.942Z
Learning: In the yiisoft/yii2 test suite (e.g., under tests/**), PHPMD `UnusedFormalParameter` warnings on test helper methods that are intentionally no-ops (such as trait hook methods like `triggerClientScriptDispatch`) should not be suppressed with annotations. Treat these as acceptable static-analysis nitpicks in test code and only suppress/adjust when the parameter is genuinely not required due to real dead code concerns.

Applied to files:

• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-04-14T10:43:29.381Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20820
File: tests/framework/console/ControllerTest.php:324-328
Timestamp: 2026-04-14T10:43:29.381Z
Learning: In yiisoft/yii2 PHP tests that assert union-type stringification, remember that `ReflectionUnionType::getTypes()` does not preserve the source declaration order. When building the expected string, follow PHP’s canonical ordering: built-in scalar types are returned in the fixed order `static, callable, array, string, int, float, bool, null`, while class/interface types come first in their declaration order. For example, a parameter declared as `int|string` should stringify to `string|int` (not `int|string`). Ensure test assertions match this canonical ordering.

Applied to files:

• tests/framework/helpers/HtmlTest.php

📚 Learning: 2026-05-01T02:09:33.591Z

Learnt from: terabytesoftw
Repo: yiisoft/yii2 PR: 20842
File: tests/framework/base/ModuleTest.php:481-519
Timestamp: 2026-05-01T02:09:33.591Z
Learning: In the yiisoft/yii2 test suite, remember that `Yii::setAlias()` stores the alias value as an exact string and `Yii::getAlias()` returns that same exact string with no path-separator normalization. As a result, tests that verify alias round-trips through `yii\base\Module::setViewPath()/getViewPath()` and `Module::setLayoutPath()/getLayoutPath()` should assert strict string equality on the stored vs. retrieved values, and do not apply platform-specific directory separator normalization.

Applied to files:

• tests/framework/helpers/HtmlTest.php

🪛 PHPMD (2.15.0)

tests/framework/helpers/HtmlTest.php

[error] 2150-2150: Avoid using static access to class '\yii\helpers\Html' in method 'testEscapeJsRegularExpressionBracketStyleDelimiters'. (undefined)

(StaticAccess)

---

[error] 2173-2173: Avoid using static access to class '\yii\helpers\Html' in method 'testEscapeJsRegularExpressionHexEscapes'. (undefined)

(StaticAccess)

🔇 Additional comments (3)

framework/helpers/BaseHtml.php (1)

2390-2413: LGTM!

framework/CHANGELOG.md (1)

39-40: LGTM!

tests/framework/helpers/HtmlTest.php (1)

2140-2187: LGTM!

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Fixed JavaScript regular-expression escaping for PCRE patterns using bracket-style delimiters.
  • Preserved complete patterns, including nested brackets, modifiers, and slash characters.
  • Improved hexadecimal escape conversion, including four-digit escapes and characters beyond the Basic Multilingual Plane.

• Documentation

  • Updated the changelog and inline guidance to clarify supported regular-expression formats and delimiters.

Walkthrough

Html::escapeJsRegularExpression() now handles bracket-style PCRE delimiters and extended hexadecimal escapes. Data-driven tests cover delimiter matching, nested brackets, modifiers, slash characters, and Unicode code points. The changelog records the fixes for Yii Framework 2.0.56.

Changes

Regular Expression Escaping

Layer / File(s)	Summary
Delimiter and hexadecimal escape handling framework/helpers/BaseHtml.php, framework/CHANGELOG.md	escapeJsRegularExpression() now finds matching closing delimiters for bracket-style patterns and converts braced and unbraced hexadecimal PCRE escapes to JavaScript-compatible Unicode escapes, including code points above the BMP.
Escaping regression tests tests/framework/helpers/HtmlTest.php	Data-driven tests cover bracket delimiters, nested opening brackets, modifiers, slash-containing patterns, padded hexadecimal escapes, literal suffixes, and supplementary-plane escapes.

Suggested labels: type:bug, status:code review

Poem

A rabbit checks each bracketed line,
And pads escapes four digits fine.
Unicode hops beyond the BMP,
While tests keep every pattern free.
The changelog marks the fix in time.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change to handle bracket-style delimiters in Html::escapeJsRegularExpression().
Description check	✅ Passed	The description accurately explains both bug fixes, affected behavior, compatibility impact, tests, and the valid-pattern limitation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.97%. Comparing base (66f00d1) to head (176accc).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
framework/helpers/BaseHtml.php	0.00%	9 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (66f00d1) and HEAD (176accc). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (66f00d1)	HEAD (176accc)
	32	31

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #21052       +/-   ##
=============================================
- Coverage     80.69%   30.97%   -49.72%     
- Complexity    11552    11554        +2     
=============================================
  Files           374      374               
  Lines         30280    30279        -1     
=============================================
- Hits          24435     9380    -15055     
- Misses         5845    20899    +15054     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.