Skip to content

fix(expander): split and glob an unquoted ${p:-w} / ${p:+w} / ${p:=w} result - #163

Open
LESdylan wants to merge 1 commit into
developfrom
fix/opword-field-split
Open

LESdylan wants to merge 1 commit into
developfrom
fix/opword-field-split

Conversation

@LESdylan

Copy link
Copy Markdown
Member

What & why

P='a b c'; set -- ${P:-x}; echo $#      # hellish 1, bash 3
set -- ${u:-*.c}                        # hellish: the word *.c, no glob

POSIX (2.6.2, 2.6.5, 2.6.6) says the result of an unquoted parameter expansion is field-split and pathname-expanded, whichever operator produced it. hellish kept the whole result as one field whenever the text of the operator word had no unquoted blank. That rule was wrong in three ways:

  • It looked at the word even when the word was not used. So the variable's own value was never split: ${P:-x}, ${P-x}, ${P:+$P}, ${P:?e}, ${U:="a b"}.
  • It could not see inside expansions. ${u:-$P} never split, and ${u:-$Q} and ${u:-a*} never globbed.
  • A word with a blank was split flat, quotes and all. ${u:-"a b"c d} gave three fields (a / bc / d) where bash gives two (a bc / d), and ${u:-a\ b} gave two.

Root cause. expand_op_token retyped the token to TT_DQWORD (one field, no glob) based on opword_no_split(word text). By the time the result reached the splitter, the quoting of each part of the word was gone.

Change.

  • The variable's value, and the new value set by ${p:=w}, stay a plain unquoted expansion, split and globbed exactly like $p.
  • A used word of - or + is expanded by pf_op_word_segments (expand_param_opword.c) into segments that keep their quoting:
    • q for '...', "...", $'...', a backslash escape, and a tilde prefix. POSIX 2.6.1 makes ${u:-~} one field even when HOME contains a blank.
    • u for everything else, literal text included.
  • The segments are parked behind a SEG_MAGIC marker, the same way ${a[@]} parks its elements. The splitter (emit_op_segments) IFS-splits the u segments into the current field and appends the q segments verbatim as TT_DQENVVAR children, so the glob stage escapes them.
  • Empty results are unchanged:
    • an unquoted empty result is still no field, as in git-completion's ${a:+"${a[@]}"} ${d:+--git-dir="$d"};
    • ${x:-""} is still one empty field.
  • opword_no_split is gone; nothing else used it.

How I verified it

  • Reproduced first. tests/scripts/57_opword_fields.sh is graded against bash --posix 5.3.9. It covers:

    • the variable's own value under each operator;
    • used words: unquoted, quoted, backslash-escaped, $'...', and mixed;
    • globs, command substitution, and $@ / $* / "$@" in the word;
    • empty results next to literal text, :=, and a tilde with a blank HOME;
    • IFS=:, set -f, and the contexts that do not split (quotes, assignment, case, for).

    The binary without this fix diverges on 25 of its 65 lines; this branch matches bash byte for byte.

  • Local gates on this commit (ASan debug build):

    • golden tests/tester: 5363/5363;
    • tests/run_scripts.sh against bash --posix: 126/126;
    • verify_alloc.sh: identical output on both heaps;
    • alloc_stress.sh: all clean;
    • tests/pty_suite.sh: 111 ok, 6 skipped, 4 failed. plugin_corpus_test, which loads git-completion and its ${a:+"${a[@]}"} idioms, passes. None of the four failures is this change:
      • prompt_compat_matrix, prompt_drift_matrix and prompt_jobs_badge expect the non-root %/$ prompt and get #, because the container runs as root. They fail the same way on develop, and CI runs them as a normal user.
      • hxp_framework_test hit the 420 s per-file limit on this 4-core container. develop's binary takes 440 s for it on the same machine, and CI's runners finish it inside the limit.
  • norminette is OK on every touched file.

Notes / trade-offs

  • This is a behaviour change for scripts that relied on the old one-field result of ${p:-a b} unquoted. The new result is what bash, dash and POSIX give; quoting the expansion keeps one field in every shell.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RAmeHfJNm7XjYbMNrQqvkG


Generated by Claude Code

… result

    P='a b c'; set -- ${P:-x}; echo $#      # hellish 1, bash 3
    set -- ${u:-*.c}                        # hellish: the word *.c, no glob

POSIX (2.6.2, 2.6.5, 2.6.6): the result of an unquoted parameter
expansion is field-split and pathname-expanded, whichever operator made
it. hellish kept the whole result as one field whenever the operator
word's TEXT held no unquoted blank:
  - it looked at the word even when the word was not used, so the
    variable's own value was never split (${P:-x}, ${P-x}, ${P:+$P},
    ${P:?e}, ${U:="a b"});
  - it could not see inside expansions, so ${u:-$P} never split and
    ${u:-$Q}, ${u:-a*} never globbed;
  - a word with a blank was split flat, quotes and all:
    ${u:-"a b"c d} gave three fields, a / bc / d, where bash gives
    two, "a bc" / d; and ${u:-a\ b} gave two.

Root cause: expand_op_token retyped the token to TT_DQWORD (one field,
no glob) based on opword_no_split(word text). The quoting of each part
of the word was gone by the time the result reached the splitter.

Now:
  - the variable's value, and the new value of ${p:=w}, stay a plain
    unquoted expansion: split and globbed exactly like $p;
  - a used word of - or + is expanded by pf_op_word_segments
    (expand_param_opword.c) into segments that keep their quoting:
    'q' for '...', "...", $'...', a backslash escape and a tilde prefix
    (POSIX 2.6.1: ${u:-~} is one field even when HOME has a blank);
    'u' for everything else, literal text included. The segments are
    parked behind a SEG_MAGIC marker, the same way ${a[@]} parks its
    elements. The splitter (emit_op_segments) IFS-splits the 'u' ones
    into the current field and appends the 'q' ones verbatim, as
    TT_DQENVVAR children, so the glob stage escapes them.
  - empty results are unchanged: an unquoted empty result is no field
    (git-completion's `${a:+"${a[@]}"} ${d:+--git-dir="$d"}`), and
    ${x:-""} is still one empty field.

opword_no_split is gone; nothing else used it.

tests/scripts/57_opword_fields.sh, graded against bash --posix, covers:
  - the value of each operator;
  - used words: unquoted, quoted, backslash-escaped, $'...', mixed;
  - globs, command substitution, $@ / $* / "$@" in the word;
  - empty results next to literal text, :=, tilde with a blank HOME;
  - IFS=:, set -f, and the contexts that do not split (quotes,
    assignment, case, for).
The binary without this fix diverges on 25 of its 65 lines; this
branch matches bash byte for byte.

Built on fix/quoted-at-no-glob: the quoted segments use its
push_new_dq_child.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RAmeHfJNm7XjYbMNrQqvkG

This branch has not been deployed

No deployments
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.

2 participants