Repository navigation
Fix slow regex on strings with many unclosed <, {{, %{ or & - #97
Merged
Merged
Conversation
sjahu
approved these changes
Sep 30, 2026
sjahu
left a comment
sjahu
left a comment
Contributor
There was a problem hiding this comment.
lgtm but the comments should probably be removed. The ones that refer to "old" logic or "behaviour change" would need to be rephrased to mean anything outside of this PR, and the comment on the regex definition doesn't seem necessary; future readers can find this PR if they want to better understand the history of the pattern.
aminpaks
approved these changes
Oct 1, 2026
… matching
The token-preserving regex used lazy wildcards (`<.*?>`, `{{.*?}}`,
`%{.*?}`, `&\S*?;`). On a backtracking engine, a string with many
opening delimiters and no closer (e.g. "<" * n) makes each attempt scan
to the end of the input, fail, and retry from the next opener, which is
O(n^2) in the input length (CWE-1333).
Replace each wildcard with a negated character class bounded by the
token's own delimiters, so an unterminated opener fails after one step.
`\n` stays excluded to mirror `.`, so a token still never spans a line.
Behaviour is unchanged for well-formed markup, Liquid tags, templates,
URLs and entities (property-tested: 0 differences over 74,499 random
strings with no nested delimiter). The only observable change is that a
token containing a nested delimiter of its own kind (`<b <i>x</i>`,
`{{ a {b} }}`, `&a&`) is no longer swallowed up to the first closer.
Note that Ruby >= 3.2 already matched the previous pattern in linear
time thanks to regex memoization (Regexp.linear_time? returns true for
it); this change makes the bound hold on every engine the gem supports
(required_ruby_version >= 2.7).
Co-authored-by: Allan Yang <allan.yang@shopify.com>
The rationale lives in the PR and CHANGELOG. Keep one note at the regex so the bounded classes and the `\n` exclusion aren't "simplified" away, and one line per new test saying what it pins. Co-authored-by: Allan Yang <allan.yang@shopify.com>
benchmark stopped being a default gem in Ruby 3.5, so under bundler on Ruby 3.5+ `require 'benchmark'` raises LoadError unless the gem is in the Gemfile. Process.clock_gettime(CLOCK_MONOTONIC) is core Ruby and is what Benchmark.realtime uses internally, so the measurement is unchanged.
Per review: the history of the pattern lives in the PR and CHANGELOG, and the test names already say what each test pins. Co-authored-by: Allan Yang <allan.yang@shopify.com>
A quoted attribute value is skipped as a unit, so <a title="a < b"> and <a title="a > b"> stay intact, as they did before the regex change. Every branch still fails at the next unquoted <, so matching stays linear. Co-authored-by: Allan Yang <allan.yang@shopify.com>
allannyang
force-pushed
the
harden-escaped-regex
branch
from
October 6, 2026 19:57
a8549f5 to
adb5201
Compare
sjahu
pushed a commit
that referenced
this pull request
Oct 7, 2026
… braces Follow-up to #97. Keeps Regexp.linear_time? true. Zero output changes on Shopify Core's English locale files except 19 Liquid email subjects where {% if %} tags are now preserved instead of pseudolocalized.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix slow regex on strings with many unclosed
<,{{,%{or&