From cafe33f048680e9b63e28ed0013c812c13b22863 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 16 May 2020 20:06:49 +0100
Subject: [PATCH] Fix 'test -t 1' in $(command substitutions)

Standard output (FD 1) tested as being on a terminal within a
command substitution, which makes no sense as the command
substitution is supposed to be catching standard output.
    ksh -c 'v=$(echo begincomsub
                [ -t 1 ] && echo oops
                echo endcomsub)
            echo "$v"'
This should not output "oops".

This is one of the many bugs with ksh93 virtual (non-forked)
subshells. On the abandoned Vashist/Rader ksh2020 branch, this bug
was fixed by changing quite a lot of code, which introduced and/or
exposed another bug:
	https://github.com/att/ast/issues/1079
	https://github.com/att/ast/commit/8e1e405e
	https://github.com/att/ast/issues/1088
That issue was unresolved when the ksh2020 branch was abandoned.

The safer and more conservative fix is simply forcing the subshell
to fork if we're in a non-forked command substitution and testing
'-t 1'. It is hard to imagine a situation where this would cause a
noticable performance hit.

Note that this fix does not affect ksh93-specific "shared"
non-subshell ${ command substitutions; } which are executed in the
main shell environment, so that variables survive, etcetera.
'test -t 1' continues to wrongly return true there, but command
substitutions of that form cannot be forked because that would
defeat their purpose.

src/cmd/ksh93/bltins/test.c:
- Fix 'test -t 1', '[ -t 1 ]' and '[[ -t 1 ]]' by forking the
  current subshell if it is a virtual/non-forked subshell
  (shp->subshell), and a command substitution (shp->comsub), but
  NOT a "shared" ${ command substitution; } (!shp->subshare).

src/cmd/ksh93/tests/bracket.sh:
- Add two regression tests for this issue, which were adapted from
  the Vashist/Rader ksh2020 branch.

NEWS, src/cmd/ksh93/include/version.h:
- Update.

(cherry picked from commit b8ef05e457ead65b83417699b8dd8632f855e2fa)
---
 NEWS                            | 12 ++++++++++++
 src/cmd/ksh93/bltins/test.c     |  3 +++
 src/cmd/ksh93/include/version.h |  2 +-
 src/cmd/ksh93/tests/bracket.sh  | 15 +++++++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index b78d45831..e12a6848f 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,18 @@ For full details, see the git log at:
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-05-16:
+
+- Fix 'test -t 1', '[ -t 1 ]', '[[ -t 1 ]]' in command substitutions.
+  Standard output (file descriptor 1) tested as being on a terminal within a
+  command substitution, which makes no sense as the command substitution is
+  supposed to be catching standard output.
+	v=$(echo begincomsub
+	    [ -t 1 ] && echo oops
+	    echo endcomsub)
+	echo "$v"
+  This now does not output "oops".
+
 2020-05-14:
 
 - Fix syncing history when print -s -f is used. For example, the
diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c
index 9a3bd59f5..e382653c5 100644
--- a/src/cmd/ksh93/bltins/test.c
+++ b/src/cmd/ksh93/bltins/test.c
@@ -425,6 +425,9 @@ int test_unop(Shell_t *shp,register int op,register const char *arg)
 	    {
 		char *last;
 		op = strtol(arg,&last, 10);
+		/* To make '-t 1' work in a $(comsub), fork. https://github.com/att/ast/issues/1079 */
+		if (op == 1 && shp->subshell && shp->comsub && !shp->subshare)
+			sh_subfork();
 		return(*last?0:tty_check(op));
 	    }
 	    case 'v':
diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h
index 8ccada637..9594b91df 100644
--- a/src/cmd/ksh93/include/version.h
+++ b/src/cmd/ksh93/include/version.h
@@ -17,4 +17,4 @@
 *                  David Korn <dgk@research.att.com>                   *
 *                                                                      *
 ***********************************************************************/
-#define SH_RELEASE	"93u+m 2020-05-14"
+#define SH_RELEASE	"93u+m 2020-05-16"
diff --git a/src/cmd/ksh93/tests/bracket.sh b/src/cmd/ksh93/tests/bracket.sh
index 39937adb7..8be30ed77 100755
--- a/src/cmd/ksh93/tests/bracket.sh
+++ b/src/cmd/ksh93/tests/bracket.sh
@@ -356,4 +356,19 @@ test ! ! ! 2> /dev/null || err_exit 'test ! ! ! should return 0'
 test ! ! x 2> /dev/null || err_exit 'test ! ! x should return 0'
 test ! ! '' 2> /dev/null && err_exit 'test ! ! "" should return non-zero'
 
+# ======
+# Verify that [ -t 1 ] behaves sensibly inside a command substitution.
+
+# This is the simple case that doesn't do any redirection of stdout within the command
+# substitution. Thus the [ -t 1 ] test should be false.
+var=$(echo begin; { [ -t 1 ] || test -t 1 || [[ -t 1 ]]; } && echo -t 1 is true; echo end)
+[[ $var == $'begin\nend' ]] || err_exit "test -t 1 in comsub fails"
+
+# This is the more complex case that does redirect stdout within the command substitution to the
+# actual tty. Thus the [ -t 1 ] test should be true.
+var=$(echo begin; exec >/dev/tty; [ -t 1 ] && test -t 1 && [[ -t 1 ]]) \
+&& [[ $var == $'begin' ]] \
+|| err_exit "test -t 1 in comsub with exec >/dev/tty fails"
+
+# ======
 exit $((Errors<125?Errors:125))