From e12c2a6d11a9ebc28b85f34230b11fb654503726 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 14 Dec 2015 08:18:05 -0700 Subject: Deprecate limit strings with commas Some backends allow `LIMIT 1,2` as a shorthand for `LIMIT 1 OFFSET 2`. Supporting this in Active Record massively complicates using bind parameters for limit and offset, and it's trivially easy to build an invalid SQL query by also calling `offset` on the same `Relation`. This is a niche syntax that is only supported by a few adapters, and can be trivially worked around by calling offset explicitly. --- activerecord/lib/active_record/relation/query_methods.rb | 6 ++++++ activerecord/test/cases/base_test.rb | 10 +++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index dbecb842b5..f7115c7a91 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -677,6 +677,12 @@ module ActiveRecord end def limit!(value) # :nodoc: + if ::String === value && value.include?(",") + ActiveSupport::Deprecation.warn(<<-WARNING) + Passing a string to limit in the form "1,2" is deprecated and will be + removed in Rails 5.1. Please call `offset` explicitly instead. + WARNING + end self.limit_value = value self end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 3a9d60a79f..b449280fb4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -112,7 +112,9 @@ class BasicsTest < ActiveRecord::TestCase unless current_adapter?(:PostgreSQLAdapter, :OracleAdapter, :SQLServerAdapter, :FbAdapter) def test_limit_with_comma - assert Topic.limit("1,2").to_a + assert_deprecated do + assert Topic.limit("1,2").to_a + end end end @@ -138,8 +140,10 @@ class BasicsTest < ActiveRecord::TestCase end def test_limit_should_sanitize_sql_injection_for_limit_with_commas - assert_raises(ArgumentError) do - Topic.limit("1, 7 procedure help()").to_a + assert_deprecated do + assert_raises(ArgumentError) do + Topic.limit("1, 7 procedure help()").to_a + end end end -- cgit v1.2.3