Skip to content

Fix doubly nested parentheses in selectors throwing a recursion error - #122

Merged
jakejackson1 merged 2 commits into
mainfrom
fix/issue-120
Oct 26, 2026
Merged

jakejackson1 merged 2 commits into
mainfrom
fix/issue-120

Conversation

@gravitypdf-bot

Copy link
Copy Markdown
Contributor

A selector with two levels of nested parentheses threw from the scanner:

qp($xml)->find('li:not(:not(:nth-child(1)))');
// before: QueryPath\Exception: Recursion error detected at iteration 29.
// after:  the first <li> of each list

Cause

Scanner::getPseudoClassString() reads a nested (...) with a recursive call, and that call already consumes the closing ). The outer loop then called nextToken() again and skipped one more character. With one level of nesting, the skipped character was the outer ). That only went unnoticed when the selector ended there: li:not(:nth-child(1)) b parsed as li:not(":nth-child(1) b"). With two levels, the outer argument never saw its ) and the scanner read past the end. find() and remove() share this scanner, so both engines threw.

Change

  • Skip the extra nextToken() after a nested argument.
  • Read quoted strings inside a parenthesised argument whole too, so :contains("a)b") no longer ends at the quoted paren, and :contains("a(b)") keeps its closing quote.

The change is confined to src/CSS/Scanner.php. The parser and both engines are untouched.

Tests

  • ParserTest: the value passed to pseudoClass() for every selector in the issue's table, plus three levels of nesting, quoted and unquoted arguments containing parens, and a selector that continues after a nested argument (li:not(:nth-child(1)) b).
  • DOMQueryTest: find() and remove() results for the same selectors.

24 of the new cases fail on main.

Behaviour change

Selectors that continue after a nested argument (li:not(:nth-child(1)) b, p:not(:has(a)).x) now apply the trailing part as a separate selector step. Before, it was folded into the pseudo-class argument.

Fixes #120.

🤖 Generated with Claude Code

getPseudoClassString() advanced past the token after every nested
parenthesised argument, so with two levels of nesting the outer argument
never saw its closing paren and the scanner ran off the end of the input
(issue #120). Quoted strings inside an argument are now read whole too, so
a paren inside them no longer ends the argument.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.98%. Comparing base (6ae6acf) to head (62f647d).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #122      +/-   ##
============================================
+ Coverage     90.69%   90.98%   +0.28%     
- Complexity     1424     1429       +5     
============================================
  Files            25       25              
  Lines          3170     3171       +1     
============================================
+ Hits           2875     2885      +10     
+ Misses          295      286       -9     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jakejackson1
jakejackson1 merged commit 791f70e into main Oct 26, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-120 branch October 26, 2026 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doubly nested parentheses in a selector throw "Recursion error detected" (e.g. li:not(:not(:nth-child(1))))

2 participants