From 2d68286d40aa01bc6c5132d560a4a496316a3e2f Mon Sep 17 00:00:00 2001 From: Claire Foster Date: Sun, 28 May 2023 07:23:10 +1000 Subject: [PATCH] Fix string escaping in REPL completion of paths REPL completion of paths within strings need to be escaped according to the usual escaping rules, and delimited by the starting " rather than whitespace. This differs from completion of paths within cmd backticks which need to be escaped according to shell escaping rules. Separate these cases and fix string escaping. This was found because JuliaSyntax emits an Expr(:error) rather than Expr(:incomplete) for paths inside strings with invalid escape sequences before whitespace. --- stdlib/REPL/src/REPLCompletions.jl | 58 +++++++++++++++++------ stdlib/REPL/test/replcompletions.jl | 71 ++++++++++++++++++++++------- 2 files changed, 98 insertions(+), 31 deletions(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 6ec7074f105fd..20d26953eb22b 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -232,7 +232,10 @@ function complete_keyword(s::Union{String,SubString{String}}) Completion[KeywordCompletion(kw) for kw in sorted_keywords[r]] end -function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_escape=false) +function complete_path(path::AbstractString, pos::Int; + use_envpath=false, shell_escape=false, + string_escape=false) + @assert !(shell_escape && string_escape) if Base.Sys.isunix() && occursin(r"^~(?:/|$)", path) # if the path is just "~", don't consider the expanded username as a prefix if path == "~" @@ -259,9 +262,9 @@ function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_ matches = Set{String}() for file in files if startswith(file, prefix) - id = try isdir(joinpath(dir, file)) catch; false end - # joinpath is not used because windows needs to complete with double-backslash - push!(matches, id ? file * (@static Sys.iswindows() ? "\\\\" : "/") : file) + p = joinpath(dir, file) + is_dir = try isdir(p) catch; false end + push!(matches, is_dir ? joinpath(file, "") : file) end end @@ -307,8 +310,14 @@ function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_ end end - matchList = Completion[PathCompletion(shell_escape ? replace(s, r"\s" => s"\\\0") : s) for s in matches] - startpos = pos - lastindex(prefix) + 1 - count(isequal(' '), prefix) + function do_escape(s) + return shell_escape ? replace(s, r"(\s|\\)" => s"\\\0") : + string_escape ? escape_string(s, ('\"','$')) : + s + end + + matchList = Completion[PathCompletion(do_escape(s)) for s in matches] + startpos = pos - lastindex(do_escape(prefix)) + 1 # The pos - lastindex(prefix) + 1 is correct due to `lastindex(prefix)-lastindex(prefix)==0`, # hence we need to add one to get the first index. This is also correct when considering # pos, because pos is the `lastindex` a larger string which `endswith(path)==true`. @@ -767,7 +776,7 @@ end function close_path_completion(str, startpos, r, paths, pos) length(paths) == 1 || return false # Only close if there's a single choice... _path = str[startpos:prevind(str, first(r))] * (paths[1]::PathCompletion).path - path = expanduser(replace(_path, r"\\ " => " ")) + path = expanduser(unescape_string(replace(_path, "\\\$"=>"\$", "\\\""=>"\""))) # ...except if it's a directory... try isdir(path) @@ -1039,23 +1048,44 @@ function completions(string::String, pos::Int, context_module::Module=Main, shif dotpos = something(findprev(isequal('.'), string, first(varrange)-1), 0) return complete_identifiers!(Completion[], ffunc, context_module, string, string[startpos:pos], pos, dotpos, startpos) - # otherwise... - elseif inc_tag in [:cmd, :string] + elseif inc_tag === :cmd m = match(r"[\t\n\r\"`><=*?|]| (?!\\)", reverse(partial)) startpos = nextind(partial, reverseind(partial, m.offset)) r = startpos:pos + # This expansion with "\\ "=>' ' replacement and shell_escape=true + # assumes the path isn't further quoted within the cmd backticks. expanded = complete_expanduser(replace(string[r], r"\\ " => " "), r) expanded[3] && return expanded # If user expansion available, return it - paths, r, success = complete_path(replace(string[r], r"\\ " => " "), pos) + paths, r, success = complete_path(replace(string[r], r"\\ " => " "), pos, + shell_escape=true) + + return sort!(paths, by=p->p.path), r, success + elseif inc_tag === :string + # Find first non-escaped quote + m = match(r"\"(?!\\)", reverse(partial)) + startpos = nextind(partial, reverseind(partial, m.offset)) + r = startpos:pos + + expanded = complete_expanduser(string[r], r) + expanded[3] && return expanded # If user expansion available, return it - if inc_tag === :string && close_path_completion(string, startpos, r, paths, pos) - paths[1] = PathCompletion((paths[1]::PathCompletion).path * "\"") + path_prefix = try + unescape_string(replace(string[r], "\\\$"=>"\$", "\\\""=>"\"")) + catch + nothing end + if !isnothing(path_prefix) + paths, r, success = complete_path(path_prefix, pos, string_escape=true) - #Latex symbols can be completed for strings - (success || inc_tag === :cmd) && return sort!(paths, by=p->p.path), r, success + if close_path_completion(string, startpos, r, paths, pos) + paths[1] = PathCompletion((paths[1]::PathCompletion).path * "\"") + end + + # Fallthrough allowed so that Latex symbols can be completed in strings + success && return sort!(paths, by=p->p.path), r, success + end end ok, ret = bslash_completions(string, pos) diff --git a/stdlib/REPL/test/replcompletions.jl b/stdlib/REPL/test/replcompletions.jl index b0d1ff4b5237a..b2199e10bef55 100644 --- a/stdlib/REPL/test/replcompletions.jl +++ b/stdlib/REPL/test/replcompletions.jl @@ -1177,7 +1177,7 @@ let current_dir, forbidden catch e e isa Base.IOError && occursin("ELOOP", e.msg) end - c, r = test_complete("\"$(joinpath(path, "selfsym"))") + c, r = test_complete("\""*escape_string(joinpath(path, "selfsym"))) @test c == ["selfsymlink"] end end @@ -1207,26 +1207,62 @@ end mktempdir() do path space_folder = randstring() * " α" dir = joinpath(path, space_folder) - dir_space = replace(space_folder, " " => "\\ ") - mkdir(dir) cd(path) do - open(joinpath(space_folder, "space .file"),"w") do f - s = Sys.iswindows() ? "rm $dir_space\\\\space" : "cd $dir_space/space" - c, r = test_scomplete(s) - @test r == lastindex(s)-4:lastindex(s) - @test "space\\ .file" in c + touch(joinpath(space_folder, "space .file")) + + dir_space = replace(space_folder, " " => "\\ ") + s = Sys.iswindows() ? "cd $dir_space\\\\space" : "cd $dir_space/space" + c, r = test_scomplete(s) + @test s[r] == "space" + @test "space\\ .file" in c + # Also use shell escape rules within cmd backticks + s = "`$s" + c, r = test_scomplete(s) + @test s[r] == "space" + @test "space\\ .file" in c + + # escape string according to Julia escaping rules + julia_esc(str) = escape_string(str, ('\"','$')) + + # For normal strings the string should be properly escaped according to + # the usual rules for Julia strings. + s = "cd(\"" * julia_esc(joinpath(path, space_folder, "space")) + c, r = test_complete(s) + @test s[r] == "space" + @test "space .file\"" in c + + # '$' is the only character which can appear in a windows filename and + # which needs to be escaped in Julia strings (on unix we could do this + # test with all sorts of special chars) + touch(joinpath(space_folder, "needs_escape\$.file")) + escpath = julia_esc(joinpath(path, space_folder, "needs_escape\$")) + s = "cd(\"$escpath" + c, r = test_complete(s) + @test s[r] == "needs_escape\\\$" + @test "needs_escape\\\$.file\"" in c - s = Sys.iswindows() ? "cd(\"β $dir_space\\\\space" : "cd(\"β $dir_space/space" + if !Sys.iswindows() + touch(joinpath(space_folder, "needs_escape2\n\".file")) + escpath = julia_esc(joinpath(path, space_folder, "needs_escape2\n\"")) + s = "cd(\"$escpath" c, r = test_complete(s) - @test r == lastindex(s)-4:lastindex(s) - @test "space .file\"" in c + @test s[r] == "needs_escape2\\n\\\"" + @test "needs_escape2\\n\\\".file\"" in c + + touch(joinpath(space_folder, "needs_escape3\\.file")) + escpath = julia_esc(joinpath(path, space_folder, "needs_escape3\\")) + s = "cd(\"$escpath" + c, r = test_complete(s) + @test s[r] == "needs_escape3\\\\" + @test "needs_escape3\\\\.file\"" in c end + # Test for issue #10324 - s = "cd(\"$dir_space" + s = "cd(\"$space_folder" c, r = test_complete(s) - @test r == 5:15 - @test s[r] == dir_space + @test r == 5:14 + @test s[r] == space_folder #Test for #18479 for c in "'`@\$;&" @@ -1240,8 +1276,9 @@ mktempdir() do path @test c[1] == test_dir*(Sys.iswindows() ? "\\\\" : "/") @test res end - c, r, res = test_complete("\""*test_dir) - @test c[1] == test_dir*(Sys.iswindows() ? "\\\\" : "/") + escdir = julia_esc(test_dir) + c, r, res = test_complete("\""*escdir) + @test c[1] == escdir*(Sys.iswindows() ? "\\\\" : "/") @test res finally rm(joinpath(path, test_dir), recursive=true) @@ -1285,7 +1322,7 @@ if Sys.iswindows() @test r == length(s)-1:length(s) @test file in c - s = "cd(\"..\\" + s = "cd(\"..\\\\" c,r = test_complete(s) @test r == length(s)+1:length(s) @test temp_name * "\\\\" in c