Skip to content

Quote non-token characters in matchFormatPattern - #141

Merged
kylekatarnls merged 3 commits into
CarbonPHP:masterfrom
iliaal:fix/quote-match-format-pattern
Sep 6, 2026
Merged

kylekatarnls merged 3 commits into
CarbonPHP:masterfrom
iliaal:fix/quote-match-format-pattern

Conversation

@iliaal

@iliaal iliaal commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

hasFormatWithModifiers() interpolated the format string into the final PCRE pattern without quoting it. Only format letters and modifiers were replaced; every other byte (parentheses, quantifiers, alternation, anchors) kept its regex meaning, so a caller-supplied format could alter the structure of the validation pattern rather than match literally:

Carbon::hasFormatWithModifiers('kkkk', 'k{4}'); // true before this PR: '{4}' acted as a quantifier
Carbon::hasFormatWithModifiers('qqqq', 'q{4}'); // true before this PR
Carbon::hasFormat('qqqq', 'q{4}');              // false: the sibling API preg_quotes its input

The inconsistency with hasFormat() also meant the two validators disagreed on identical inputs.

How it is fixed

matchFormatPattern() now wraps each token-generated regex fragment between null-byte sentinels during the substitution pass, then quotes every remaining byte of the pattern. Backslash escapes in the format (\T, \.) still resolve to their literal target, and token fragments (d, #, *, ?, !, |, +) keep their regex meaning. Raw null bytes in the format are stripped before processing so they cannot collide with the sentinels.

Behavior change to be aware of: a format containing unescaped regex metacharacters that used to be interpreted as pattern syntax (e.g. .*, (a|b)) will now only match those characters literally. This matches what hasFormat() already did and what createFromFormat() escape semantics imply.

@kylekatarnls kylekatarnls added this to the 3.13.3 milestone Aug 30, 2026
@kylekatarnls kylekatarnls added the need tests Unit tests will need to be added to merge this code change label Aug 30, 2026
Comment thread src/Carbon/Factory.php Outdated
Comment thread src/Carbon/Factory.php Outdated
@kylekatarnls

Copy link
Copy Markdown
Contributor

I confirm that it was not initially intended that {4} would be read as a multiplier.

The "modifier" in this context refers to the non-date characters listed in https://www.php.net/manual/en/datetimeimmutable.createfromformat.php#datetimeimmutable.createfromformat.parameters

However I suspect some might have misinterpreted this as a feature, may have think that {number} is one of the supported modifiers.

I'll add a warning in the release since it can be a breaking change for a minority of users.

I still proceed as I think the number of use cases where a format would contain multiple in a row the same unit must be quite small.

iliaal and others added 3 commits September 6, 2026 17:01
hasFormatWithModifiers() interpolated the format string into the final
PCRE pattern without quoting it: parentheses, quantifiers, alternation
and anchors coming from the format had a regex meaning instead of
matching literally. hasFormat() quoted its input while its sibling did
not, so the two validators disagreed on identical inputs.

Wrap token-generated fragments between null-byte sentinels during the
substitution pass, then quote every remaining byte of the pattern,
resolving backslash escapes to their literal target first.
Concatenate the leading-escapes capture instead of interpolating it, and
drop the by-reference foreach: the loop already writes through
$chunks[$index], so the reference and its trailing unset() were both
unused.
@kylekatarnls
kylekatarnls force-pushed the fix/quote-match-format-pattern branch from 825ce2a to 15bd6b6 Compare September 6, 2026 15:01
@kylekatarnls kylekatarnls removed the need tests Unit tests will need to be added to merge this code change label Sep 6, 2026
@kylekatarnls
kylekatarnls merged commit 374f68d into CarbonPHP:master Sep 6, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants