refactor: rename comment tag unit test

fix parsing comment tag with extra string

README.md

@@ -1,4 +1,4 @@
-[![Build Status](https://api.travis-ci.org/Shopify/liquid.svg?branch=master)](http://travis-ci.org/Shopify/liquid)
+[![Build status](https://github.com/Shopify/liquid/actions/workflows/liquid.yml/badge.svg)](https://github.com/Shopify/liquid/actions/workflows/liquid.yml)
 [![Inline docs](http://inch-ci.org/github/Shopify/liquid.svg?branch=master)](http://inch-ci.org/github/Shopify/liquid)
 
 # Liquid template engine

lib/liquid/standardfilters.rb

@@ -29,6 +29,19 @@ module Liquid
     )
     STRIP_HTML_TAGS = /<.*?>/m
 
+    class << self
+      def try_coerce_encoding(input, encoding:)
+        original_encoding = input.encoding
+        if input.encoding != encoding
+          input.force_encoding(encoding)
+          unless input.valid_encoding?
+            input.force_encoding(original_encoding)
+          end
+        end
+        input
+      end
+    end
+
     # @liquid_public_docs
     # @liquid_type filter
     # @liquid_category array
@@ -150,7 +163,8 @@ module Liquid
     # @liquid_syntax string | base64_decode
     # @liquid_return [string]
     def base64_decode(input)
-      Base64.strict_decode64(input.to_s)
+      input = input.to_s
+      StandardFilters.try_coerce_encoding(Base64.strict_decode64(input), encoding: input.encoding)
     rescue ::ArgumentError
       raise Liquid::ArgumentError, "invalid base64 provided to base64_decode"
     end
@@ -174,7 +188,8 @@ module Liquid
     # @liquid_syntax string | base64_url_safe_decode
     # @liquid_return [string]
     def base64_url_safe_decode(input)
-      Base64.urlsafe_decode64(input.to_s)
+      input = input.to_s
+      StandardFilters.try_coerce_encoding(Base64.urlsafe_decode64(input), encoding: input.encoding)
     rescue ::ArgumentError
       raise Liquid::ArgumentError, "invalid base64 provided to base64_url_safe_decode"
     end
@@ -892,9 +907,11 @@ module Liquid
         raise_property_error(property)
       end
 
-      InputIterator.new(values_for_sum, context).sum do |item|
+      result = InputIterator.new(values_for_sum, context).sum do |item|
         Utils.to_number(item)
       end
+
+      result.is_a?(BigDecimal) ? result.to_f : result
     end
 
     private

lib/liquid/tag.rb

@@ -54,7 +54,8 @@ module Liquid
     # of the `render_to_output_buffer` method will become the default and the `render`
     # method will be removed.
     def render_to_output_buffer(context, output)
-      output << render(context)
+      render_result = render(context)
+      output << render_result if render_result
       output
     end
 

lib/liquid/tags/comment.rb

@@ -15,8 +15,6 @@ module Liquid
   #   {% endcomment %}
   # @liquid_syntax_keyword content The content of the comment.
   class Comment < Block
-    TAG_DELIMITER = /\A(.*)#{TagStart}#{WhitespaceControl}?\s*(endcomment)\s*(.*)?#{WhitespaceControl}?#{TagEnd}\z/om
-
     def render_to_output_buffer(_context, output)
       output
     end
@@ -51,18 +49,9 @@ module Liquid
             next if tag_name_match.nil?
 
             tag_name_match[1]
-          elsif TAG_DELIMITER.match?(token)
-            # aggressively match comment delimiter
-            "endcomment"
-          elsif token =~ BlockBody::FullToken && Regexp.last_match(2) == "comment"
-            # aggressively match comment tag
-            "comment"
           else
-            tag_name_match = BlockBody::FullTokenPossiblyInvalid.match(token)
-
-            next if tag_name_match.nil?
-
-            tag_name_match[2]
+            token =~ BlockBody::FullToken
+            Regexp.last_match(2)
           end
 
           case tag_name
@@ -73,7 +62,10 @@ module Liquid
           when "endcomment"
             comment_tag_depth -= 1
 
-            return false if comment_tag_depth.zero?
+            if comment_tag_depth.zero?
+              parse_context.trim_whitespace = (token[-3] == WhitespaceControl)
+              return false
+            end
           end
         end
 

test/integration/standard_filter_test.rb

@@ -176,7 +176,17 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_base64_decode
-    assert_equal('one two three', @filters.base64_decode('b25lIHR3byB0aHJlZQ=='))
+    decoded = @filters.base64_decode('b25lIHR3byB0aHJlZQ==')
+    assert_equal('one two three', decoded)
+    assert_equal(Encoding::UTF_8, decoded.encoding)
+
+    decoded = @filters.base64_decode('4pyF')
+    assert_equal('✅', decoded)
+    assert_equal(Encoding::UTF_8, decoded.encoding)
+
+    decoded = @filters.base64_decode("/w==")
+    assert_equal(Encoding::ASCII_8BIT, decoded.encoding)
+    assert_equal((+"\xFF").force_encoding(Encoding::ASCII_8BIT), decoded)
 
     exception = assert_raises(Liquid::ArgumentError) do
       @filters.base64_decode("invalidbase64")
@@ -194,10 +204,21 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_base64_url_safe_decode
+    decoded = @filters.base64_url_safe_decode('YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXogQUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVogMTIzNDU2Nzg5MCAhQCMkJV4mKigpLT1fKy8_Ljo7W117fVx8')
     assert_equal(
       'abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 !@#$%^&*()-=_+/?.:;[]{}\|',
-      @filters.base64_url_safe_decode('YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXogQUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVogMTIzNDU2Nzg5MCAhQCMkJV4mKigpLT1fKy8_Ljo7W117fVx8'),
+      decoded,
     )
+    assert_equal(Encoding::UTF_8, decoded.encoding)
+
+    decoded = @filters.base64_url_safe_decode('4pyF')
+    assert_equal('✅', decoded)
+    assert_equal(Encoding::UTF_8, decoded.encoding)
+
+    decoded = @filters.base64_url_safe_decode("_w==")
+    assert_equal(Encoding::ASCII_8BIT, decoded.encoding)
+    assert_equal((+"\xFF").force_encoding(Encoding::ASCII_8BIT), decoded)
+
     exception = assert_raises(Liquid::ArgumentError) do
       @filters.base64_url_safe_decode("invalidbase64")
     end
@@ -994,6 +1015,42 @@ class StandardFiltersTest < Minitest::Test
     assert(t.foo > 0)
   end
 
+  def test_sum_of_floats
+    input = [0.1, 0.2, 0.3]
+    assert_equal(0.6, @filters.sum(input))
+    assert_template_result("0.6", "{{ input | sum }}", { "input" => input })
+  end
+
+  def test_sum_of_negative_floats
+    input = [0.1, 0.2, -0.3]
+    assert_equal(0.0, @filters.sum(input))
+    assert_template_result("0.0", "{{ input | sum }}", { "input" => input })
+  end
+
+  def test_sum_with_float_strings
+    input = [0.1, "0.2", "0.3"]
+    assert_equal(0.6, @filters.sum(input))
+    assert_template_result("0.6", "{{ input | sum }}", { "input" => input })
+  end
+
+  def test_sum_resulting_in_negative_float
+    input = [0.1, -0.2, -0.3]
+    assert_equal(-0.4, @filters.sum(input))
+    assert_template_result("-0.4", "{{ input | sum }}", { "input" => input })
+  end
+
+  def test_sum_with_floats_and_indexable_map_values
+    input = [{ "quantity" => 1 }, { "quantity" => 0.2, "weight" => -0.3 }, { "weight" => 0.4 }]
+    assert_equal(0.0, @filters.sum(input))
+    assert_equal(1.2, @filters.sum(input, "quantity"))
+    assert_equal(0.1, @filters.sum(input, "weight"))
+    assert_equal(0.0, @filters.sum(input, "subtotal"))
+    assert_template_result("0", "{{ input | sum }}", { "input" => input })
+    assert_template_result("1.2", "{{ input | sum: 'quantity' }}", { "input" => input })
+    assert_template_result("0.1", "{{ input | sum: 'weight' }}", { "input" => input })
+    assert_template_result("0", "{{ input | sum: 'subtotal' }}", { "input" => input })
+  end
+
   private
 
   def with_timezone(tz)

test/unit/tag_unit_test.rb

@@ -20,4 +20,13 @@ class TagUnitTest < Minitest::Test
     tag = Tag.parse("some_tag", "", Tokenizer.new(""), ParseContext.new)
     assert_equal('some_tag', tag.tag_name)
   end
+
+  class CustomTag < Liquid::Tag
+    def render(_context); end
+  end
+
+  def test_tag_render_to_output_buffer_nil_value
+    custom_tag = CustomTag.parse("some_tag", "", Tokenizer.new(""), ParseContext.new)
+    assert_equal('some string', custom_tag.render_to_output_buffer(Context.new, "some string"))
+  end
 end

test/unit/tags/comment_tag_unit_test.rb

@@ -31,25 +31,45 @@ class CommentTagUnitTest < Minitest::Test
     LIQUID
   end
 
-  def test_allows_incomplete_tags_inside_a_comment
-    assert_template_result("", <<~LIQUID.chomp)
+  def test_allows_unclosed_tags
+    assert_template_result('', <<~LIQUID.chomp)
       {% comment %}
-        {% assign foo = "1"
+        {% if true %}
+      {% endcomment %}
+    LIQUID
+  end
+
+  def test_open_tags_in_comment
+    assert_template_result('', <<~LIQUID.chomp)
+      {% comment %}
+        {% assign a = 123 {% comment %}
       {% endcomment %}
     LIQUID
 
-    assert_template_result("", <<~LIQUID.chomp)
-      {% comment %}
+    assert_raises(Liquid::SyntaxError) do
+      assert_template_result("", <<~LIQUID.chomp)
         {% comment %}
-          {% invalid
+          {% assign foo = "1"
         {% endcomment %}
-      {% endcomment %}
-    LIQUID
+      LIQUID
+    end
 
-    assert_template_result("", <<~LIQUID.chomp)
-      {% comment %}
-      {% {{ {%- endcomment %}
-    LIQUID
+    assert_raises(Liquid::SyntaxError) do
+      assert_template_result("", <<~LIQUID.chomp)
+        {% comment %}
+          {% comment %}
+            {% invalid
+          {% endcomment %}
+        {% endcomment %}
+      LIQUID
+    end
+
+    assert_raises(Liquid::SyntaxError) do
+      assert_template_result("", <<~LIQUID.chomp)
+        {% comment %}
+        {% {{ {%- endcomment %}
+      LIQUID
+    end
   end
 
   def test_child_comment_tags_need_to_be_closed
@@ -135,4 +155,36 @@ class CommentTagUnitTest < Minitest::Test
       LIQUID
     )
   end
+
+  def test_ignores_delimiter_with_extra_strings
+    assert_template_result(
+      '',
+      <<~LIQUID.chomp,
+        {% if true %}
+          {% comment %}
+            {% commentXXXXX %}wut{% endcommentXXXXX %}
+          {% endcomment %}
+        {% endif %}
+      LIQUID
+    )
+  end
+
+  def test_delimiter_can_have_extra_strings
+    assert_template_result('', "{% comment %}123{% endcomment xyz %}")
+    assert_template_result('', "{% comment %}123{% endcomment\txyz %}")
+    assert_template_result('', "{% comment %}123{% endcomment\nxyz %}")
+    assert_template_result('', "{% comment %}123{% endcomment\n   xyz  endcomment %}")
+    assert_template_result('', "{%comment}{% assign a = 1 %}{%endcomment}{% endif %}")
+  end
+
+  def test_with_whitespace_control
+    assert_template_result("Hello!", "      {%- comment -%}123{%- endcomment -%}Hello!")
+    assert_template_result("Hello!", "{%- comment -%}123{%- endcomment -%}     Hello!")
+    assert_template_result("Hello!", "      {%- comment -%}123{%- endcomment -%}     Hello!")
+
+    assert_template_result("Hello!", <<~LIQUID.chomp)
+      {%- comment %}Whitespace control!{% endcomment -%}
+      Hello!
+    LIQUID
+  end
 end