From 07f8a96aa14b642a8641dcb22dad07f995d3917e Mon Sep 17 00:00:00 2001
From: starbelly <starbelly@pobox.com>
Date: Sat, 1 Aug 2015 16:48:56 -0500
Subject: Add run_cmd class method to ActiveRecord::Tasks::DatabaseTasks
- Added run_cmd() class method to dry up Kernel.system() messages within
this namespace and avoid shell expansion by passing a list of
arguments instead of a string
- Update structure_dump, structure_load, and related tests units to
pass a list of params instead of using a string to
avoid shell expansion
---
activerecord/CHANGELOG.md | 8 ++++++++
.../lib/active_record/tasks/database_tasks.rb | 13 ++++++++++++
.../tasks/postgresql_database_tasks.rb | 23 +++++++++++-----------
.../test/cases/tasks/postgresql_rake_test.rb | 12 +++++------
4 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 42b8515133..5c76319f47 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+* Added run_cmd class method to ActiveRecord::Tasks::DatabaseTasks for
+ drying up Kernel.system() calls within this namespace and to avoid
+ shell expansion by using a paramter list instead of string as arguments
+ for Kernel.system(). Thanks to Nate Berkopec for supply patch to get
+ test units passing.
+
+ *Bryan Paxton*
+
* Properly allow uniqueness validations on primary keys.
Fixes #20966.
diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb
index 683741768b..94a8116b8b 100644
--- a/activerecord/lib/active_record/tasks/database_tasks.rb
+++ b/activerecord/lib/active_record/tasks/database_tasks.rb
@@ -252,6 +252,19 @@ module ActiveRecord
end
end
+ def self.run_cmd(cmd, args, action)
+ fail run_cmd_error(cmd, args, action) unless Kernel.system(cmd, *args)
+ end
+
+ def run_cmd_error(cmd, args, action)
+ msg = "failed to execute:\n"
+ msg << "#{cmd} #{args.join(' ')}\n\n"
+ msg << "Please check that `#{cmd}` is :\n"
+ msg << " - present on this system\n"
+ msg << " - in your PATH\n"
+ msg << " - has proper permissions\n\n"
+ end
+
private
def class_for_adapter(adapter)
diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb
index d7da95c8a9..d406f24c31 100644
--- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb
+++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb
@@ -1,5 +1,3 @@
-require 'shellwords'
-
module ActiveRecord
module Tasks # :nodoc:
class PostgreSQLDatabaseTasks # :nodoc:
@@ -55,19 +53,22 @@ module ActiveRecord
when String
ActiveRecord::Base.dump_schemas
end
- unless search_path.blank?
- search_path = search_path.split(",").map{|search_path_part| "--schema=#{Shellwords.escape(search_path_part.strip)}" }.join(" ")
- end
-
- command = "pg_dump -i -s -x -O -f #{Shellwords.escape(filename)} #{search_path} #{Shellwords.escape(configuration['database'])}"
- raise 'Error dumping database' unless Kernel.system(command)
+ args = ['-i', '-s', '-x', '-O', '-f', filename]
+ unless search_path.blank?
+ args << search_path.split(',').map do |part|
+ "--schema=#{part.strip}"
+ end.join(' ')
+ end
+ args << configuration['database']
+ ActiveRecord::Tasks::DatabaseTasks.run_cmd('pg_dump', args, 'dumping')
File.open(filename, "a") { |f| f << "SET search_path TO #{connection.schema_search_path};\n\n" }
end
- def structure_load(filename)
- set_psql_env
- Kernel.system("psql -X -q -f #{Shellwords.escape(filename)} #{configuration['database']}")
+ def structure_load(filename)
+ set_psql_env
+ args = [ '-q', '-f', filename, configuration['database'] ]
+ ActiveRecord::Tasks::DatabaseTasks.run_cmd('psql', args, 'loading' )
end
private
diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb
index 084302cde5..184ff7fc63 100644
--- a/activerecord/test/cases/tasks/postgresql_rake_test.rb
+++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb
@@ -204,7 +204,7 @@ module ActiveRecord
end
def test_structure_dump
- Kernel.expects(:system).with("pg_dump -i -s -x -O -f #{@filename} my-app-db").returns(true)
+ Kernel.expects(:system).with('pg_dump', '-i', '-s', '-x', '-O', '-f', @filename, 'my-app-db').returns(true)
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
end
@@ -212,7 +212,7 @@ module ActiveRecord
def test_structure_dump_with_schema_search_path
@configuration['schema_search_path'] = 'foo,bar'
- Kernel.expects(:system).with("pg_dump -i -s -x -O -f #{@filename} --schema=foo --schema=bar my-app-db").returns(true)
+ Kernel.expects(:system).with('pg_dump', '-i', '-s', '-x', '-O', '-f', @filename, '--schema=foo --schema=bar', 'my-app-db').returns(true)
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
end
@@ -220,7 +220,7 @@ module ActiveRecord
def test_structure_dump_with_schema_search_path_and_dump_schemas_all
@configuration['schema_search_path'] = 'foo,bar'
- Kernel.expects(:system).with("pg_dump -i -s -x -O -f #{@filename} my-app-db").returns(true)
+ Kernel.expects(:system).with("pg_dump", '-i', '-s', '-x', '-O', '-f', @filename, 'my-app-db').returns(true)
with_dump_schemas(:all) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@@ -228,7 +228,7 @@ module ActiveRecord
end
def test_structure_dump_with_dump_schemas_string
- Kernel.expects(:system).with("pg_dump -i -s -x -O -f #{@filename} --schema=foo --schema=bar my-app-db").returns(true)
+ Kernel.expects(:system).with("pg_dump", '-i', '-s', '-x', '-O', '-f', @filename, '--schema=foo --schema=bar', "my-app-db").returns(true)
with_dump_schemas('foo,bar') do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@@ -261,14 +261,14 @@ module ActiveRecord
def test_structure_load
filename = "awesome-file.sql"
- Kernel.expects(:system).with("psql -X -q -f #{filename} my-app-db")
+ Kernel.expects(:system).with('psql', '-q', '-f', filename, @configuration['database']).returns(true)
ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename)
end
def test_structure_load_accepts_path_with_spaces
filename = "awesome file.sql"
- Kernel.expects(:system).with("psql -X -q -f awesome\\ file.sql my-app-db")
+ Kernel.expects(:system).with('psql', '-q', '-f', filename, @configuration['database']).returns(true)
ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename)
end
--
cgit v1.2.3