1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Isolate ARGV in Rails::Command.invoke

Follow-up to #38463.

By isolating ARGV, we guard against commands inadvertently depending on
prior ARGV contents.  Any such command will now behave consistently when
run via `Rails::Command.invoke`, whether coming from the Rails CLI or
from library code.  Likewise, any ARGV mutations done by a command will
not affect code that executes after `Rails::Command.invoke`.
This commit is contained in:
Jonathan Hefner 2020-02-17 23:59:06 -06:00
parent 373ad2c624
commit 8ec7a2b7aa
3 changed files with 34 additions and 10 deletions

View file

@ -40,12 +40,19 @@ module Rails
command_name, namespace = "help", "help" if command_name.blank? || HELP_MAPPINGS.include?(command_name)
command_name, namespace = "version", "version" if %w( -v --version ).include?(command_name)
# isolate ARGV to ensure that commands depend only on the args they are given
args = args.dup # args might *be* ARGV so dup before clearing
old_argv = ARGV.dup
ARGV.clear
command = find_by_namespace(namespace, command_name)
if command && command.all_commands[command_name]
command.perform(command_name, args, config)
else
find_by_namespace("rake").perform(full_namespace, args, config)
end
ensure
ARGV.replace(old_argv)
end
# Rails finds namespaces similar to Thor, it only adds one rule:

View file

@ -172,20 +172,19 @@ module ApplicationTests
assert File.exist?(File.join(rails_root, "app/views/notifier_mailer/foo.html.erb"))
end
test "ARGV is mutated as expected" do
test "ARGV is populated" do
require "#{app_path}/config/environment"
require "rails/command"
Rails::Command.const_set("APP_PATH", "rails/all")
Rails.application.load_generators
FileUtils.cd(rails_root) do
ARGV = ["mailer", "notifier", "foo"]
Rails::Command.const_set("ARGV", ARGV)
quietly { Rails::Command.invoke :generate, ARGV }
assert_equal ["notifier", "foo"], ARGV
class Rails::Generators::CheckArgvGenerator < Rails::Generators::Base
def check_expected
raise "ARGV.first is not expected" unless ARGV.first == "expected"
end
end
Rails::Command.send(:remove_const, "APP_PATH")
quietly do
Rails::Command.invoke(:generate, ["check_argv", "expected"]) # should not raise
end
end
test "help does not show hidden namespaces and hidden commands" do

View file

@ -12,4 +12,22 @@ class Rails::Command::BaseTest < ActiveSupport::TestCase
assert_equal %w(secrets:setup secrets:edit secrets:show), Rails::Command::SecretsCommand.printing_commands
assert_equal %w(db:system:change), Rails::Command::Db::System::ChangeCommand.printing_commands
end
test "ARGV is isolated" do
class Rails::Command::ArgvCommand < Rails::Command::Base
def check_isolation
raise "not isolated" unless ARGV.empty?
ARGV << "isolate this"
end
end
old_argv = ARGV.dup
new_argv = ["foo", "bar"]
ARGV.replace(new_argv)
Rails::Command.invoke("argv:check_isolation") # should not raise
assert_equal new_argv, ARGV
ensure
ARGV.replace(old_argv)
end
end