[BUG] process substitution breaks when nested or traverses a function

[BUG] process substitution breaks when nested or traverses a function

[Francisco de Zuviría Allende]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

#!/bin/bash
foo() { cat <(cat "$@"); }; foo <(echo bar);
bar

#!/bin/zsh
foo() { cat <(cat "$@"); }; foo <(echo bar);
cat: /proc/self/fd/11: No such file or directory

Best of regards

Re: [BUG] process substitution breaks when nested or traverses a function

[Peter Stephenson]

On Sat, 21 Apr 2018 06:35:23 -0300
Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> #!/bin/zsh
> foo() { cat <(cat "$@"); }; foo <(echo bar);
> cat: /proc/self/fd/11: No such file or directory

Files and file descriptors for process substitution are handled
specially, so shouldn't be tidied up by closem(), which is called on the
inner process substitution (that's why both were required for this to
show up).

Is there a better way of doing this; or is this patch overkill, or not
careful enough...?  If we don't have /proc/self, tidying up is done only
with the job filelist, so there wouldn't be a problem.  But the get out
"all" argument looks like a reasonable safety compromise.

pws

diff --git a/Src/Modules/clone.c b/Src/Modules/clone.c
index 9304292..ef6275d 100644
--- a/Src/Modules/clone.c
+++ b/Src/Modules/clone.c
@@ -69,7 +69,7 @@ bin_clone(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	dup2(ttyfd,2);
 	if (ttyfd > 2)
 	    close(ttyfd);
-	closem(0);
+	closem(FDT_UNUSED, 0);
 	close(coprocin);
 	close(coprocout);
 	/* Acquire a controlling terminal */
diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 1c93a1d..2f83f7c 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -400,7 +400,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 	dup2(slave, 1);
 	dup2(slave, 2);
 
-	closem(0);
+	closem(FDT_UNUSED, 0);
 	close(slave);
 	close(master);
 	close(coprocin);
diff --git a/Src/exec.c b/Src/exec.c
index e853aff..65139de 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -698,7 +698,7 @@ execute(LinkList args, int flags, int defpath)
      * Note that we don't close fd's attached to process substitution
      * here, which should be visible to external processes.
      */
-    closem(FDT_XTRACE);
+    closem(FDT_XTRACE, 0);
 #ifndef FD_CLOEXEC
     if (SHTTY != -1) {
 	close(SHTTY);
@@ -3924,7 +3924,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		LinkList assigns = (LinkList)0;
 		int postassigns = eparams->postassigns;
 		if (forked)
-		    closem(FDT_INTERNAL);
+		    closem(FDT_INTERNAL, 0);
 		if (postassigns) {
 		    Wordcode opc = state->pc;
 		    state->pc = eparams->assignspc;
@@ -4110,7 +4110,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		    if (errflag)
 			_exit(1);
 		}
-		closem(FDT_INTERNAL);
+		closem(FDT_INTERNAL, 0);
 		if (coprocin != -1) {
 		    zclose(coprocin);
 		    coprocin = -1;
@@ -4172,7 +4172,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	for (i = 0; i < 10; i++)
 	    if (fdtable[i] != FDT_UNUSED)
 		close(i);
-	closem(FDT_UNUSED);
+	closem(FDT_UNUSED, 1);
 	if (thisjob != -1)
 	    waitjobs();
 	_exit(lastval);
@@ -4352,12 +4352,17 @@ fixfds(int *save)
 
 /**/
 mod_export void
-closem(int how)
+closem(int how, int all)
 {
     int i;
 
     for (i = 10; i <= max_zsh_fd; i++)
 	if (fdtable[i] != FDT_UNUSED &&
+	    /*
+	     * Process substitution needs to be visible to user;
+	     * fd's are explicitly cleaned up by filelist handling.
+	     */
+	    (all || fdtable[i] != FDT_PROC_SUBST) &&
 	    (how == FDT_UNUSED || (fdtable[i] & FDT_TYPE_MASK) == how)) {
 	    if (i == SHTTY)
 		SHTTY = -1;
@@ -4840,7 +4845,7 @@ getproc(char *cmd, char **eptr)
 	    addproc(pid, NULL, 1, &bgtime);
 	return pnam;
     }
-    closem(FDT_UNUSED);
+    closem(FDT_UNUSED, 0);
     fd = open(pnam, out ? O_WRONLY | O_NOCTTY : O_RDONLY | O_NOCTTY);
     if (fd == -1) {
 	zerr("can't open %s: %e", pnam, errno);
@@ -4879,7 +4884,7 @@ getproc(char *cmd, char **eptr)
     }
     entersubsh(ESUB_ASYNC|ESUB_PGRP);
     redup(pipes[out], out);
-    closem(FDT_UNUSED);   /* this closes pipes[!out] as well */
+    closem(FDT_UNUSED, 0);   /* this closes pipes[!out] as well */
 #endif /* PATH_DEV_FD */
 
     cmdpush(CS_CMDSUBST);
@@ -4929,7 +4934,7 @@ getpipe(char *cmd, int nullexec)
     }
     entersubsh(ESUB_PGRP);
     redup(pipes[out], out);
-    closem(FDT_UNUSED);	/* this closes pipes[!out] as well */
+    closem(FDT_UNUSED, 0);	/* this closes pipes[!out] as well */
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1, out ? "outsubst" : "insubst");
     cmdpop();
diff --git a/Test/D03procsubst.ztst b/Test/D03procsubst.ztst
index ca8d56f..861aa9c 100644
--- a/Test/D03procsubst.ztst
+++ b/Test/D03procsubst.ztst
@@ -149,3 +149,10 @@
   fi
 0:proc subst fd in forked subshell closed in parent (external command)
 >1	1
+
+  procfunc() {
+    cat <(cat "$@")
+  }
+  procfunc <(echo argument)
+0:With /proc/self file descriptors must not be tidied up too early
+>argument

Re: [BUG] process substitution breaks when nested or traverses a function

[Daniel Shahaf]

Peter Stephenson wrote on Tue, 24 Apr 2018 10:43 +0100:
> On Sat, 21 Apr 2018 06:35:23 -0300
> Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> > #!/bin/zsh
> > foo() { cat <(cat "$@"); }; foo <(echo bar);
> > cat: /proc/self/fd/11: No such file or directory
> 
> Files and file descriptors for process substitution are handled
> specially, so shouldn't be tidied up by closem(), which is called on the
> inner process substitution (that's why both were required for this to
> show up).
> 
> Is there a better way of doing this; or is this patch overkill, or not
> careful enough...?  If we don't have /proc/self, tidying up is done only
> with the job filelist, so there wouldn't be a problem.  But the get out
> "all" argument looks like a reasonable safety compromise.

I don't know about "better", but I'd looked into this and wondered why
getproc() had two compile-time alternative implementations, one with
mkfifo() and one with /proc/self.  Is this just about portability, or...?

> @@ -4352,12 +4352,17 @@ fixfds(int *save)
>  /**/
>  mod_export void
> -closem(int how)
> +closem(int how, int all)

Maybe add the new parameter 'all' to the docstring?

Cheers,

Daniel

Re: [BUG] process substitution breaks when nested or traverses a function

[Peter Stephenson]

On Tue, 24 Apr 2018 10:30:16 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> I don't know about "better", but I'd looked into this and wondered why
> getproc() had two compile-time alternative implementations, one with
> mkfifo() and one with /proc/self.  Is this just about portability,
> or...?

Using a file descriptor is neater, because there's less cruft in the
regular file system to clear up (and fewer associated security worries
etc. etc.); however, /proc/self isn't guaranteed to be available on
older systems, whereas FIFOs have been around for a good quarter
century.

Whether zsh is being complied on sufficiently old systems we don't tend
to hear about, but I don't think having the FIFO alternative is causing
problems.

I'll update the comment.

pws

Re: [BUG] process substitution breaks when nested or traverses a function

[Martijn Dekker]

Op 24-04-18 om 12:47 schreef Peter Stephenson:
> On Tue, 24 Apr 2018 10:30:16 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>> I don't know about "better", but I'd looked into this and wondered why
>> getproc() had two compile-time alternative implementations, one with
>> mkfifo() and one with /proc/self.  Is this just about portability,
>> or...?
> 
> Using a file descriptor is neater, because there's less cruft in the
> regular file system to clear up (and fewer associated security worries
> etc. etc.); however, /proc/self isn't guaranteed to be available on
> older systems, whereas FIFOs have been around for a good quarter
> century.

/proc/self (or /proc in general) is not available on some very much 
current systems either, including at least macOS and OpenBSD.

/dev/fd is available on both of those, though.

- M.

Re: [BUG] process substitution breaks when nested or traverses a function

[Peter Stephenson]

On Tue, 24 Apr 2018 13:09:27 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> /proc/self (or /proc in general) is not available on some very much 
> current systems either, including at least macOS and OpenBSD.
> 
> /dev/fd is available on both of those, though.

We check in turn for /proc/self/fd and /dev/fd, so they're treated the
same.

pws

Re: [BUG] process substitution breaks when nested or traverses a function

[Francisco de Zuviría Allende]

This problem still happens. There is still no bug tracking system for
zsh, right?

On Sat, Apr 21, 2018 at 6:35 AM, Francisco de Zuviría Allende
<franciscodezuviria@gmail.com> wrote:
> #!/bin/bash
> foo() { cat <(cat "$@"); }; foo <(echo bar);
> bar
>
> #!/bin/zsh
> foo() { cat <(cat "$@"); }; foo <(echo bar);
> cat: /proc/self/fd/11: No such file or directory
>
> Best of regards

Re: [BUG] process substitution breaks when nested or traverses a function

[Peter Stephenson]

On Sun, 17 Jun 2018 00:14:10 -0300
Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> This problem still happens.
> > #!/bin/zsh
> > foo() { cat <(cat "$@"); }; foo <(echo bar);
> > cat: /proc/self/fd/11: No such file or directory

This was fixed in 5.5.  There's now a test for it at the end of
D03procsubst.ztst, so failures will get picked up.

> There is still no bug tracking system for zsh, right?

No, we've never had a volunteer to look after one.  The list is the
right place to report bugs.

pws

Re: [BUG] process substitution breaks when nested or traverses a function

[Francisco de Zuviría Allende]

I have zsh 5.5.1 in Debian Testing.

I can confirm it is fixed. HOWEVER, in a shell that uses the oh my zsh
framework, the bug persists! (even if I disable all plugins)

I had to login as root (which has no .zshrc) to make foo() { cat <(cat
"$@"); }; foo <(echo bar); work

I will report in oh my zsh as well, but I don't think it's their
fault, and I don't think they'll be able to do anything about it.
declaring a foo and using cat should work, regardless of how many
functions have been loaded previously. There's something messing
zsh...


On Mon, Jun 18, 2018 at 6:28 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Sun, 17 Jun 2018 00:14:10 -0300
> Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
>> This problem still happens.
>> > #!/bin/zsh
>> > foo() { cat <(cat "$@"); }; foo <(echo bar);
>> > cat: /proc/self/fd/11: No such file or directory
>
> This was fixed in 5.5.  There's now a test for it at the end of
> D03procsubst.ztst, so failures will get picked up.
>
>> There is still no bug tracking system for zsh, right?
>
> No, we've never had a volunteer to look after one.  The list is the
> right place to report bugs.
>
> pws
>
>

Re: [BUG] process substitution breaks when nested or traverses a function

[Francisco de Zuviría Allende]

I created https://github.com/robbyrussell/oh-my-zsh/issues/6951
Hopefully some can narrow down the problem

On Wed, Jun 27, 2018 at 6:04 AM, Francisco de Zuviría Allende
<franciscodezuviria@gmail.com> wrote:
> I have zsh 5.5.1 in Debian Testing.
>
> I can confirm it is fixed. HOWEVER, in a shell that uses the oh my zsh
> framework, the bug persists! (even if I disable all plugins)
>
> I had to login as root (which has no .zshrc) to make foo() { cat <(cat
> "$@"); }; foo <(echo bar); work
>
> I will report in oh my zsh as well, but I don't think it's their
> fault, and I don't think they'll be able to do anything about it.
> declaring a foo and using cat should work, regardless of how many
> functions have been loaded previously. There's something messing
> zsh...
>
>
> On Mon, Jun 18, 2018 at 6:28 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
>> On Sun, 17 Jun 2018 00:14:10 -0300
>> Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
>>> This problem still happens.
>>> > #!/bin/zsh
>>> > foo() { cat <(cat "$@"); }; foo <(echo bar);
>>> > cat: /proc/self/fd/11: No such file or directory
>>
>> This was fixed in 5.5.  There's now a test for it at the end of
>> D03procsubst.ztst, so failures will get picked up.
>>
>>> There is still no bug tracking system for zsh, right?
>>
>> No, we've never had a volunteer to look after one.  The list is the
>> right place to report bugs.
>>
>> pws
>>
>>

Re: [BUG] process substitution breaks when nested or traverses a function

[Francisco de Zuviría Allende]

silly me, I forgot to fire up zsh while logged as root.

The problem persists :(

Maybe a regression from 5.5.1 ?

On Wed, Jun 27, 2018 at 6:14 AM, Francisco de Zuviría Allende
<franciscodezuviria@gmail.com> wrote:
> I created https://github.com/robbyrussell/oh-my-zsh/issues/6951
> Hopefully some can narrow down the problem
>
> On Wed, Jun 27, 2018 at 6:04 AM, Francisco de Zuviría Allende
> <franciscodezuviria@gmail.com> wrote:
>> I have zsh 5.5.1 in Debian Testing.
>>
>> I can confirm it is fixed. HOWEVER, in a shell that uses the oh my zsh
>> framework, the bug persists! (even if I disable all plugins)
>>
>> I had to login as root (which has no .zshrc) to make foo() { cat <(cat
>> "$@"); }; foo <(echo bar); work
>>
>> I will report in oh my zsh as well, but I don't think it's their
>> fault, and I don't think they'll be able to do anything about it.
>> declaring a foo and using cat should work, regardless of how many
>> functions have been loaded previously. There's something messing
>> zsh...
>>
>>
>> On Mon, Jun 18, 2018 at 6:28 AM, Peter Stephenson
>> <p.stephenson@samsung.com> wrote:
>>> On Sun, 17 Jun 2018 00:14:10 -0300
>>> Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
>>>> This problem still happens.
>>>> > #!/bin/zsh
>>>> > foo() { cat <(cat "$@"); }; foo <(echo bar);
>>>> > cat: /proc/self/fd/11: No such file or directory
>>>
>>> This was fixed in 5.5.  There's now a test for it at the end of
>>> D03procsubst.ztst, so failures will get picked up.
>>>
>>>> There is still no bug tracking system for zsh, right?
>>>
>>> No, we've never had a volunteer to look after one.  The list is the
>>> right place to report bugs.
>>>
>>> pws
>>>
>>>

Re: [BUG] process substitution breaks when nested or traverses a function

[Daniel Tameling]

On Wed, Jun 27, 2018 at 06:18:20AM -0300, Francisco de Zuviría Allende wrote:
> >>> On Sun, 17 Jun 2018 00:14:10 -0300
> >>> Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> >>>> This problem still happens.
> >>>> > #!/bin/zsh
> >>>> > foo() { cat <(cat "$@"); }; foo <(echo bar);
> >>>> > cat: /proc/self/fd/11: No such file or directory

It also doesn't work for me with 5.5.1, but it works with master.

--
Best,
Daniel

Re: [BUG] process substitution breaks when nested or traverses a function

[Peter Stephenson]

On Thu, 28 Jun 2018 20:37:53 +0200
Daniel Tameling <tamelingdaniel@gmail.com> wrote:
> On Wed, Jun 27, 2018 at 06:18:20AM -0300, Francisco de Zuviría
> Allende wrote:
> > >>> On Sun, 17 Jun 2018 00:14:10 -0300
> > >>> Francisco de Zuviría Allende <franciscodezuviria@gmail.com>
> > >>> wrote:  
> > >>>> This problem still happens.  
> > >>>> > #!/bin/zsh
> > >>>> > foo() { cat <(cat "$@"); }; foo <(echo bar);
> > >>>> > cat: /proc/self/fd/11: No such file or directory  
> 
> It also doesn't work for me with 5.5.1, but it works with master.

Thanks --- my mistake.  Yes, looking at the logs it was about a
week *after* the release.

pws