Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backticks nested in $( ) result in misdirected output #478

Closed
sckendall2 opened this issue Apr 17, 2018 · 8 comments · Fixed by #482
Closed

Backticks nested in $( ) result in misdirected output #478

sckendall2 opened this issue Apr 17, 2018 · 8 comments · Fixed by #482
Labels

Comments

@sckendall2
Copy link

In the following, either the /bin/echo command or the first print command must be sending their output directly to the script's stdout:

$ foo=$(print `/bin/echo bar`)   # should print nothing
bar
$ print foo=$foo                 # should print "foo=bar"
foo=
$ 

This bug is in the latest code, fetched earlier today. It's a regression from older versions of ksh93, such as the one that ships with RHEL 7.

@krader1961
Copy link
Contributor

Note that this bug exists in the 2016-01-10-beta branch. It's not something introduced in the master branch after we forked from the beta branch and changed the build system.

@krader1961 krader1961 added the bug label Apr 17, 2018
@sckendall2
Copy link
Author

By "the one that ships with RHEL 7" I mean Version AJM 93u+ 2012-08-01, RPM ksh-20120801-22.el7_1.2.x86_64, on RHEL 7.2. That version of ksh93 doesn't have the bug. Nor do ksh93 versions and other KornShells on a wide variety of Linux and other OSes.

@siteshwar
Copy link
Contributor

@sckendall2 We have built current development version from the last beta release by changing the build system, reformatting code etc. @krader1961's comment means that this bug was not introduced by those changes. It helps us in narrowing down how the regression got introduced.

@sckendall2
Copy link
Author

@siteshwar Whoops, I didn't mean my comment as a response to @krader1961. I started my comment before his appeared. I was just clarifying my initial post.

@siteshwar
Copy link
Contributor

fwiw this works as expected:

x=$(print $(/bin/echo bar))

ksh handles input output for $() and `` differently. We have seen multiple issues with each of these syntaxes in past. Unfortunately the code to handle input/output in subshells is too complicated and it's very likely that fixing each bug will create another regression.

@McDutchie
Copy link
Contributor

An effective workaround is forcing ksh93 to fork off the command substitution subshell in a separate process by including a dummy redirection in it:

$ foo=$( : 1>&1; print `/bin/echo bar`)
$ echo "$foo"
bar

@krader1961
Copy link
Contributor

Unfortunately the code to handle input/output in subshells is too complicated and it's very likely that fixing each bug will create another regression.

Agreed. The backtick substitution syntax has been deprecated for more than two decades and has lots of quirks. So unless the fix is obviously correct (trivial would be even better) it's probably not going to happen.

siteshwar added a commit that referenced this issue Apr 19, 2018
There was a regression introduced after the last stable release which
causes mishandling of redirections in subshells. For e.g.

foo=$(print `/bin/echo bar`)
print foo=$foo

prints `bar` on stdout and the variable foo is empty.

Handling of redirections in $() style of command substitution was
changed after the last stable release. sh_subshell() function replaces
sfstdout with a newly created temporary stream with this line :

sp->saveout = sfswap(sfstdout, NULL);

The newly created stream has it's fd value set to -1. If `` style of
command subtitution happens inside $(), it tries to use this stream with
invalid fd and fails to close it. So redirections for stdout do not
work.

We should check if the stream that we are closing actually corresponds
to the file descriptor that we want to close. If not, directly invoke
close() on the fd.

Resolves: #478
siteshwar added a commit that referenced this issue Apr 19, 2018
siteshwar added a commit that referenced this issue Apr 19, 2018
sfio provides an api to get file descritptor of a stream. Use it instead
of directly accessing private member of Sfio_t struct.

Related: #478
@siteshwar
Copy link
Contributor

@sckendall2 I have merged a fix for this. If you want to use latest development version of ksh on RHEL, you can subscribe to this copr repository. If you see any issues, please report it with output of rpm -q ksh. We appreciate any testing performed on the development builds. Thanks!

McDutchie added a commit to ksh93/ksh that referenced this issue Jun 15, 2020
ksh 93v- beta introduced a regression with nested command
substitutions: backticks nested in $( ) result in misdirected
output. This has never been in 93u+, but since we're often
backporting things, let's avoid backporting this bug. It is also
useful if this shows up when running our bin/shtests against the
actual beta by adding a SHELL=... argument.
Ref.: att#478

src/cmd/ksh93/tests/subshell.sh:
- Add reproducer submitted by the reporter as a regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants