Dash Archive mirror
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: dash@vger.kernel.org
Subject: [PATCH] eval: Check eflag after redirection error
Date: Wed, 7 Dec 2022 11:59:20 +0800	[thread overview]
Message-ID: <Y5APmNVQrIXFS7es@gondor.apana.org.au> (raw)
In-Reply-To: <20220818114412.zeqv5auujpuvfskk@tarta.nabijaczleweli.xyz>

наб <nabijaczleweli@nabijaczleweli.xyz> 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 наб <nabijaczleweli@nabijaczleweli.xyz> -----
> 
> 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: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

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) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

      parent reply	other threads:[~2022-12-07  3:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-18 11:44 Fwd: Bug#1017531: dash: for/while/if suppress errors from redirections with -e наб
2022-08-18 14:22 ` Michael Greenberg
2022-08-18 15:02   ` наб
2022-12-07  3:59 ` Herbert Xu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5APmNVQrIXFS7es@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=dash@vger.kernel.org \
    --cc=nabijaczleweli@nabijaczleweli.xyz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).