eval: Check eflag after redirection error

Commit Message

Herbert Xu Dec. 7, 2022, 3:59 a.m. UTC

наб <[e-mail]> wrote:
> 
> Thought I'd forward this upstream, considering I already tested on
> current git, and it's pretty grave.
> 
> https://bugs.debian.org/1017531, trimmed down a bit, follows:
> 
> ----- Forwarded message from наб <[e-mail]> -----
> 
> Subject: Bug#1017531: dash: for/while/if suppress errors from redirections
> with -e, POSIX violation
> 
> Dear Maintainer,
> 
> On current git (057cd650a4edd5856213d431a974ff35c6594489) and 0.5.11.5
> the following holds:
> -- >8 --
> $ ./src/dash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> -- >8 --
> 
> The correct output, as demonstrated by bash, is:
> -- >8 --
> $ bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> -- >8 --
> 
> This is a POSIX violation, and quite a grave one at that:
> set -e is oft[1] used to guard against precisely this type of error!
> 
> The same happens if set -e is executed.
> 
> All quotes POSIX.1, Issue 7, TC2:
> sh, OPTIONS:
>  > The -a, -b, -C, -e, -f, -m, -n, -o option, -u, -v, and -x options
>  > are described as part of the set utility in Special Built-In
>  > Utilities. 
> 
> set, DESCRIPTION, -e:
>  > When this option is on, when any command fails (for any of the
>  > reasons listed in Consequences of Shell Errors or by returning an
>  > exit status greater than zero), the shell immediately shall exit, as
>  > if by executing the exit special built-in utility with no arguments,
>  > with the following exceptions:
>  >
>  > 1. The failure of any individual command in a multi-command pipeline
>  >    shall not cause the shell to exit. Only the failure of the
>  >    pipeline itself shall be considered.
>  > 2. The -e setting shall be ignored when executing the compound list
>  >    following the while, until, if, or elif reserved word, a pipeline
>  >    beginning with the ! reserved word, or any command of an AND-OR
>  >    list other than the last.
>  > 3. If the exit status of a compound command other than a subshell
>  >    command was the result of a failure while -e was being ignored,
>  >    then -e shall not apply to this command.
> 
> XCU, 2.9.4: Shell Command Language, Shell Commands, Compound Commands:
> The while Loop:
>  > The format of the while loop is as follows:
>  > 
>  > while compound-list-1
>  > do
>  >   compound-list-2
>  > done
> (until is equivalent).
> The if Conditional Construct: 
>  > The format for the if construct is as follows:
>  >
>  > if compound-list
>  > then
>  >   compound-list
>  > [elif compound-list
>  > then
>  >   compound-list] ...
>  > [else
>  >   compound-list]
>  > fi
> 
> It follows, therefore, that
>  * Exception 1. does not apply as there is no pipeline
>  * Exception 2. does not apply, as the redirection does /not/ follow
>    "while" or "if" directly and is /not/ part of the conditional
>        compound-list
>  * in the "for" case, there is no such provision, so this is likely not
>    a confusion w.r.t. the conditional compound-lists
>  * Exception 3. does not apply as -e was not being ignored while the
>    compound commands were being executed (indeed, the compound commands
>    do not run at all, as evidenced by the program terminating)
> 
> [1]: https://salsa.debian.org/glibc-team/glibc/-/merge_requests/6#note_329899
> ----- End forwarded message -----

Yes we should check the exit status after redirections.

Reported-by: наб <[e-mail]>
Signed-off-by: Herbert Xu <[e-mail]>

Patch

diff --git a/src/eval.c b/src/eval.c
index eda251e..7aa5cc2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -246,15 +246,18 @@  evaltree(union node *n, int flags)
 			lineno -= funcline - 1;
 		expredir(n->nredir.redirect);
 		pushredir(n->nredir.redirect);
-		status = redirectsafe(n->nredir.redirect, REDIR_PUSH) ?:
-			 evaltree(n->nredir.n, flags & EV_TESTED);
+		status = redirectsafe(n->nredir.redirect, REDIR_PUSH);
+		if (status)
+			checkexit = EV_TESTED;
+		else
+			status = evaltree(n->nredir.n, flags & EV_TESTED);
 		if (n->nredir.redirect)
 			popredir(0);
 		break;
 	case NCMD:
 		evalfn = evalcommand;
 checkexit:
-		checkexit = ~flags & EV_TESTED;
+		checkexit = EV_TESTED;
 		goto calleval;
 	case NFOR:
 		evalfn = evalfor;
@@ -316,7 +319,7 @@  calleval:
 out:
 	dotrap();
 
-	if (eflag && checkexit && status)
+	if (eflag && (~flags & checkexit) && status)
 		goto exexit;
 
 	if (flags & EV_EXIT) {