From ef0ea782b1f5cf7b08e74ea3002a16c708f66645 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 31 May 2008 16:57:46 -0700 Subject: Added SQL escaping for :limit and :offset [#288 state:closed] (Aaron Bedra, Steven Bristol, Jonathan Wiess) --- .../abstract/database_statements.rb | 9 +++++++-- activerecord/test/cases/adapter_test.rb | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 16d405d3bd..5358491cde 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -106,11 +106,16 @@ module ActiveRecord # SELECT * FROM suppliers LIMIT 10 OFFSET 50 def add_limit_offset!(sql, options) if limit = options[:limit] - sql << " LIMIT #{limit}" + sql << " LIMIT #{sanitize_limit(limit)}" if offset = options[:offset] - sql << " OFFSET #{offset}" + sql << " OFFSET #{offset.to_i}" end end + sql + end + + def sanitize_limit(limit) + limit.to_s[/,/] ? limit.split(',').map{ |i| i.to_i }.join(',') : limit.to_i end # Appends a locking clause to an SQL statement. diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 91504af901..c77446f880 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -104,4 +104,24 @@ class AdapterTest < ActiveRecord::TestCase end end + def test_add_limit_offset_should_sanitize_sql_injection_for_limit_without_comas + sql_inject = "1 select * from schema" + assert_equal " LIMIT 1", @connection.add_limit_offset!("", :limit=>sql_inject) + if current_adapter?(:MysqlAdapter) + assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + else + assert_equal " LIMIT 1 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + end + end + + def test_add_limit_offset_should_sanitize_sql_injection_for_limit_with_comas + sql_inject = "1, 7 procedure help()" + if current_adapter?(:MysqlAdapter) + assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) + assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + else + assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) + assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + end + end end -- cgit v1.2.3 From f6e921f9568d7f2e4807edf8728e6b0df8991816 Mon Sep 17 00:00:00 2001 From: "John D. Hume" Date: Wed, 28 May 2008 23:35:56 -0400 Subject: Substitute value into validates_format_of message Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/validations.rb | 2 +- activerecord/test/cases/validations_test.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 0e150bddb2..c97aafb126 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -692,7 +692,7 @@ module ActiveRecord raise(ArgumentError, "A regular expression must be supplied as the :with option of the configuration hash") unless configuration[:with].is_a?(Regexp) validates_each(attr_names, configuration) do |record, attr_name, value| - record.errors.add(attr_name, configuration[:message]) unless value.to_s =~ configuration[:with] + record.errors.add(attr_name, configuration[:message] % value) unless value.to_s =~ configuration[:with] end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index a4d9da4806..7b71647d25 100755 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -583,6 +583,12 @@ class ValidationsTest < ActiveRecord::TestCase assert_nil t.errors.on(:title) end + def test_validate_format_with_formatted_message + Topic.validates_format_of(:title, :with => /^Valid Title$/, :message => "can't be %s") + t = Topic.create(:title => 'Invalid title') + assert_equal "can't be Invalid title", t.errors.on(:title) + end + def test_validates_inclusion_of Topic.validates_inclusion_of( :title, :in => %w( a b c d e f g ) ) -- cgit v1.2.3 From f9db7695fe3c148c8d1077f1564e5b94d126b83b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 31 May 2008 17:03:03 -0700 Subject: Making ready for release of 2.1 --- activerecord/CHANGELOG | 6 ++++-- activerecord/Rakefile | 2 +- activerecord/lib/active_record/version.rb | 4 ++-- activerecord/test/connections/native_mysql/connection.rb | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 78082665a4..f901e432d5 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,6 +1,8 @@ -* Add first/last methods to associations/named_scope. Resolved #226. [Ryan Bates] +*2.1.0 (May 31st, 2008)* -*2.1.0 RC1 (May 11th, 2008)* +* Added SQL escaping for :limit and :offset #288 [Aaron Bedra, Steven Bristol, Jonathan Wiess] + +* Added first/last methods to associations/named_scope. Resolved #226. [Ryan Bates] * Ensure hm:t preloading honours reflection options. Resolves #137. [Frederick Cheung] diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 043ab6d551..fc068b16c3 100755 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -171,7 +171,7 @@ spec = Gem::Specification.new do |s| s.files = s.files + Dir.glob( "#{dir}/**/*" ).delete_if { |item| item.include?( "\.svn" ) } end - s.add_dependency('activesupport', '= 2.0.991' + PKG_BUILD) + s.add_dependency('activesupport', '= 2.1.0' + PKG_BUILD) s.files.delete FIXTURES_ROOT + "/fixture_database.sqlite" s.files.delete FIXTURES_ROOT + "/fixture_database_2.sqlite" diff --git a/activerecord/lib/active_record/version.rb b/activerecord/lib/active_record/version.rb index 1463e84764..aaadef9979 100644 --- a/activerecord/lib/active_record/version.rb +++ b/activerecord/lib/active_record/version.rb @@ -1,8 +1,8 @@ module ActiveRecord module VERSION #:nodoc: MAJOR = 2 - MINOR = 0 - TINY = 991 + MINOR = 1 + TINY = 0 STRING = [MAJOR, MINOR, TINY].join('.') end diff --git a/activerecord/test/connections/native_mysql/connection.rb b/activerecord/test/connections/native_mysql/connection.rb index 1fab444e58..3ee031f384 100644 --- a/activerecord/test/connections/native_mysql/connection.rb +++ b/activerecord/test/connections/native_mysql/connection.rb @@ -12,13 +12,13 @@ ActiveRecord::Base.logger = RAILS_DEFAULT_LOGGER ActiveRecord::Base.configurations = { 'arunit' => { :adapter => 'mysql', - :username => 'rails', + :username => 'root', :encoding => 'utf8', :database => 'activerecord_unittest', }, 'arunit2' => { :adapter => 'mysql', - :username => 'rails', + :username => 'root', :database => 'activerecord_unittest2' } } -- cgit v1.2.3 From ea03b0885c110003496c1f99dc7d9d2f1534955b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 31 May 2008 17:07:44 -0700 Subject: revert mysql test credential change --- activerecord/test/connections/native_mysql/connection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/connections/native_mysql/connection.rb b/activerecord/test/connections/native_mysql/connection.rb index 3ee031f384..1fab444e58 100644 --- a/activerecord/test/connections/native_mysql/connection.rb +++ b/activerecord/test/connections/native_mysql/connection.rb @@ -12,13 +12,13 @@ ActiveRecord::Base.logger = RAILS_DEFAULT_LOGGER ActiveRecord::Base.configurations = { 'arunit' => { :adapter => 'mysql', - :username => 'root', + :username => 'rails', :encoding => 'utf8', :database => 'activerecord_unittest', }, 'arunit2' => { :adapter => 'mysql', - :username => 'root', + :username => 'rails', :database => 'activerecord_unittest2' } } -- cgit v1.2.3