Skip to content

Commit e956611

Browse files
authored
[core/process] Fix 2 redirect bugs (#723)
* [test/spec] Failing test cases for fd bugs. * [core/process] Fix fd leak after redirection failure. * [core/process] Fix a problem >&100 does not fail. * [core/process] Fix a bug that fd is not closed by {fd}>&-.
2 parents 2fd9816 + 48474aa commit e956611

File tree

2 files changed

+66
-1
lines changed

2 files changed

+66
-1
lines changed

core/process.py

+10-1
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,13 @@ def _PushDup(self, fd1, loc):
257257
# already returned 3, e.g. echo 3>out.txt
258258
return NO_FD
259259

260+
# Check the validity of fd1 before _PushSave(fd2)
261+
try:
262+
fcntl.fcntl(fd1, fcntl.F_GETFD)
263+
except IOError as e:
264+
self.errfmt.Print('%d: %s', fd1, posix.strerror(e.errno))
265+
raise
266+
260267
need_restore = self._PushSave(fd2)
261268

262269
#log('==== dup2 %s %s\n' % (fd1, fd2))
@@ -295,11 +302,12 @@ def _PushCloseFd(self, loc):
295302

296303
elif loc.tag_() == redir_loc_e.Fd:
297304
fd = cast(redir_loc__Fd, UP_loc).fd
298-
self._PushSave(fd)
299305

300306
else:
301307
raise AssertionError()
302308

309+
self._PushSave(fd)
310+
303311
return True
304312

305313
def _PushClose(self, fd):
@@ -441,6 +449,7 @@ def Push(self, redirects, waiter):
441449
try:
442450
self._ApplyRedirect(r, waiter)
443451
except (IOError, OSError) as e:
452+
self.Pop()
444453
return False # for bad descriptor, etc.
445454
finally:
446455
self.errfmt.PopLocation()

spec/redirect.test.sh

+56
Original file line numberDiff line numberDiff line change
@@ -551,3 +551,59 @@ hi
551551
hi
552552
hi
553553
## END
554+
555+
#### : >/dev/null 2> / (OSH regression: fail to pop fd frame)
556+
# oil 0.8.pre4 fails to restore fds after redirection failure. In the
557+
# following case, the fd frame remains after the redirection failure
558+
# "2> /" so that the effect of redirection ">/dev/null" remains after
559+
# the completion of the command.
560+
: >/dev/null 2> /
561+
echo hello
562+
## stdout: hello
563+
## OK dash stdout-json: ""
564+
## OK dash status: 2
565+
## OK mksh stdout-json: ""
566+
## OK mksh status: 1
567+
# dash/mksh terminates the execution of script on the redirection.
568+
569+
#### echo foo >&100 (OSH regression: does not fail with invalid fd 100)
570+
# oil 0.8.pre4 does not fail with non-existent fd 100.
571+
fd=100
572+
echo foo >&$fd
573+
## stdout-json: ""
574+
## status: 1
575+
## OK dash status: 2
576+
577+
#### echo foo >&N where N is first unused fd
578+
# 1. prepare default fd for internal uses
579+
minfd=10
580+
case ${SH##*/} in
581+
(mksh) minfd=24 ;;
582+
(osh) minfd=100 ;;
583+
esac
584+
585+
# 2. prepare first unused fd
586+
fd=$minfd
587+
is-fd-open() { : >&$1; }
588+
while is-fd-open "$fd"; do
589+
: $((fd+=1))
590+
done
591+
592+
# 3. test
593+
echo foo >&$fd
594+
## stdout-json: ""
595+
## status: 1
596+
## OK dash status: 2
597+
598+
#### exec {fd}>&- (OSH regression: fails to close fd)
599+
# mksh, dash do not implement {fd} redirections.
600+
case $SH in (mksh|dash) exit 1 ;; esac
601+
# oil 0.8.pre4 fails to close fd by {fd}&-.
602+
exec {fd}>file1
603+
echo foo >&$fd
604+
exec {fd}>&-
605+
echo bar >&$fd
606+
cat file1
607+
## stdout: foo
608+
## N-I mksh/dash stdout-json: ""
609+
## N-I mksh/dash status: 1

0 commit comments

Comments
 (0)