Skip to content

[TASK] Remove thecodingmachine/safe dependency (part 1) - #1612

Merged
oliverklee merged 2 commits into
MyIntervals:mainfrom
SjorsO:drop-dependency-part-1
Aug 14, 2026
Merged

[TASK] Remove thecodingmachine/safe dependency (part 1)#1612
oliverklee merged 2 commits into
MyIntervals:mainfrom
SjorsO:drop-dependency-part-1

Conversation

@SjorsO

@SjorsO SjorsO commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

As discussed here: #1484 (comment)

preg_match can return false if:

  • The pattern is invalid
  • the u flag is used and the subject or pattern aren't valid UTF-8
  • Backtrack/recursion limits are hit

This preg_match can't fail because:

  • The pattern is always valid ($match is always a fixed value)
  • There is no u flag
  • There is no backtracking or recursion

Comment thread src/CSSList/CSSList.php Outdated
Co-authored-by: Oliver Klee <github@oliverklee.de>
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 72.604% (+0.03%) from 72.579% — SjorsO:drop-dependency-part-1 into MyIntervals:main

@oliverklee oliverklee changed the title Remove thecodingmachine/safe dependency (part 1) [TASK] Remove thecodingmachine/safe dependency (part 1) Aug 14, 2026
@oliverklee
oliverklee merged commit b0d6ba9 into MyIntervals:main Aug 14, 2026
24 checks passed
@JakeQZ

JakeQZ commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

This preg_match can't fail because:

  • There is no backtracking or recursion

Actually there is backtracking, or at least the storage of backtracking information. The engine is simple and does not know what it might be matching next. So when matching the optional vendor-prefix, it needs to be able to rewind in case the following part will match without it but not with. We can see that's not possible, but it can't. So it will build up a rewind stack. If presented with something like -veryverylongvendorprefix-translate (shortened for readability), it could bail.

Use of the possessive quantifier will mitigate this. I think that's beyond the scope of these PRs, so have added #1614.

The library as it is should bail out somehow reasonably in such unrealistic situations, as well as it did before the introduction of 'Safe', and at least there is now an assert().

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.

4 participants