fix(redir): validate the fd word of a {var} dup redirect - #659
Merged
Conversation
`{v}<&word' / `{v}>&word' converted the target word with atoi, so a
non-numeric word (`{v}<&foo') silently duplicated fd 0 and succeeded.
bash rejects any word that is not a plain fd number -- `foo', `0junk',
`-1', `+1', an empty expansion -- as an ambiguous redirect naming the
VARIABLE, leaving it untouched, and the redirection fails with status 1.
Validate the word before converting (strtoll, range-checked: an fd
beyond INT_MAX cannot exist and reports Bad file descriptor rather than
being truncated). A failed dup on the {var} form prints bash's
two-line report -- the generic `cannot duplicate fd' line, then the
CONVERTED number (`010' reports as `10'), or the raw digits when the
value does not fit an intmax. bash's reports for a quoted-empty or
set-variable non-move target print an uninitialized number (a bash
bug); the ambiguous/converted reports here cover those cases
deterministically instead.
Closes #656
…lure
A failed redirection on a simple command returned status 1 but skipped
the ERR trap and `set -e' entirely: `set -e; exec {v}<&foo; echo x'
kept going where bash exits 1. The compound-command path already
handled this; mirror it at the simple-command apply_redirects failure,
including `!' inverting the failure to success (`! echo hi >/bad/f'
is status 0, and does not trip errexit).
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.
Summary
{v}<&word/{v}>&wordconverted the target word withatoi, so a non-numeric word (exec {v}<&foo) silently duplicated fd 0 and succeeded. bash rejects any word that is not a plain fd number as an ambiguous redirect naming the variable (v: ambiguous redirect), leaves the variable untouched, and fails the redirection with status 1.Closes #656
Changes
core/src/executor.cpp(dup path): validate the word before converting —foo,0junk,-1,+1, and empty expansions arev: ambiguous redirect. Conversion usesstrtollwith a range check: an fd beyondINT_MAXcannot exist and reportsBad file descriptorinstead of being truncated into a valid fd. A failed dup prints bash's two-line report for the{var}form — the genericcannot duplicate fdline, then the converted number (010reports as10), or the raw digits when the value doesn't fit an intmax. Two bash-side quirks (it prints an uninitialized garbage number for<&""and for set-variable non-move targets — nondeterministic across runs) are deliberately not replicated; gnash reports deterministically.core/src/executor.cpp(separate commit): the issue's repro (set -e; exec {v}<&foo; echo success) exposed that a failed redirection on a simple command returned status 1 but skipped the ERR trap andset -eentirely — a pre-existing gap affecting every redirect failure (set -e; echo hi > /nonexistent-dir/f; ...also kept going). It now mirrors the compound-command path, including!inverting the failure to success.tests/harness/run_diff.sh: 14 differential cases (484 total).Verification
v: ambiguous redirect, exit 1, nosuccess.set -e,!inversion, errexit-suppressed contexts).run_diff484/484 (three consecutive clean runs);ctest23/23; full 83-suite scoreboard sweep with fresh oracles: no regressions (redir/vredir0; only the standing comsub-eof=5 and the pre-existing errors=125 of errors.tests: remaining fatal-error divergences (readonly/posix aborts, ${$var} and ${-N} substitutions, set -u statuses) #658 remain).