Repository navigation
fix(parser): read a backquoted substitution after a quoted segment - #2661
Conversation
`'b='`cmd`` and `"b="`cmd`` lexed the backquote as a literal word character, so the substitution swallowed the script up to the next backquote: `echo 'b='`echo X`` failed with "unterminated backtick substitution". A quoted segment followed by `$(cmd)` already worked; the backquoted form is now read the same way.
chaliy
left a comment
There was a problem hiding this comment.
Approved for shipment at 1ec7edcd38de13b7307d013d507cf8019fda8e26.
The contribution reuses the existing backquote reader. Follow-up fixes preserve quoted glob literals and empty fields, keep unquoted expansion output active, and charge escaped substitution bytes consistently. Quote metadata is checked once per word rather than repeatedly scanning long mixed words.
No introduced sandbox escape, permission bypass, unsafe code, dependency, or host-I/O change found. Tests verify malformed delimiters, runtime-only body errors, analysis visibility, dynamic command detection, dispatch vetoes, and shared execution budgets.
All 48 current checks pass, including required Check and Rust CodeQL; no new CodeQL alerts. Local validation includes 3,737 Bashkit library tests, 188 parser tests, 21 analysis tests, nine focused regressions with 18 Bash comparisons, seven arithmetic-budget tests, 19 fail-point tests, and 96 benchmark cases with zero errors and 100% Bash output match.
The full local macOS gates retain two upstream failures: nested-substitution stack overflow and CPython static alignment. Both reproduce on clean main and are documented in the PR body. GitHub CI and merge status are green/clean.
chaliy
left a comment
There was a problem hiding this comment.
Reviewed the conflict resolution against main c6fd211. The pending-array allocation guards, capacity leases, consumed-substitution lease release, and boxed recursive future are preserved alongside per-part quote escaping and matching byte charges. No security or design-alignment blockers found in this resolution.
All 42 current CI checks pass, including required Check and security suites. Local quoting tests include 21 Bash comparisons and compound-array cases; all 25 execution-budget regressions pass. CLI smoke and 96 benchmark cases match Bash.
Limitations recorded in the PR body: the local macOS stack-overflow gate reproduces on clean current main; no fresh combined CodeQL scan started for this fork revision (previous PR head and current main scans passed). Approve for shipment.
What changed
Backquoted substitutions immediately after single- or double-quoted text now form one word and execute correctly. Their unquoted output still splits and globs, matching Bash.
Words beginning with quoted text also preserve literal glob characters when an adjacent substitution or variable expands to nothing. Quoted expansion output stays literal; unquoted expansion output remains active in patterns. Empty quoted fields and escaped internal marker bytes retain their meaning.
Why
The quote-continuation reader treated an opening backquote as ordinary text. Reuse the existing backquote reader so delimiter handling, runtime syntax errors, analysis visibility, dispatch hooks, and execution limits follow the established path.
Review also exposed a shared quote/glob bug affecting both backticks and
$(). Preserve the existing per-part quote metadata through glob escaping and use the same per-part decision for command-substitution byte charges. Cache quote-metadata presence once per word to avoid repeated scans on long mixed words.Before / After
Before: quoted-adjacent backticks were misread; the contributor's spec cases and focused regressions failed.
After, identical to Bash:
With virtual files
/tmp/p-aand/tmp/p-b:Before:
[/tmp/p-a]and[/tmp/p-b].After:
[/tmp/p-*]. The same guarantee holds for backticks and"$prefix"$empty.Risk
Validation
just pre-prreaches the existing macOS stack overflow inblackbox_security_tests::finding_nested_cmd_subst_stack_overflow::depth_50_is_bounded; reproduced again on a clean source snapshot of current mainc6fd211920d534ba0e4ca2c642e65747879efbbc. An earlier workspace library run also hit the unchanged macOS CPython alignment test; it was reproduced on its then-current clean mainbe8ec33c. Full local gates are not green.just bench: 96 cases, zero errors, 100% Bash output match. Parallel-execution Criterion smoke and measured runs complete on the conflict-resolution revision. No performance claim inferred from a busy local machine.c6fd2119: per-part quoting and byte-charge decisions now use the budgeted buffer introduced by the pending-array security fix. Its allocation guards, borrowed scalar expansion, capacity leases, boxed future, and consumed-output lease release are preserved. All 25 execution-budget, seven prompt-resource, seven arithmetic-resource, and 19 fail-point tests pass.a8bcbe730c968b5c45c660fb4a0518ed7d707d81, including the required aggregate Check, Linux tests, strict Bash/sed/grep parity, CPython native/interpreted tests, filesystem and terminal tests, fail-point tests, and property-based security tests. GitHub reports the PR clean and mergeable.1ec7edcdand current mainc6fd2119. No fresh combined scan started fora8bcbe73; GitHub default setup excludes fork PRs (GitHub documentation). The exact conflict-resolution diff was reviewed for preserving per-part escaping, allocation-before-growth checks, shared budget leases, and boxed recursive futures. No workflows or security configuration were changed.Checklist