Merge pull request #1617 from Shopify/better-filter-overflow-handling

Support big integers in truncatewords and slice filters

lib/liquid/standardfilters.rb

@@ -6,7 +6,14 @@ require 'bigdecimal'
 
 module Liquid
   module StandardFilters
-    MAX_INT = (1 << 31) - 1
+    MAX_I32 = (1 << 31) - 1
+    private_constant :MAX_I32
+
+    MIN_I64 = -(1 << 63)
+    MAX_I64 = (1 << 63) - 1
+    I64_RANGE = MIN_I64..MAX_I64
+    private_constant :MIN_I64, :MAX_I64, :I64_RANGE
+
     HTML_ESCAPE = {
       '&' => '&amp;',
       '>' => '&gt;',
@@ -186,10 +193,19 @@ module Liquid
       offset = Utils.to_integer(offset)
       length = length ? Utils.to_integer(length) : 1
 
-      if input.is_a?(Array)
-        input.slice(offset, length) || []
-      else
-        input.to_s.slice(offset, length) || ''
+      begin
+        if input.is_a?(Array)
+          input.slice(offset, length) || []
+        else
+          input.to_s.slice(offset, length) || ''
+        end
+      rescue RangeError
+        if I64_RANGE.cover?(length) && I64_RANGE.cover?(offset)
+          raise # unexpected error
+        end
+        offset = offset.clamp(I64_RANGE)
+        length = length.clamp(I64_RANGE)
+        retry
       end
     end
 
@@ -239,9 +255,9 @@ module Liquid
       wordlist = begin
         input.split(" ", words + 1)
       rescue RangeError
-        raise if words + 1 < MAX_INT
-        # e.g. integer #{words} too big to convert to `int'
-        raise Liquid::ArgumentError, "integer #{words} too big for truncatewords"
+        # integer too big for String#split, but we can semantically assume no truncation is needed
+        return input if words + 1 > MAX_I32
+        raise # unexpected error
       end
       return input if wordlist.length <= words
 

test/integration/standard_filter_test.rb

@@ -109,6 +109,10 @@ class StandardFiltersTest < Minitest::Test
     assert_raises(Liquid::ArgumentError) do
       @filters.slice('foobar', 0, "")
     end
+    assert_equal("", @filters.slice("foobar", 0, -(1 << 64)))
+    assert_equal("foobar", @filters.slice("foobar", 0, 1 << 63))
+    assert_equal("", @filters.slice("foobar", 1 << 63, 6))
+    assert_equal("", @filters.slice("foobar", -(1 << 63), 6))
   end
 
   def test_slice_on_arrays
@@ -123,6 +127,10 @@ class StandardFiltersTest < Minitest::Test
     assert_equal(%w(r), @filters.slice(input, -1))
     assert_equal(%w(), @filters.slice(input, 100, 10))
     assert_equal(%w(), @filters.slice(input, -100, 10))
+    assert_equal([], @filters.slice(input, 0, -(1 << 64)))
+    assert_equal(input, @filters.slice(input, 0, 1 << 63))
+    assert_equal([], @filters.slice(input, 1 << 63, 6))
+    assert_equal([], @filters.slice(input, -(1 << 63), 6))
   end
 
   def test_truncate
@@ -132,6 +140,8 @@ class StandardFiltersTest < Minitest::Test
     assert_equal('1234567890', @filters.truncate('1234567890'))
     assert_equal("测试...", @filters.truncate("测试测试测试测试", 5))
     assert_equal('12341', @filters.truncate("1234567890", 5, 1))
+    assert_equal("foobar", @filters.truncate("foobar", 1 << 63))
+    assert_equal("...", @filters.truncate("foobar", -(1 << 63)))
   end
 
   def test_split
@@ -227,10 +237,8 @@ class StandardFiltersTest < Minitest::Test
     assert_equal('one two three...', @filters.truncatewords("one  two\tthree\nfour", 3))
     assert_equal('one two...', @filters.truncatewords("one two three four", 2))
     assert_equal('one...', @filters.truncatewords("one two three four", 0))
-    exception = assert_raises(Liquid::ArgumentError) do
-      @filters.truncatewords("one two three four", 1 << 31)
-    end
-    assert_equal("Liquid error: integer #{1 << 31} too big for truncatewords", exception.message)
+    assert_equal('one two three four', @filters.truncatewords("one two three four", 1 << 31))
+    assert_equal('one...', @filters.truncatewords("one two three four", -(1 << 32)))
   end
 
   def test_strip_html