Skip to content

Commit

Permalink
Avoid escaping paths when editing credentials
Browse files Browse the repository at this point in the history
`Shellwords.escape` escapes unquoted spaces with a backslash, but
Windows does not treat backslash as an escape character.  Escaping is
also a problem when paths are expressed in shortened 8.3 format (e.g.
`C:\Users\RubyOn~1\AppData\Local\Temp\...`) because a backslash will be
erroneously added before the `~`.

We can avoid the need to escape by using `system(command_name, *args)`
instead of `system(command_line)`, but we must still support
`ENV["EDITOR"]` values that embed command line arguments, such as
`subl -w`.

This commit changes to `system(command_name, *args)`, but uses
`Shellwords.split` to extract any embedded arguments from
`ENV["EDITOR"]`.  This requires that Windows users put quotes around the
entire path of their editor if it contains spaces, such as:

```
SET EDITOR="C:\Program Files\Microsoft VS Code\Code.exe" -w
```

In other words, the following are **not** supported on Windows:

```
SET "EDITOR=C:\Program Files\Microsoft VS Code\Code.exe"
SET EDITOR=C:\Program Files\Microsoft VS Code\Code.exe
SET EDITOR=C:\"Program Files"\"Microsoft VS Code"\Code.exe -w
SET EDITOR=C:\Program^ Files\Microsoft^ VS^ Code\Code.exe -w
SET EDITOR=C:\Program` Files\Microsoft` VS` Code\Code.exe -w
```

Fixes #41617 (again).
Closes #44890.
  • Loading branch information
jonathanhefner authored and guilleiguaran committed Jun 11, 2023
1 parent b2ec76d commit 30a287a
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Expand Up @@ -92,7 +92,7 @@ def ensure_credentials_have_been_added

def change_credentials_in_system_editor
credentials.change do |tmp_path|
system("#{ENV["EDITOR"]} #{Shellwords.escape(tmp_path)}")
system(*Shellwords.split(ENV["EDITOR"]), tmp_path.to_s)
end
end

Expand Down
2 changes: 1 addition & 1 deletion railties/test/commands/credentials_test.rb
Expand Up @@ -37,7 +37,7 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
end

test "edit command does not overwrite by default if credentials already exists" do
run_edit_command(editor: "eval echo api_key: abc >")
run_edit_command(editor: 'ruby -e "File.write ARGV[0], %(api_key: abc)"')
assert_match(/api_key: abc/, run_show_command)

run_edit_command
Expand Down

0 comments on commit 30a287a

Please sign in to comment.