From 0f11c97623922bfb9f71ed1e9488f7667a5446c0 Mon Sep 17 00:00:00 2001 From: Michael Go Date: Mon, 16 Jan 2023 18:51:34 -0400 Subject: [PATCH 1/2] 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 << "\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 = "\n" \ + "\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 = "\n" \ + "1:false" \ + "2:true" \ + "\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 From e889a9da0b5e83887f55ee5539c716bb75f5999f Mon Sep 17 00:00:00 2001 From: Michael Go Date: Tue, 17 Jan 2023 17:48:16 -0400 Subject: [PATCH 2/2] use to_i to parse parameters of tablerow tag --- lib/liquid/tags/table_row.rb | 25 +++++++++++-------------- lib/liquid/utils.rb | 5 +---- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index f404efd6..d52f268a 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -45,24 +45,13 @@ module Liquid def render_to_output_buffer(context, output) (collection = context.evaluate(@collection_name)) || (return '') - 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 + from = @attributes.key?('offset') ? to_integer(context.evaluate(@attributes['offset'])) : 0 + to = @attributes.key?('limit') ? from + to_integer(context.evaluate(@attributes['limit'])) : nil collection = Utils.slice_collection(collection, from, to) length = collection.length - cols = if @attributes.key?('cols') - Utils.to_integer(context.evaluate(@attributes['cols']), allow_nil: true) - else - length - end + cols = @attributes.key?('cols') ? to_integer(context.evaluate(@attributes['cols'])) : length output << "\n" context.stack do @@ -93,6 +82,14 @@ module Liquid super + @node.attributes.values + [@node.collection_name] end end + + private + + def to_integer(value) + value.to_i + rescue NoMethodError + raise Liquid::ArgumentError, "invalid integer" + end end Template.register_tag('tablerow', TableRow) diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index fce8e4c7..4ec7d811 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -39,11 +39,8 @@ module Liquid segments end - def self.to_integer(num, allow_nil: false) + def self.to_integer(num) 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)