From 0f11c97623922bfb9f71ed1e9488f7667a5446c0 Mon Sep 17 00:00:00 2001
From: Michael Go <michael.go@shopify.com>
Date: Mon, 16 Jan 2023 18:51:34 -0400
Subject: [PATCH] raise invalid integer argument error from tablerow

---
 lib/liquid/tags/table_row.rb            | 17 +++++++--
 lib/liquid/utils.rb                     |  5 ++-
 test/integration/tags/table_row_test.rb | 46 +++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb
index 2c1d24bf..f404efd6 100644
--- a/lib/liquid/tags/table_row.rb
+++ b/lib/liquid/tags/table_row.rb
@@ -45,13 +45,24 @@ module Liquid
     def render_to_output_buffer(context, output)
       (collection = context.evaluate(@collection_name)) || (return '')
 
-      from = @attributes.key?('offset') ? context.evaluate(@attributes['offset']).to_i : 0
-      to   = @attributes.key?('limit')  ? from + context.evaluate(@attributes['limit']).to_i : nil
+      from = if @attributes.key?('offset')
+        Utils.to_integer(context.evaluate(@attributes['offset']), allow_nil: true)
+      else
+        0
+      end
+
+      to = if @attributes.key?('limit')
+        from + Utils.to_integer(context.evaluate(@attributes['limit']), allow_nil: true)
+      end
 
       collection = Utils.slice_collection(collection, from, to)
       length     = collection.length
 
-      cols = @attributes.key?('cols') ? context.evaluate(@attributes['cols']).to_i : length
+      cols = if @attributes.key?('cols')
+        Utils.to_integer(context.evaluate(@attributes['cols']), allow_nil: true)
+      else
+        length
+      end
 
       output << "<tr class=\"row1\">\n"
       context.stack do
diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb
index 4ec7d811..fce8e4c7 100644
--- a/lib/liquid/utils.rb
+++ b/lib/liquid/utils.rb
@@ -39,8 +39,11 @@ module Liquid
       segments
     end
 
-    def self.to_integer(num)
+    def self.to_integer(num, allow_nil: false)
       return num if num.is_a?(Integer)
+      # with allow_nil param, return 0 which is equal to nil.to_i
+      return 0 if num.nil? && allow_nil
+
       num = num.to_s
       begin
         Integer(num)
diff --git a/test/integration/tags/table_row_test.rb b/test/integration/tags/table_row_test.rb
index 8556ed2d..5a7d1db2 100644
--- a/test/integration/tags/table_row_test.rb
+++ b/test/integration/tags/table_row_test.rb
@@ -80,6 +80,32 @@ class TableRowTest < Minitest::Test
       { "var" => nil })
   end
 
+  def test_nil_limit_is_treated_as_zero
+    expect = "<tr class=\"row1\">\n" \
+      "</tr>\n"
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) limit:nil %}{{ i }}{% endtablerow %}")
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) limit:var %}{{ i }}{% endtablerow %}",
+      { "var" => nil })
+  end
+
+  def test_nil_offset_is_treated_as_zero
+    expect = "<tr class=\"row1\">\n" \
+      "<td class=\"col1\">1:false</td>" \
+      "<td class=\"col2\">2:true</td>" \
+      "</tr>\n"
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) offset:nil %}{{ i }}:{{ tablerowloop.col_last }}{% endtablerow %}")
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) offset:var %}{{ i }}:{{ tablerowloop.col_last }}{% endtablerow %}",
+      { "var" => nil })
+  end
+
   def test_tablerow_loop_drop_attributes
     template = <<~LIQUID.chomp
       {% tablerow i in (1...2) %}
@@ -131,4 +157,24 @@ class TableRowTest < Minitest::Test
 
     assert_template_result(expected_output, template)
   end
+
+  def test_table_row_renders_correct_error_message_for_invalid_parameters
+    assert_template_result(
+      "Liquid error (line 1): invalid integer",
+      '{% tablerow n in (1...10) limit:true %} {{n}} {% endtablerow %}',
+      render_errors: true,
+    )
+
+    assert_template_result(
+      "Liquid error (line 1): invalid integer",
+      '{% tablerow n in (1...10) offset:true %} {{n}} {% endtablerow %}',
+      render_errors: true,
+    )
+
+    assert_template_result(
+      "Liquid error (line 1): invalid integer",
+      '{% tablerow n in (1...10) cols:true %} {{n}} {% endtablerow %}',
+      render_errors: true,
+    )
+  end
 end