Merge remote-tracking branch 'origin/master' into pr-1422

History.md

@@ -1,5 +1,11 @@
 # Liquid Change Log
 
+## 5.1.1 (unreleased)
+
+### Fixes
+
+* Fix some internal errors in filters from invalid input [Dylan Thacker-Smith]
+
 ## 5.1.0 / 2021-09-09
 
 ### Features

README.md

@@ -5,7 +5,7 @@
 
 * [Contributing guidelines](CONTRIBUTING.md)
 * [Version history](History.md)
-* [Liquid documentation from Shopify](http://docs.shopify.com/themes/liquid-basics)
+* [Liquid documentation from Shopify](https://shopify.dev/api/liquid)
 * [Liquid Wiki at GitHub](https://github.com/Shopify/liquid/wiki)
 * [Website](http://liquidmarkup.org/)
 

lib/liquid.rb

@@ -36,7 +36,7 @@ module Liquid
   VariableIncompleteEnd       = /\}\}?/
   QuotedString                = /"[^"]*"|'[^']*'/
   QuotedFragment              = /#{QuotedString}|(?:[^\s,\|'"]|#{QuotedString})+/o
-  TagAttributes               = /(\w+)\s*\:\s*(#{QuotedFragment})/o
+  TagAttributes               = /(\w[\w-]*)\s*\:\s*(#{QuotedFragment})/o
   AnyStartingTag              = /#{TagStart}|#{VariableStart}/o
   PartialTemplateParser       = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/om
   TemplateParser              = /(#{PartialTemplateParser}|#{AnyStartingTag})/om

lib/liquid/block_body.rb

@@ -231,8 +231,8 @@ module Liquid
     end
 
     def create_variable(token, parse_context)
-      token.scan(ContentOfVariable) do |content|
+      if token =~ ContentOfVariable
-        markup = content.first
+        markup = Regexp.last_match(1)
         return Variable.new(markup, parse_context)
       end
       BlockBody.raise_missing_variable_terminator(token, parse_context)

lib/liquid/expression.rb

@@ -10,21 +10,23 @@ module Liquid
       'empty' => ''
     }.freeze
 
-    SINGLE_QUOTED_STRING = /\A\s*'(.*)'\s*\z/m
+    INTEGERS_REGEX       = /\A(-?\d+)\z/
-    DOUBLE_QUOTED_STRING = /\A\s*"(.*)"\s*\z/m
+    FLOATS_REGEX         = /\A(-?\d[\d\.]+)\z/
-    INTEGERS_REGEX       = /\A\s*(-?\d+)\s*\z/
-    FLOATS_REGEX         = /\A\s*(-?\d[\d\.]+)\s*\z/
 
     # Use an atomic group (?>...) to avoid pathological backtracing from
     # malicious input as described in https://github.com/Shopify/liquid/issues/1357
-    RANGES_REGEX         = /\A\s*\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\s*\z/
+    RANGES_REGEX         = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/
 
     def self.parse(markup)
+      return nil unless markup
+
+      markup = markup.strip
+      if (markup.start_with?('"') && markup.end_with?('"')) ||
+         (markup.start_with?("'") && markup.end_with?("'"))
+        return markup[1..-2]
+      end
+
       case markup
-      when nil
-        nil
-      when SINGLE_QUOTED_STRING, DOUBLE_QUOTED_STRING
-        Regexp.last_match(1)
       when INTEGERS_REGEX
         Regexp.last_match(1).to_i
       when RANGES_REGEX
@@ -32,7 +34,6 @@ module Liquid
       when FLOATS_REGEX
         Regexp.last_match(1).to_f
       else
-        markup = markup.strip
         if LITERALS.key?(markup)
           LITERALS[markup]
         else

lib/liquid/standardfilters.rb

@@ -213,17 +213,23 @@ module Liquid
 
       if ary.empty?
         []
-      elsif ary.first.respond_to?(:[]) && target_value.nil?
+      elsif target_value.nil?
-        begin
+        ary.select do |item|
-          ary.select { |item| item[property] }
+          item[property]
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
-      elsif ary.first.respond_to?(:[])
+      else
-        begin
+        ary.select do |item|
-          ary.select { |item| item[property] == target_value }
+          item[property] == target_value
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -237,11 +243,14 @@ module Liquid
         ary.uniq
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[])
+      else
-        begin
+        ary.uniq do |item|
-          ary.uniq { |a| a[property] }
+          item[property]
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -277,11 +286,14 @@ module Liquid
         ary.compact
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[])
+      else
-        begin
+        ary.reject do |item|
-          ary.reject { |a| a[property].nil? }
+          item[property].nil?
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -506,10 +518,16 @@ module Liquid
     end
 
     def nil_safe_compare(a, b)
-      if !a.nil? && !b.nil?
+      result = a <=> b
-        a <=> b
+
+      if result
+        result
+      elsif a.nil?
+        1
+      elsif b.nil?
+        -1
       else
-        a.nil? ? 1 : -1
+        raise Liquid::ArgumentError, "cannot sort values of incompatible types"
       end
     end
 

test/integration/filter_kwarg_test.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class FilterKwargTest < Minitest::Test
+  module KwargFilter
+    def html_tag(_tag, attributes)
+      attributes
+        .map { |key, value| "#{key}='#{value}'" }
+        .join(' ')
+    end
+  end
+
+  include Liquid
+
+  def test_can_parse_data_kwargs
+    with_global_filter(KwargFilter) do
+      assert_equal(
+        "data-src='src' data-widths='100, 200'",
+        Template.parse("{{ 'img' | html_tag: data-src: 'src', data-widths: '100, 200' }}").render(nil, nil)
+      )
+    end
+  end
+end

test/integration/standard_filter_test.rb

@@ -259,8 +259,8 @@ class StandardFiltersTest < Minitest::Test
       { "price" => 1, "handle" => "gamma" },
       { "price" => 2, "handle" => "epsilon" },
       { "price" => 4, "handle" => "alpha" },
-      { "handle" => "delta" },
       { "handle" => "beta" },
+      { "handle" => "delta" },
     ]
     assert_equal(expectation, @filters.sort(input, "price"))
   end
@@ -872,23 +872,14 @@ class StandardFiltersTest < Minitest::Test
       { 1 => "bar" },
       ["foo", 123, nil, true, false, Drop, ["foo"], { foo: "bar" }],
     ]
-    test_types.each do |first|
+    StandardFilters.public_instance_methods(false).each do |method|
-      test_types.each do |second|
+      arg_count = @filters.method(method).arity
-        test_types.each do |third|
+      arg_count *= -1 if arg_count < 0
-          (@filters.methods - Object.methods).each do |method|
-            arg_count = @filters.method(method).arity
-            arg_count *= -1 if arg_count < 0
-            inputs = [first]
-            inputs << ([second] * (arg_count - 1)) if arg_count > 1
-            inputs << ([third] * (arg_count - 1)) if arg_count > 2
 
-            begin
+      test_types.repeated_permutation(arg_count) do |args|
-              @filters.send(method, *inputs)
+        @filters.send(method, *args)
-            rescue Liquid::ArgumentError, Liquid::ZeroDivisionError
+      rescue Liquid::Error
-              nil
+        nil
-            end
-          end
-        end
       end
     end
   end