diff --git a/History.md b/History.md index 9bbbedb4..cdcca146 100644 --- a/History.md +++ b/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 diff --git a/README.md b/README.md index 6802a717..ec259230 100644 --- a/README.md +++ b/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/) @@ -56,7 +56,7 @@ For standard use you can just pass it the content of a file and call render with Setting the error mode of Liquid lets you specify how strictly you want your templates to be interpreted. Normally the parser is very lax and will accept almost anything without error. Unfortunately this can make -it very hard to debug and can lead to unexpected behaviour. +it very hard to debug and can lead to unexpected behaviour. Liquid also comes with a stricter parser that can be used when editing templates to give better error messages when templates are invalid. You can enable this new parser like this: diff --git a/lib/liquid.rb b/lib/liquid.rb index 28337943..7772bb5f 100644 --- a/lib/liquid.rb +++ b/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 diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index 76ab0a85..2921ce88 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -231,8 +231,8 @@ module Liquid end def create_variable(token, parse_context) - token.scan(ContentOfVariable) do |content| - markup = content.first + if token =~ ContentOfVariable + markup = Regexp.last_match(1) return Variable.new(markup, parse_context) end BlockBody.raise_missing_variable_terminator(token, parse_context) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index c2c27909..fcdd4d8a 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -10,21 +10,23 @@ module Liquid 'empty' => '' }.freeze - SINGLE_QUOTED_STRING = /\A\s*'(.*)'\s*\z/m - DOUBLE_QUOTED_STRING = /\A\s*"(.*)"\s*\z/m - INTEGERS_REGEX = /\A\s*(-?\d+)\s*\z/ - FLOATS_REGEX = /\A\s*(-?\d[\d\.]+)\s*\z/ + INTEGERS_REGEX = /\A(-?\d+)\z/ + FLOATS_REGEX = /\A(-?\d[\d\.]+)\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 diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 29cf602b..f361c02b 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -213,17 +213,23 @@ module Liquid if ary.empty? [] - elsif ary.first.respond_to?(:[]) && target_value.nil? - begin - ary.select { |item| item[property] } + elsif target_value.nil? + ary.select do |item| + item[property] rescue TypeError raise_property_error(property) + rescue NoMethodError + return nil unless item.respond_to?(:[]) + raise end - elsif ary.first.respond_to?(:[]) - begin - ary.select { |item| item[property] == target_value } + else + ary.select do |item| + 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?(:[]) - begin - ary.uniq { |a| a[property] } + else + ary.uniq do |item| + 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?(:[]) - begin - ary.reject { |a| a[property].nil? } + else + ary.reject do |item| + 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? - a <=> b + result = 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 diff --git a/test/integration/filter_kwarg_test.rb b/test/integration/filter_kwarg_test.rb new file mode 100644 index 00000000..2bd3cbdf --- /dev/null +++ b/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 diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 0b68482d..c307e65a 100644 --- a/test/integration/standard_filter_test.rb +++ b/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| - test_types.each do |second| - test_types.each do |third| - (@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 + StandardFilters.public_instance_methods(false).each do |method| + arg_count = @filters.method(method).arity + arg_count *= -1 if arg_count < 0 - begin - @filters.send(method, *inputs) - rescue Liquid::ArgumentError, Liquid::ZeroDivisionError - nil - end - end - end + test_types.repeated_permutation(arg_count) do |args| + @filters.send(method, *args) + rescue Liquid::Error + nil end end end