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

shell_(parse|split|escape): fix some issues with deprecation #19985

Merged
merged 2 commits into from
Jan 12, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions base/managers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ function launch_on_machine(manager::SSHManager, machine, cnt, params, launched,
cmd = `cd $dir '&&' $tval $exename $exeflags`

# shell login (-l) with string command (-c) to launch julia process
cmd = `sh -l -c $(shell_escape(cmd, special = ""))`
cmd = `sh -l -c $(shell_escape(cmd))`

# remote launch with ssh with given ssh flags / host / port information
# -T → disable pseudo-terminal allocation
Expand All @@ -195,7 +195,7 @@ function launch_on_machine(manager::SSHManager, machine, cnt, params, launched,
# forwarded connections are causing collisions
# -n → Redirects stdin from /dev/null (actually, prevents reading from stdin).
# Used when running ssh in the background.
cmd = `ssh -T -a -x -o ClearAllForwardings=yes -n $sshflags $host $(shell_escape(cmd, special = ""))`
cmd = `ssh -T -a -x -o ClearAllForwardings=yes -n $sshflags $host $(shell_escape(cmd))`

# launch the remote Julia process

Expand Down
6 changes: 3 additions & 3 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ end
hash(x::AndCmds, h::UInt) = hash(x.a, hash(x.b, h))
==(x::AndCmds, y::AndCmds) = x.a == y.a && x.b == y.b

shell_escape(cmd::Cmd; special::AbstractString=shell_special) =
shell_escape(cmd::Cmd; special::AbstractString="") =
shell_escape(cmd.exec..., special=special)

function show(io::IO, cmd::Cmd)
print_env = cmd.env !== nothing
print_dir = !isempty(cmd.dir)
(print_env || print_dir) && print(io, "setenv(")
esc = shell_escape(cmd)
esc = shell_escape(cmd, special=shell_special)
print(io, '`')
for c in esc
if c == '`'
Expand Down Expand Up @@ -797,7 +797,7 @@ function cmd_gen(parsed)
end

macro cmd(str)
return :(cmd_gen($(shell_parse(str)[1])))
return :(cmd_gen($(shell_parse(str, special=shell_special)[1])))
end

wait(x::Process) = if !process_exited(x); stream_wait(x, x.exitnotify); end
Expand Down
34 changes: 21 additions & 13 deletions base/shell.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@

const shell_special = "#{}()[]<>|&*?~;"

function shell_parse(str::AbstractString, interpolate::Bool=true)
# needs to be factored out so depwarn only warns once
@noinline warn_shell_special(special) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the leading spaces?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

editor playing tricks on me?

depwarn("special characters \"$special\" should now be quoted in commands", :warn_shell_special)

function shell_parse(
str::AbstractString,
interpolate::Bool=true;
special::AbstractString=""
)
s = lstrip(str)
# strips the end but respects the space when the string ends with "\\ "
r = RevString(s)
Expand Down Expand Up @@ -94,8 +102,8 @@ function shell_parse(str::AbstractString, interpolate::Bool=true)
update_arg(s[i:j-1]); i = k
c, k = next(s,k)
end
elseif !in_single_quotes && !in_double_quotes && c in shell_special
depwarn("special characters \"$shell_special\" should now be quoted in commands", :shell_parse)
elseif !in_single_quotes && !in_double_quotes && c in special
warn_shell_special(special) # noinline depwarn
end
j = k
end
Expand All @@ -118,15 +126,15 @@ function shell_parse(str::AbstractString, interpolate::Bool=true)
end

function shell_split(s::AbstractString)
parsed = shell_parse(s,false)[1]
parsed = shell_parse(s, false)[1]
args = AbstractString[]
for arg in parsed
push!(args, string(arg...))
end
args
end

function print_shell_word(io::IO, word::AbstractString, special::AbstractString = shell_special)
function print_shell_word(io::IO, word::AbstractString, special::AbstractString = "")
if isempty(word)
print(io, "''")
end
Expand Down Expand Up @@ -158,30 +166,30 @@ end

function print_shell_escaped(
io::IO, cmd::AbstractString, args::AbstractString...;
special::AbstractString=shell_special
special::AbstractString=""
)
print_shell_word(io, cmd, special)
for arg in args
print(io, ' ')
print_shell_word(io, arg, special)
end
end
print_shell_escaped(io::IO; special::String=shell_special) = nothing
print_shell_escaped(io::IO; special::String="") = nothing

"""
shell_escape(args::Union{Cmd,AbstractString...}; special::AbstractString="$shell_special")
shell_escape(args::Union{Cmd,AbstractString...}; special::AbstractString="")

The unexported `shell_escape` function is the inverse of the unexported `shell_split` function:
it takes a string or command object and escapes any special characters in such a way that calling
`shell_split` on it would give back the array of words in the original command. The `special`
keyword argument controls what characters in addition to whitespace, backslashes, quotes and
dollar signs are considered to be special. Examples:
dollar signs are considered to be special (default: none). Examples:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"in addition" isn't accurate any more then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is: whitespace, backslashes, quotes and dollar signs are always considered special; the special keyword allows you to supply a string of characters that are also considered special.


julia> Base.shell_escape("cat", "/foo/bar baz", "&&", "echo", "done")
"cat '/foo/bar baz' && echo done"

julia> Base.shell_escape("echo", "this", "&&", "that")
"echo this '&&' that"
Copy link
Contributor

@tkelman tkelman May 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer displays with quotes around the &&, and these should be doctests so this would have been noticed sooner

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a PR to fix it.


julia> Base.shell_escape("cat", "/foo/bar baz", "&&", "echo", "done", special="")
"cat '/foo/bar baz' && echo done"
"""
shell_escape(args::AbstractString...; special::AbstractString=shell_special) =
shell_escape(args::AbstractString...; special::AbstractString="") =
sprint(io->print_shell_escaped(io, args..., special=special))