bin/48631: sh synerrs case where list to execute is entirely redirections (+FIX?)

[kre%munnari.OZ.AU@localhost]

>Number:         48631
>Category:       bin
>Synopsis:       sh synerrs case where list to execute is entirely redirections 
>(+FIX?)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 02 14:10:00 +0000 2014
>Originator:     Robert Elz
>Release:        NetBSD -current (6.99.31 or whatever) (and all earlier?)
>Organization:
        Prince of Songkla University
>Environment:
System: NetBSD perseus.noi.kre.to 6.99.17 NetBSD 6.99.17 (GENERIC) #1: Fri Feb 
22 22:09:50 ICT 2013 
kre%jade.coe.psu.ac.th@localhost:/usr/obj/current/i386/sys/arch/i386/compile/GENERIC
 i386
Architecture: i386
Machine: i386
>Description:
        sh cannot correctly parse a case statement where the commands
        to execute (after any pattern) consist entirely of redirections.

        That is, something like
                case $var in
                create) >$filename ;;
                esac

        If the ">$filename" is missing (empty list) all is OK, as it
        is if there is some other command there.

        Other shells (ksh, bash at least) don't have this problem, and
        I see nothing in the sh standard that would suggest this should
        not be legal syntax.

>How-To-Repeat:
        echo 'case foo in x) >/tmp/foobar ;; esac' | /bin/sh

        Also:
        echo 'case foo in x) >/tmp/foobar esac' | /bin/sh

        Expect to see:
                sh: Syntax error: ";;" unexpected
        in the first case and
                sh: Syntax error: end of file unexpected (expecting ";;")
        in the second.

        Both work if the redirect is omitted.

        This is a syntax error, so whether the case word matches the
        pattern or not is irrelevant - by forcing no match in the
        test cases above, you don't need to worry if you have a /tmp/foobar,
        it won't get clobbered anyway...

>Fix:
        The following patch (down below) corrects the first problem for me.

        Notes:

        1) I have not run sh tests, I am not sure that the patch does
           not allow illegal syntax in cases where it should not, though
           I have been unable to find any in a few (try stuff) tests I
           did attempt.

        2) I have actually tested this in a sh from an older version of
           6.99.x than is current - then I downloaded the most recent
           version of the file affected (parser.c) and applied the same
           fix for the purposes of sending this patch.   That (ie: this)
           version has not even been compile tested, though I did check
           the diffs fairly carefully, and I believe this to be as I
           intended.

        3) This patch does not allow the ;; to be omitted (the second case
           above), which should be legal before the esac, but I don't use
           that (I always include the ;;) so I didn't bother looking for a
           fix that would handle that syntax as well.   I don't feel all
           that bad about that, neither ksh (NetBSD's /bin/ksh anyway)
           nor bash handle this either....

        Hence: please DO NOT commit this patch without actually checking
        that it works!   (And I won't be in the slightest offended if
        someone decides there's a much better way to fix the problem than
        this one).

        Apply this patch to parser.c (the filenames in the patch won't help!)
        in src/bin/sh of current.

        When verified correct, the fix (Whatever it ends up being) for this
        should probably be pulled up to all other currently supported versions.

        ps: I have no idea what it would mean to have a ! with just a
        redirect, so I don't know if the "checkneg" goto is really
        correct, but when there's no ! it works... (or seems to).

--- parser.c-current    2014-03-01 16:17:05.000000000 +0700
+++ parser.c-current-fixed      2014-03-02 12:06:27.000000000 +0700
@@ -513,8 +513,14 @@
        case TRP:
                tokpushback++;
                n1 = simplecmd(rpp, redir);
                goto checkneg;
+       case TENDCASE:
+               if (redir) {
+                       tokpushback++;
+                       goto checkneg;
+               }
+               /* FALLTHROUGH */
        default:
                synexpect(-1);
                /* NOTREACHED */
        }