🐛 Fixed filters losing values that contain ^ or $ - #213
Conversation
`^` and `$` are both characters a value can contain and the way a regex
writes its anchors. A value carrying one arrives escaped — `5\$`, `\^caret`
— but `processRegExp` removed the escaping before it looked for the anchors,
so by the time it tested `startsWith('^')` and `endsWith('$')` a literal
character and an anchor were indistinguishable.
That corrupted the value and the operator together. `contains '5$'` compiled
to `LIKE '%5'`, which is `endsWith '5'`: the dollar was dropped and the
question changed. `contains '^caret'` compiled to `LIKE 'caret%'` the same
way. Both still returned rows, just the wrong ones, so nothing surfaced the
mistake — a saved Ghost segment silently changed who it was about.
Reading the anchors off the escaped source fixes both. A trailing `$` is an
anchor only when an even number of backslashes precedes it, so a value
ending in a backslash is still told apart from an escaped dollar.
A pattern anchored at both ends was also mishandled: the old branch was an
`else if`, so `/^exact$/` took the start-anchor path and left the `$` in the
pattern. It is now an exact match with no wildcard on either end.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. WalkthroughThe regex-to-SQL conversion now detects start and end anchors before unescaping characters. It distinguishes literal escaped Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change preserves literal ^ and $ characters in filters and correctly handles matches anchored at both ends. It is merge-ready after normal checks and review, with no actionable merge-blocking risk remaining. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
==========================================
+ Coverage 85.31% 85.52% +0.20%
==========================================
Files 9 9
Lines 2261 2293 +32
Branches 479 484 +5
==========================================
+ Hits 1929 1961 +32
Misses 326 326
Partials 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ref https://linear.app/ghost/issue/BER-3868 A member's name or email can contain a dollar sign, and a publisher can reasonably filter on one. But `$` is also how a regex marks the end of a pattern, and the admin filter engine used the same character for both. Reading a saved filter back, the engine asked twice whether the pattern ended in a `$` — once to work out which operator had been used, and again to strip the anchor off the value. Neither check considered that the character might have been escaped because it was part of the value, and neither knew what the other had concluded. So a filter for names containing "5$" came back as a filter for names ending in "5", with the value itself left mangled as "5\". Starts-with came back as contains. This hit contains, does-not-contain, starts-with and does-not-start-with on member name and email, and on comment filters. The anchors are now read once, in a single decomposition that also produces the value, so the operator and the value can no longer disagree about what the pattern held. That check is escape-aware: a trailing `$` anchors the pattern only when it is not escaped. Round-trip coverage was missing, so it is added across every codec and operator. A browser test covers the step above them, since the reload is where the filter stops being state the page holds and becomes NQL that has to be parsed again — the same path a saved view takes, which is how the misreading became permanent rather than cosmetic. The query layer has a related but independent fault, where the same characters are unescaped before the anchors are read, so the server answers the wrong question even for a correctly written filter. That is fixed in TryGhost/NQL#213 and reaches Ghost through a dependency bump.
ref https://linear.app/ghost/issue/BER-3868 A member's name or email can contain a dollar sign, and a publisher can reasonably filter on one. But `$` is also how a regex marks the end of a pattern, and the admin filter engine used the same character for both. Reading a saved filter back, the engine asked twice whether the pattern ended in a `$` — once to work out which operator had been used, and again to strip the anchor off the value. Neither check considered that the character might have been escaped because it was part of the value, and neither knew what the other had concluded. So a filter for names containing "5$" came back as a filter for names ending in "5", with the value itself left mangled as "5\". Starts-with came back as contains. This hit contains, does-not-contain, starts-with and does-not-start-with on member name and email, and on comment filters. The anchors are now read once, in a single decomposition that also produces the value, so the operator and the value can no longer disagree about what the pattern held. That check is escape-aware: a trailing `$` anchors the pattern only when it is not escaped. Round-trip coverage was missing, so it is added across every codec and operator. A browser test covers the step above them, since the reload is where the filter stops being state the page holds and becomes NQL that has to be parsed again — the same path a saved view takes, which is how the misreading became permanent rather than cosmetic. The query layer has a related but independent fault, where the same characters are unescaped before the anchors are read, so the server answers the wrong question even for a correctly written filter. That is fixed in TryGhost/NQL#213 and reaches Ghost through a dependency bump.
Problem
A filter value can contain the very characters a regex uses for its anchors. Searching for a price that ends in a dollar, or a title that starts with a caret, are ordinary things to ask for.
Those characters arrive escaped, to mark them as part of the value rather than as anchors. But the conversion to a LIKE pattern removed that escaping before it looked for the anchors, so by the time it checked, a literal character and an anchor were the same character.
The result changed both the value and the question. Asking for names containing
5$was answered as if it asked for names ending in5: the dollar was dropped and the operator silently became a different one. Asking for names containing^caretwas answered as if it asked for names starting withcaret.Nothing surfaced this. Both filters still returned rows, just the wrong ones, so a saved Ghost segment could quietly change who it was about without anyone seeing an error.
A pattern anchored at both ends was mishandled too. Only one anchor was ever considered, so anchoring both ends left the trailing character sitting in the pattern as a literal and matched almost nothing.
Solution
Read the anchors before removing the escaping, rather than after. At that point an escaped character is still visibly escaped, so it can be kept as part of the value while a genuine anchor is recognised as one.
A trailing anchor character counts as an anchor only when an even number of backslashes precedes it, so a value that itself ends in a backslash is still told apart from an escaped dollar.
Both anchors are now handled independently, which makes anchoring both ends an exact match with no wildcard on either side.
Six tests cover the cases. Three of them fail without the change: the two values holding a literal anchor character, and the both-ends match. The rest guard the boundaries the parity counting exists for.
Verified against Ghost with the package linked locally: its members filter suite and full integration suite pass, and the two filters above now select the members they name. The behaviour only changes for filters that were already wrong, so there is nothing for a saved filter to have depended on.