From 970069a6feb71424f4a98b9f3005181eeaa1c448 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 21 Sep 2020 23:02:08 +0200 Subject: [PATCH] Fix command substitutions in here-docs (rhbz#994241, rhbz#1036802) When ksh was compiled with SHOPT_SPAWN (the default), any command substitution embedded in a here-document returned an empty string. The bug was also present in 93u+ 2012-08-01 (although not in every case as some systems compile it without SHOPT_SPAWN). This fixes it by applying a slightly edited combination of two Red Hat patches (the second containing a fix for the first), which backport a new command substitution mechanism from the abandoned ksh 93v- beta version. The originals are: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-macro.patch https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fd2lost.patch src/cmd/ksh93/include/io.h: - The iopipe() function from xec.c is now needed in sh_subshell() (subshell.c), so rename it to sh_iounpipe() and declare it as an extern here. The 93v- beta did it as well. (The Red Hat patch did this without renaming it.) src/cmd/ksh93/sh/xec.c: - Backport new versions of iousepipe() and sh_iounpipe() from ksh 93v-. New 'type' flaggery is introduced to distinguish between different command substitution conditions. What all that means remains to be determined. - sh_exec(): I made one change to the Red Hat patch myself: if in a subshell and the type flags FAMP (for "ampersand" as in '&' as in background job) and TFORK are set, continue to call sh_subfork() to fork the subshell unconditionally, instead of only if we're in a command substitution connected to an unseekable file. Maybe the latter works for the 93v- code, but on 93u+(m) it causes a couple of regressions, which are fixed by my change: signal.sh[273]: subshell ignoring signal does not send signal to parent signal.sh[276]: subshell catching signal does not send signal to parent Details: https://github.com/ksh93/ksh/issues/104#issuecomment-696341902 src/cmd/ksh93/sh/macro.c, src/cmd/ksh93/sh/subshell.c: - Updates that go with those new functions. Fixes: https://github.com/ksh93/ksh/issues/104 Affects: https://github.com/ksh93/ksh/issues/124 --- src/cmd/ksh93/include/io.h | 1 + src/cmd/ksh93/sh/macro.c | 4 +- src/cmd/ksh93/sh/subshell.c | 19 ++++++++-- src/cmd/ksh93/sh/xec.c | 66 +++++++++++++++++++++------------ src/cmd/ksh93/tests/signal.sh | 16 +++++++- src/cmd/ksh93/tests/subshell.sh | 35 +++++++++++++++++ 6 files changed, 111 insertions(+), 30 deletions(-) diff --git a/src/cmd/ksh93/include/io.h b/src/cmd/ksh93/include/io.h index ee7d0cb92..5e4058e4e 100644 --- a/src/cmd/ksh93/include/io.h +++ b/src/cmd/ksh93/include/io.h @@ -81,6 +81,7 @@ extern void sh_iosave(Shell_t *, int,int,char*); extern int sh_iovalidfd(Shell_t*, int); extern int sh_inuse(Shell_t*, int); extern void sh_iounsave(Shell_t*); +extern void sh_iounpipe(Shell_t*); extern int sh_chkopen(const char*); extern int sh_ioaccess(int,int); extern int sh_devtofd(const char*); diff --git a/src/cmd/ksh93/sh/macro.c b/src/cmd/ksh93/sh/macro.c index 7af9ca1dc..69319f1d5 100644 --- a/src/cmd/ksh93/sh/macro.c +++ b/src/cmd/ksh93/sh/macro.c @@ -385,7 +385,7 @@ void sh_machere(Shell_t *shp,Sfio_t *infile, Sfio_t *outfile, char *string) break; } case S_PAR: - comsubst(mp,(Shnode_t*)0,1); + comsubst(mp,(Shnode_t*)0,3); break; case S_EOF: if((c=fcfill()) > 0) @@ -1150,7 +1150,7 @@ retry1: case S_PAR: if(type) goto nosub; - comsubst(mp,(Shnode_t*)0,1); + comsubst(mp,(Shnode_t*)0,3); return(1); case S_DIG: var = 0; diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 9a6993cc6..a999207c0 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -123,7 +123,8 @@ void sh_subtmpfile(Shell_t *shp) else if(errno!=EBADF) errormsg(SH_DICT,ERROR_system(1),e_toomany); /* popping a discipline forces a /tmp file create */ - sfdisc(sfstdout,SF_POPDISC); + if(shp->comsub != 1) + sfdisc(sfstdout,SF_POPDISC); if((fd=sffileno(sfstdout))<0) { /* unable to create the /tmp file so use a pipe */ @@ -173,6 +174,7 @@ void sh_subfork(void) register struct subshell *sp = subshell_data; Shell_t *shp = sp->shp; unsigned int curenv = shp->curenv; + char comsub = shp->comsub; pid_t pid; char *trap = shp->st.trapcom[0]; if(trap) @@ -204,7 +206,7 @@ void sh_subfork(void) shp->subshell = 0; shp->comsub = 0; sp->subpid=0; - shp->st.trapcom[0] = trap; + shp->st.trapcom[0] = (comsub==2 ? NULL : trap); shp->savesig = 0; /* sh_fork() increases ${.sh.subshell} but we forked an existing virtual subshell, so undo */ SH_SUBSHELLNOD->nvalue.s--; @@ -653,6 +655,13 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) } else { + if(comsub!=1 && shp->spid) + { + job_wait(shp->spid); + if(shp->pipepid==shp->spid) + shp->spid = 0; + shp->pipepid = 0; + } /* move tmp file to iop and restore sfstdout */ iop = sfswap(sfstdout,NIL(Sfio_t*)); if(!iop) @@ -761,7 +770,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) shp->coutpipe = sp->coutpipe; } shp->subshare = sp->subshare; - shp->comsub = sp->comsub; shp->subdup = sp->subdup; if(shp->subshell) { @@ -790,7 +798,12 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) if(nsig>0) kill(getpid(),nsig); if(sp->subpid) + { job_wait(sp->subpid); + if(comsub>1) + sh_iounpipe(shp); + } + shp->comsub = sp->comsub; if(comsub && iop && sp->pipefd<0) sfseek(iop,(off_t)0,SEEK_SET); if(shp->trapnote) diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 02151aef8..6b257d895 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -151,26 +151,30 @@ static /*inline*/ double timeval_to_double(struct timeval tv) * temp file. */ static int subpipe[3],subdup,tsetio,usepipe; -static void iounpipe(Shell_t*); static int iousepipe(Shell_t *shp) { - int i; + int fd=sffileno(sfstdout),i,err=errno; if(usepipe) { usepipe++; - iounpipe(shp); + sh_iounpipe(shp); } if(sh_rpipe(subpipe) < 0) return(0); usepipe++; - fcntl(subpipe[0],F_SETFD,FD_CLOEXEC); - subpipe[2] = sh_fcntl(1,F_DUPFD,10); - fcntl(subpipe[2],F_SETFD,FD_CLOEXEC); + if(shp->comsub!=1) + { + subpipe[2] = sh_fcntl(subpipe[1],F_DUPFD,10); + sh_close(subpipe[1]); + return(1); + } + subpipe[2] = sh_fcntl(fd,F_dupfd_cloexec,10); shp->fdstatus[subpipe[2]] = shp->fdstatus[1]; - close(1); - fcntl(subpipe[1],F_DUPFD,1); - shp->fdstatus[1] = shp->fdstatus[subpipe[1]]; + while(close(fd)<0 && errno==EINTR) + errno = err; + fcntl(subpipe[1],F_DUPFD,fd); + shp->fdstatus[1] = shp->fdstatus[subpipe[1]]&~IOCLEX; sh_close(subpipe[1]); if(subdup=shp->subdup) for(i=0; i < 10; i++) { @@ -184,14 +188,23 @@ static int iousepipe(Shell_t *shp) return(1); } -static void iounpipe(Shell_t *shp) +void sh_iounpipe(Shell_t *shp) { - int n; + int fd=sffileno(sfstdout),n,err=errno; char buff[SF_BUFSIZE]; - close(1); - fcntl(subpipe[2], F_DUPFD, 1); - shp->fdstatus[1] = shp->fdstatus[subpipe[2]]; + if(!usepipe) + return; --usepipe; + if(shp->comsub>1) + { + sh_close(subpipe[2]); + while(read(subpipe[0],buff,sizeof(buff))>0); + goto done; + } + while(close(fd)<0 && errno==EINTR) + errno = err; + fcntl(subpipe[2], F_DUPFD, fd); + shp->fdstatus[1] = shp->fdstatus[subpipe[2]]; if(subdup) for(n=0; n < 10; n++) { if(subdup&(1<subshell) { sh_subtmpfile(shp); - if(shp->comsub==1 && !(shp->fdstatus[1]&IONOSEEK)) - unpipe=iousepipe(shp); if((type&(FAMP|TFORK))==(FAMP|TFORK)) + { + if(shp->comsub && !(shp->fdstatus[1]&IONOSEEK)) + unpipe = iousepipe(shp); sh_subfork(); + } } no_fork = !ntflag && !(type&(FAMP|FPOU)) && !shp->subshell && !(shp->st.trapcom[SIGINT] && *shp->st.trapcom[SIGINT]) && @@ -1572,7 +1588,7 @@ int sh_exec(register const Shnode_t *t, int flags) if(parent<0) { if(shp->comsub==1 && usepipe && unpipe) - iounpipe(shp); + sh_iounpipe(shp); break; } #else @@ -1590,6 +1606,8 @@ int sh_exec(register const Shnode_t *t, int flags) nlock--; job_unlock(); } + if(shp->subshell) + shp->spid = parent; if(type&FPCL) sh_close(shp->inpipe[0]); if(type&(FCOOP|FAMP)) @@ -1605,11 +1623,15 @@ int sh_exec(register const Shnode_t *t, int flags) if(shp->pipepid) shp->pipepid = parent; else + { job_wait(parent); + if(parent==shp->spid) + shp->spid = 0; + } if(shp->topfd > topfd) sh_iorestore(shp,topfd,0); if(usepipe && tsetio && subdup && unpipe) - iounpipe(shp); + sh_iounpipe(shp); if(!sh_isoption(SH_MONITOR)) { shp->trapnote &= ~SH_SIGIGNORE; @@ -1809,7 +1831,7 @@ int sh_exec(register const Shnode_t *t, int flags) shp->exitval = type; } if(shp->comsub==1 && usepipe) - iounpipe(shp); + sh_iounpipe(shp); shp->pipepid = 0; shp->st.ioset = 0; if(simple && was_errexit) @@ -2857,7 +2879,7 @@ pid_t _sh_fork(Shell_t *shp,register pid_t parent,int flags,int *jobid) { if(shp->topfd > restorefd) sh_iorestore(shp,restorefd,0); - iounpipe(shp); + sh_iounpipe(shp); } } return(parent); @@ -3169,8 +3191,7 @@ static void sh_funct(Shell_t *shp,Namval_t *np,int argn, char *argv[],struct arg struct funenv fun; char *fname = nv_getval(SH_FUNNAMENOD); struct Level *lp =(struct Level*)(SH_LEVELNOD->nvfun); - int level, pipepid=shp->pipepid, comsub=shp->comsub; - shp->comsub = 0; + int level, pipepid=shp->pipepid; shp->pipepid = 0; sh_stats(STAT_FUNCT); if(!lp->hdr.disc) @@ -3213,7 +3234,6 @@ static void sh_funct(Shell_t *shp,Namval_t *np,int argn, char *argv[],struct arg lp->maxlevel = level; SH_LEVELNOD->nvalue.s = lp->maxlevel; shp->last_root = nv_dict(DOTSHNOD); - shp->comsub = comsub; nv_putval(SH_FUNNAMENOD,fname,NV_NOFREE); nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE); shp->pipepid = pipepid; diff --git a/src/cmd/ksh93/tests/signal.sh b/src/cmd/ksh93/tests/signal.sh index aee0873b4..1bec29701 100755 --- a/src/cmd/ksh93/tests/signal.sh +++ b/src/cmd/ksh93/tests/signal.sh @@ -272,10 +272,22 @@ done < tst.got if [[ ${SIG[USR1]} ]] then float s=$SECONDS - [[ $(LC_ALL=C $SHELL -c 'trap "print SIGUSR1 ; exit 0" USR1; (trap "" USR1 ; exec kill -USR1 $$ & sleep .5); print done') == SIGUSR1 ]] || err_exit 'subshell ignoring signal does not send signal to parent' + exp=SIGUSR1 + got=$(LC_ALL=C $SHELL -c ' + trap "print SIGUSR1 ; exit 0" USR1 + (trap "" USR1 ; exec kill -USR1 $$ & sleep .5) + print done') + [[ $got == "$exp" ]] || err_exit 'subshell ignoring signal does not send signal to parent' \ + "(expected '$exp', got '$got')" (( (SECONDS-s) < .4 )) && err_exit 'parent does not wait for child to complete before handling signal' ((s = SECONDS)) - [[ $(LC_ALL=C $SHELL -c 'trap "print SIGUSR1 ; exit 0" USR1; (trap "exit" USR1 ; exec kill -USR1 $$ & sleep .5); print done') == SIGUSR1 ]] || err_exit 'subshell catching signal does not send signal to parent' + exp=SIGUSR1 + got=$(LC_ALL=C $SHELL -c ' + trap "print SIGUSR1 ; exit 0" USR1 + (trap "exit" USR1 ; exec kill -USR1 $$ & sleep .5) + print done') + [[ $got == "$exp" ]] || err_exit 'subshell catching signal does not send signal to parent' \ + "(expected '$exp', got '$got')" (( SECONDS-s < .4 )) && err_exit 'parent completes early' fi diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 66cbf3274..cd20387d5 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -759,5 +759,40 @@ SHELL=$SHELL "$SHELL" -c ' ' | awk '/^DEBUG/ { pid[NR] = $2; } END { exit !(pid[1] == pid[2] && pid[2] == pid[3]); }' \ || err_exit "setting PATH to readonly in subshell triggers an erroneous fork" +# ======' +# Test command substitution with external command in here-document +# https://github.com/ksh93/ksh/issues/104 +expect=$'/dev/null\n/dev/null' +actual=$( + cat <<-EOF + $(ls /dev/null) + `ls /dev/null` + EOF +) +[[ $actual == "$expect" ]] || err_exit 'Command substitution in here-document fails' \ + "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" + +# ...and in pipeline (rhbz#994251) +expect=/dev/null +actual=$(cat /dev/null | "$binecho" `ls /dev/null`) +[[ $actual == "$expect" ]] || err_exit 'Command substitution in pipeline fails (1)' \ + "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" + +# ...and in pipeline again (rhbz#1036802: standard error was misdirected) +expect=$'END2\naaa\nEND1\nEND3' +actual=$(export bincat binecho; "$SHELL" 2>&1 -c \ + 'function foo + { + "$binecho" hello >/dev/null 2>&1 + "$binecho" aaa | "$bincat" + echo END1 + echo END2 >&2 + } + echo "$(foo)" >&2 + echo END3 >&2 + exit') +[[ $actual == "$expect" ]] || err_exit 'Command substitution in pipeline fails (2)' \ + "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" + # ====== exit $((Errors<125?Errors:125))