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

REPL shell mode calls the shell with incorrectly escaped arguments #10120

Closed
vtjnash opened this issue Feb 8, 2015 · 12 comments · Fixed by #23881
Closed

REPL shell mode calls the shell with incorrectly escaped arguments #10120

vtjnash opened this issue Feb 8, 2015 · 12 comments · Fixed by #23881
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version REPL Julia's REPL (Read Eval Print Loop) system:linux Affects only Linux

Comments

@vtjnash
Copy link
Member

vtjnash commented Feb 8, 2015

shell> ./julia -E "println(ARGS)"
bash: syntax error near unexpected token `('

shell> ./julia -E \"println(ARGS)\"
"println(ARGS)"

shell> ./julia -E println\(ARGS\)
bash: syntax error near unexpected token `('
@vtjnash vtjnash added bug Indicates an unexpected problem or unintended behavior system:linux Affects only Linux REPL Julia's REPL (Read Eval Print Loop) regression Regression in behavior compared to a previous version labels Feb 8, 2015
@vtjnash
Copy link
Member Author

vtjnash commented Sep 17, 2015

@StefanKarpinski #4635 seems to have had a long tail of bug fixes, but it still doesn't work work for any non-trivial argument list (see above). Is there any chance of fixing this behavior? Or is nobody actually using this feature?

Brainstorming with Jeff, it seems we might need a custom command parser that handles $ and \$, but leaves ' and " alone, and wraps the results of interpolating any values in quotes and then calls Base.shell_escape (after accounting for iterables), unless the result is a Cmd in which case it shouldn't wrap the value in quotes before calling Base.shell_escape. Have I missed anything?

ping other possible interested parties from looking at who's contributed to this function: @stevengj @staticfloat @Keno @JeffBezanson

since nobody else seems to notice this, perhaps we should just reclaim the ; key? I'm pretty sure I could find some new uses for it.

@staticfloat
Copy link
Member

I use ; all the time, but I guess I've just never tried to run shell commands with parens; I'm in favor of fixing this properly. I'm too busy right now to dig into the problematic behavior, but as finnicky as quoting rules are, I think we will be able to arrive on the "proper" behavior.

@vtjnash
Copy link
Member Author

vtjnash commented Sep 17, 2015

do you just run simple commands like ls and cd? we probably don't need to be invoking the shell just to handle those.

@staticfloat
Copy link
Member

I tend to use pipes and redirects pretty often as well, so the shell mode is definitely useful.

@tkelman
Copy link
Contributor

tkelman commented Sep 18, 2015

Please ask for a second opinion before adding the backport pending tag on anything non-trivial.

@eschnett
Copy link
Contributor

@tkelman Apologies. Removed.

@tkelman
Copy link
Contributor

tkelman commented Sep 18, 2015

We can maybe put it back, but interactive shell-mode related changes need a lot of hands-on testing time and are at higher-than-usual risk of platform differences so I don't want to rush things between RC and final. Maybe post-0.4.0.

@lkuper
Copy link

lkuper commented Oct 20, 2015

I think I just stumbled over this bug while trying to use run:

julia> run(`./example.sh`)
./example.sh: 2: ./example.sh: Syntax error: "(" unexpected
ERROR: failed process: Process(`./example.sh`, ProcessExited(2)) [2]
 in run at ./process.jl:531

where example.sh is essentially the following:

var1="foo:bar"
var2=(${var1//:/ })
echo $var2

It works fine run from the command line with bash:

$ ./example.sh 
foo

Incidentally, Syntax error: "(" unexpected is also the error raised by the above script when run with dash, not bash, and so at first I thought that Julia was somehow calling out to dash!

(Edit: turns out we can work around the bug by calling run(./example-wrapper.sh) where example-wrapper.sh is just a script that calls ./example.sh. So this isn't a showstopper.)

(Edit 2: On second thought, the issue was that there was a comment in the file before #!/bin/bash. It worked fine from the command line, and not from within Julia.)

@tkelman
Copy link
Contributor

tkelman commented Oct 20, 2015

what is the shebang line set to in the script? julia isnt calling the shell, it's calling uv_spawn

@vtjnash
Copy link
Member Author

vtjnash commented Oct 20, 2015

please do not hijack this issue to discuss unrelated questions. use Stack-Overflow or julia-users instead.

@vtjnash vtjnash changed the title shell mode calls the shell with incorrectly escaped arguments REPL shell mode calls the shell with incorrectly escaped arguments Oct 20, 2015
@lkuper
Copy link

lkuper commented Oct 20, 2015

@vtjnash Sorry. I wouldn't have brought it up if I didn't think my issue was the same fundamental issue.

@ihnorton
Copy link
Member

@lkuper no worries, and thanks for using Julia. The shell situation still clearly has a few rough edges.

@StefanKarpinski StefanKarpinski self-assigned this Jun 17, 2017
@vtjnash vtjnash removed their assignment Sep 21, 2017
vtjnash added a commit that referenced this issue Sep 26, 2017
ensures that arguments will be passed through a posix shell correctly,
(including those such as being added in #21849 for -C)

fix #10120
vtjnash added a commit that referenced this issue Sep 26, 2017
ensures that arguments will be passed through a posix shell correctly,
(including those such as being added in #21849 for -C)

fix #10120
vtjnash added a commit that referenced this issue Sep 26, 2017
ensures that arguments will be passed through a posix shell correctly,
(including those such as being added in #21849 for -C)

fix #10120
vtjnash added a commit that referenced this issue Oct 12, 2017
ensures that arguments will be passed through a posix shell correctly,
(including those such as being added in #21849 for -C)

fix #10120
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version REPL Julia's REPL (Read Eval Print Loop) system:linux Affects only Linux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants