From fe66edb8254a4af4af3a69a32884178e2263c421 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 23 Oct 2020 10:53:16 -0400 Subject: [PATCH 1/9] Freeze block body after parsing completes --- Gemfile | 2 +- lib/liquid/block.rb | 1 + lib/liquid/block_body.rb | 9 +++++++++ lib/liquid/document.rb | 1 + lib/liquid/tags/case.rb | 6 +++++- lib/liquid/tags/for.rb | 2 ++ lib/liquid/tags/if.rb | 1 + 7 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 14537c6..338fe56 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,6 @@ group :test do gem 'rubocop-performance', require: false platform :mri, :truffleruby do - gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'master' + gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'pz-block-body-buffer' end end diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 47d5f25..d6619f9 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -13,6 +13,7 @@ module Liquid @body = new_body while parse_body(@body, tokens) end + @body.freeze(parse_context) end # For backwards compatibility diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index 67d5d63..f61ae87 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -16,9 +16,12 @@ module Liquid def initialize @nodelist = [] @blank = true + @frozen = false end def parse(tokenizer, parse_context, &block) + raise FrozenError, "can't modify frozen Liquid::BlockBody" if @frozen + parse_context.line_number = tokenizer.line_number if tokenizer.for_liquid_tag @@ -28,6 +31,10 @@ module Liquid end end + def freeze(_context) + @frozen = true + end + private def parse_for_liquid_tag(tokenizer, parse_context) while (token = tokenizer.shift) unless token.empty? || token =~ WhitespaceOrNothing @@ -192,6 +199,8 @@ module Liquid end def render_to_output_buffer(context, output) + raise "Can only render when frozen" unless @frozen + context.resource_limits.increment_render_score(@nodelist.length) idx = 0 diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb index 2e47ffb..21ddec2 100644 --- a/lib/liquid/document.rb +++ b/lib/liquid/document.rb @@ -22,6 +22,7 @@ module Liquid def parse(tokenizer, parse_context) while parse_body(tokenizer) end + @body.freeze(parse_context) rescue SyntaxError => e e.line_number ||= parse_context.line_number raise diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index b46fa05..cc85ed4 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -20,7 +20,11 @@ module Liquid def parse(tokens) body = new_body - body = @blocks.last.attachment while parse_body(body, tokens) + while parse_body(body, tokens) + body.freeze(parse_context) + body = @blocks.last.attachment + end + body.freeze(parse_context) if blank? @blocks.each { |condition| condition.attachment.remove_blank_strings } end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 6f66ebd..9a81b4d 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -61,7 +61,9 @@ module Liquid def parse(tokens) if parse_body(@for_block, tokens) parse_body(@else_block, tokens) + @else_block.freeze(parse_context) end + @for_block.freeze(parse_context) if blank? @for_block.remove_blank_strings @else_block&.remove_blank_strings diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index c910b0b..7632be9 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -31,6 +31,7 @@ module Liquid def parse(tokens) while parse_body(@blocks.last.attachment, tokens) end + @blocks.each { |block| block.attachment.freeze(parse_context) } if blank? @blocks.each { |condition| condition.attachment.remove_blank_strings } end From 0bedc71854196ebb0164a6eaccc93f8c8e889aa3 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 26 Oct 2020 15:11:00 -0400 Subject: [PATCH 2/9] Address comments --- Gemfile | 2 +- lib/liquid/block.rb | 20 +++++++++++++++----- lib/liquid/block_body.rb | 10 +++++----- lib/liquid/document.rb | 2 +- lib/liquid/tags/case.rb | 9 +-------- lib/liquid/tags/for.rb | 6 ------ lib/liquid/tags/if.rb | 4 ---- 7 files changed, 23 insertions(+), 30 deletions(-) diff --git a/Gemfile b/Gemfile index 338fe56..14537c6 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,6 @@ group :test do gem 'rubocop-performance', require: false platform :mri, :truffleruby do - gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'pz-block-body-buffer' + gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'master' end end diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index d6619f9..4299915 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -11,9 +11,9 @@ module Liquid def parse(tokens) @body = new_body - while parse_body(@body, tokens) + while parse_body(@body, tokens, partial: true) end - @body.freeze(parse_context) + @body.freeze end # For backwards compatibility @@ -68,7 +68,9 @@ module Liquid end # @api public - def parse_body(body, tokens) + def parse_body(body, tokens, partial: false) + block_terminated = true + if parse_context.depth >= MAX_DEPTH raise StackLevelError, "Nesting too deep" end @@ -77,7 +79,10 @@ module Liquid body.parse(tokens, parse_context) do |end_tag_name, end_tag_params| @blank &&= body.blank? - return false if end_tag_name == block_delimiter + if end_tag_name == block_delimiter + block_terminated = false + next + end raise_tag_never_closed(block_name) unless end_tag_name # this tag is not registered with the system @@ -88,7 +93,12 @@ module Liquid parse_context.depth -= 1 end - true + unless partial + body.remove_blank_strings if body.blank? + body.freeze + end + + block_terminated end end end diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index f61ae87..b8350a3 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -16,11 +16,10 @@ module Liquid def initialize @nodelist = [] @blank = true - @frozen = false end def parse(tokenizer, parse_context, &block) - raise FrozenError, "can't modify frozen Liquid::BlockBody" if @frozen + raise FrozenError, "can't modify frozen Liquid::BlockBody" if frozen? parse_context.line_number = tokenizer.line_number @@ -31,8 +30,9 @@ module Liquid end end - def freeze(_context) - @frozen = true + def freeze + @nodelist.freeze + super end private def parse_for_liquid_tag(tokenizer, parse_context) @@ -199,7 +199,7 @@ module Liquid end def render_to_output_buffer(context, output) - raise "Can only render when frozen" unless @frozen + raise "Can only render when frozen" unless frozen? context.resource_limits.increment_render_score(@nodelist.length) diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb index 21ddec2..9bf13b4 100644 --- a/lib/liquid/document.rb +++ b/lib/liquid/document.rb @@ -22,7 +22,7 @@ module Liquid def parse(tokenizer, parse_context) while parse_body(tokenizer) end - @body.freeze(parse_context) + @body.freeze rescue SyntaxError => e e.line_number ||= parse_context.line_number raise diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index cc85ed4..734f166 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -20,14 +20,7 @@ module Liquid def parse(tokens) body = new_body - while parse_body(body, tokens) - body.freeze(parse_context) - body = @blocks.last.attachment - end - body.freeze(parse_context) - if blank? - @blocks.each { |condition| condition.attachment.remove_blank_strings } - end + body = @blocks.last.attachment while parse_body(body, tokens) end def nodelist diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 9a81b4d..d7e93bb 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -61,12 +61,6 @@ module Liquid def parse(tokens) if parse_body(@for_block, tokens) parse_body(@else_block, tokens) - @else_block.freeze(parse_context) - end - @for_block.freeze(parse_context) - if blank? - @for_block.remove_blank_strings - @else_block&.remove_blank_strings end end diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 7632be9..c3b8915 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -31,10 +31,6 @@ module Liquid def parse(tokens) while parse_body(@blocks.last.attachment, tokens) end - @blocks.each { |block| block.attachment.freeze(parse_context) } - if blank? - @blocks.each { |condition| condition.attachment.remove_blank_strings } - end end def unknown_tag(tag, markup, tokens) From 5c082472a10a7795fa188dfd01b0a2763fea006d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 26 Oct 2020 16:07:43 -0400 Subject: [PATCH 3/9] Address comments --- lib/liquid/block.rb | 18 ++++-------------- lib/liquid/block_body.rb | 2 +- lib/liquid/tags/case.rb | 4 ++++ lib/liquid/tags/for.rb | 6 ++++++ lib/liquid/tags/if.rb | 4 ++++ 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 4299915..dc22a47 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -11,7 +11,7 @@ module Liquid def parse(tokens) @body = new_body - while parse_body(@body, tokens, partial: true) + while parse_body(@body, tokens) end @body.freeze end @@ -68,9 +68,7 @@ module Liquid end # @api public - def parse_body(body, tokens, partial: false) - block_terminated = true - + def parse_body(body, tokens) if parse_context.depth >= MAX_DEPTH raise StackLevelError, "Nesting too deep" end @@ -79,10 +77,7 @@ module Liquid body.parse(tokens, parse_context) do |end_tag_name, end_tag_params| @blank &&= body.blank? - if end_tag_name == block_delimiter - block_terminated = false - next - end + return false if end_tag_name == block_delimiter raise_tag_never_closed(block_name) unless end_tag_name # this tag is not registered with the system @@ -93,12 +88,7 @@ module Liquid parse_context.depth -= 1 end - unless partial - body.remove_blank_strings if body.blank? - body.freeze - end - - block_terminated + true end end end diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index b8350a3..af62a14 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -199,7 +199,7 @@ module Liquid end def render_to_output_buffer(context, output) - raise "Can only render when frozen" unless frozen? + freeze unless frozen? context.resource_limits.increment_render_score(@nodelist.length) diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 734f166..caa6fff 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -21,6 +21,10 @@ module Liquid def parse(tokens) body = new_body body = @blocks.last.attachment while parse_body(body, tokens) + if blank? + @blocks.each { |condition| condition.attachment.remove_blank_strings } + end + @blocks.each { |condition| condition.attachment.freeze } end def nodelist diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index d7e93bb..df2d4db 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -62,6 +62,12 @@ module Liquid if parse_body(@for_block, tokens) parse_body(@else_block, tokens) end + if blank? + @for_block.remove_blank_strings + @else_block&.remove_blank_strings + end + @for_block.freeze + @else_block&.freeze end def nodelist diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index c3b8915..7bef1af 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -31,6 +31,10 @@ module Liquid def parse(tokens) while parse_body(@blocks.last.attachment, tokens) end + if blank? + @blocks.each { |condition| condition.attachment.remove_blank_strings } + end + @blocks.each { |block| block.attachment.freeze } end def unknown_tag(tag, markup, tokens) From 292d971937e418283d68788502ad09acfd799f6d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 27 Oct 2020 10:42:30 -0400 Subject: [PATCH 4/9] Merge loops --- lib/liquid/tags/case.rb | 6 +++--- lib/liquid/tags/if.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index caa6fff..12d1712 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -21,10 +21,10 @@ module Liquid def parse(tokens) body = new_body body = @blocks.last.attachment while parse_body(body, tokens) - if blank? - @blocks.each { |condition| condition.attachment.remove_blank_strings } + @blocks.each do |condition| + condition.attachment.remove_blank_strings if blank? + condition.attachment.freeze end - @blocks.each { |condition| condition.attachment.freeze } end def nodelist diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 7bef1af..d4de9c2 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -31,10 +31,10 @@ module Liquid def parse(tokens) while parse_body(@blocks.last.attachment, tokens) end - if blank? - @blocks.each { |condition| condition.attachment.remove_blank_strings } + @blocks.each do |block| + block.attachment.remove_blank_strings if blank? + block.attachment.freeze end - @blocks.each { |block| block.attachment.freeze } end def unknown_tag(tag, markup, tokens) From 10ea6144e0b083e894924e18a5a2319b085fd231 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Tue, 27 Oct 2020 11:00:04 -0400 Subject: [PATCH 5/9] Add Liquid::ParseContext#parse_expression for liquid-c node disabling (#1333) We would like to be able to disable liquid-c VM rendering at runtime, but right now expression parsing is done using Expression.parse, which isn't aware of the parse context. That prevents us from conditionally compiling to VM code based on a parse option. --- lib/liquid/condition.rb | 4 ++-- lib/liquid/parse_context.rb | 4 ++++ lib/liquid/tag.rb | 6 ++++++ lib/liquid/tags/case.rb | 4 ++-- lib/liquid/tags/cycle.rb | 4 ++-- lib/liquid/tags/for.rb | 8 ++++---- lib/liquid/tags/if.rb | 2 +- lib/liquid/tags/include.rb | 6 +++--- lib/liquid/tags/render.rb | 6 +++--- lib/liquid/tags/table_row.rb | 4 ++-- 10 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index 56900c7..8b7dd65 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -45,8 +45,8 @@ module Liquid @@operators end - def self.parse_expression(markup) - @@method_literals[markup] || Expression.parse(markup) + def self.parse_expression(parse_context, markup) + @@method_literals[markup] || parse_context.parse_expression(markup) end attr_reader :attachment, :child_condition diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 4724ec2..f8fb9b0 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -23,6 +23,10 @@ module Liquid Liquid::BlockBody.new end + def parse_expression(markup) + Expression.parse(markup) + end + def partial=(value) @partial = value @options = value ? partial_options : @template_options diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index bd77335..085c0ef 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -55,5 +55,11 @@ module Liquid def blank? false end + + private + + def parse_expression(markup) + parse_context.parse_expression(markup) + end end end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index b46fa05..5dc3cb3 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -12,7 +12,7 @@ module Liquid @blocks = [] if markup =~ Syntax - @left = Expression.parse(Regexp.last_match(1)) + @left = parse_expression(Regexp.last_match(1)) else raise SyntaxError, options[:locale].t("errors.syntax.case") end @@ -68,7 +68,7 @@ module Liquid markup = Regexp.last_match(2) - block = Condition.new(@left, '==', Condition.parse_expression(Regexp.last_match(1))) + block = Condition.new(@left, '==', Condition.parse_expression(parse_context, Regexp.last_match(1))) block.attach(body) @blocks << block end diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 6aee14c..96f0d57 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -24,7 +24,7 @@ module Liquid case markup when NamedSyntax @variables = variables_from_string(Regexp.last_match(2)) - @name = Expression.parse(Regexp.last_match(1)) + @name = parse_expression(Regexp.last_match(1)) when SimpleSyntax @variables = variables_from_string(markup) @name = @variables.to_s @@ -61,7 +61,7 @@ module Liquid def variables_from_string(markup) markup.split(',').collect do |var| var =~ /\s*(#{QuotedFragment})\s*/o - Regexp.last_match(1) ? Expression.parse(Regexp.last_match(1)) : nil + Regexp.last_match(1) ? parse_expression(Regexp.last_match(1)) : nil end.compact end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 6f66ebd..d92fa68 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -97,7 +97,7 @@ module Liquid collection_name = Regexp.last_match(2) @reversed = !!Regexp.last_match(3) @name = "#{@variable_name}-#{collection_name}" - @collection_name = Expression.parse(collection_name) + @collection_name = parse_expression(collection_name) markup.scan(TagAttributes) do |key, value| set_attribute(key, value) end @@ -112,7 +112,7 @@ module Liquid raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') collection_name = p.expression - @collection_name = Expression.parse(collection_name) + @collection_name = parse_expression(collection_name) @name = "#{@variable_name}-#{collection_name}" @reversed = p.id?('reversed') @@ -198,10 +198,10 @@ module Liquid @from = if expr == 'continue' :continue else - Expression.parse(expr) + parse_expression(expr) end when 'limit' - @limit = Expression.parse(expr) + @limit = parse_expression(expr) end end diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index aa437b1..90692fd 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -71,7 +71,7 @@ module Liquid end def parse_expression(markup) - Condition.parse_expression(markup) + Condition.parse_expression(parse_context, markup) end def lax_parse(markup) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index f697e51..1a31544 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -32,12 +32,12 @@ module Liquid variable_name = Regexp.last_match(3) @alias_name = Regexp.last_match(5) - @variable_name_expr = variable_name ? Expression.parse(variable_name) : nil - @template_name_expr = Expression.parse(template_name) + @variable_name_expr = variable_name ? parse_expression(variable_name) : nil + @template_name_expr = parse_expression(template_name) @attributes = {} markup.scan(TagAttributes) do |key, value| - @attributes[key] = Expression.parse(value) + @attributes[key] = parse_expression(value) end else diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 8ce560a..6380249 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -19,13 +19,13 @@ module Liquid variable_name = Regexp.last_match(4) @alias_name = Regexp.last_match(6) - @variable_name_expr = variable_name ? Expression.parse(variable_name) : nil - @template_name_expr = Expression.parse(template_name) + @variable_name_expr = variable_name ? parse_expression(variable_name) : nil + @template_name_expr = parse_expression(template_name) @for = (with_or_for == FOR) @attributes = {} markup.scan(TagAttributes) do |key, value| - @attributes[key] = Expression.parse(value) + @attributes[key] = parse_expression(value) end end diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index 353f9e3..eda0bc4 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -10,10 +10,10 @@ module Liquid super if markup =~ Syntax @variable_name = Regexp.last_match(1) - @collection_name = Expression.parse(Regexp.last_match(2)) + @collection_name = parse_expression(Regexp.last_match(2)) @attributes = {} markup.scan(TagAttributes) do |key, value| - @attributes[key] = Expression.parse(value) + @attributes[key] = parse_expression(value) end else raise SyntaxError, options[:locale].t("errors.syntax.table_row") From f23c2a83f25bf2c9755bfd7d2402f9ee3c95516f Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Tue, 27 Oct 2020 14:53:57 -0400 Subject: [PATCH 6/9] Fix lax parsing expressions surrounded by spaces (#1335) to make it compatible with strict parsing and liquid-c --- lib/liquid/expression.rb | 35 ++++++++++++---------- test/integration/expression_test.rb | 46 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 16 deletions(-) create mode 100644 test/integration/expression_test.rb diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index fdbe382..69c7472 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -10,25 +10,28 @@ module Liquid 'empty' => '' }.freeze - SINGLE_QUOTED_STRING = /\A'(.*)'\z/m - DOUBLE_QUOTED_STRING = /\A"(.*)"\z/m - INTEGERS_REGEX = /\A(-?\d+)\z/ - FLOATS_REGEX = /\A(-?\d[\d\.]+)\z/ - RANGES_REGEX = /\A\((\S+)\.\.(\S+)\)\z/ + 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/ + RANGES_REGEX = /\A\s*\(\s*(\S+)\s*\.\.\s*(\S+)\s*\)\s*\z/ def self.parse(markup) - if LITERALS.key?(markup) - LITERALS[markup] + 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 + RangeLookup.parse(Regexp.last_match(1), Regexp.last_match(2)) + when FLOATS_REGEX + Regexp.last_match(1).to_f else - case markup - when SINGLE_QUOTED_STRING, DOUBLE_QUOTED_STRING - Regexp.last_match(1) - when INTEGERS_REGEX - Regexp.last_match(1).to_i - when RANGES_REGEX - RangeLookup.parse(Regexp.last_match(1), Regexp.last_match(2)) - when FLOATS_REGEX - Regexp.last_match(1).to_f + markup = markup.strip + if LITERALS.key?(markup) + LITERALS[markup] else VariableLookup.parse(markup) end diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb new file mode 100644 index 0000000..91a0494 --- /dev/null +++ b/test/integration/expression_test.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'test_helper' + +class ExpressionTest < Minitest::Test + def test_keyword_literals + assert_equal(true, parse_and_eval("true")) + assert_equal(true, parse_and_eval(" true ")) + end + + def test_string + assert_equal("single quoted", parse_and_eval("'single quoted'")) + assert_equal("double quoted", parse_and_eval('"double quoted"')) + assert_equal("spaced", parse_and_eval(" 'spaced' ")) + assert_equal("spaced2", parse_and_eval(' "spaced2" ')) + end + + def test_int + assert_equal(123, parse_and_eval("123")) + assert_equal(456, parse_and_eval(" 456 ")) + assert_equal(12, parse_and_eval("012")) + end + + def test_float + assert_equal(1.5, parse_and_eval("1.5")) + assert_equal(2.5, parse_and_eval(" 2.5 ")) + end + + def test_range + assert_equal(1..2, parse_and_eval("(1..2)")) + assert_equal(3..4, parse_and_eval(" ( 3 .. 4 ) ")) + end + + private + + def parse_and_eval(markup, **assigns) + if Liquid::Template.error_mode == :strict + p = Liquid::Parser.new(markup) + markup = p.expression + p.consume(:end_of_string) + end + expression = Liquid::Expression.parse(markup) + context = Liquid::Context.new(assigns) + context.evaluate(expression) + end +end From 26640368e5e1fac5aa66a8f26bdd43b50bac9cc8 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 28 Oct 2020 12:45:10 +0530 Subject: [PATCH 7/9] Run workflows for pull requests from repo forks --- .github/workflows/liquid.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/liquid.yml b/.github/workflows/liquid.yml index 00c51a4..c9101eb 100644 --- a/.github/workflows/liquid.yml +++ b/.github/workflows/liquid.yml @@ -1,5 +1,5 @@ name: Liquid -on: [push] +on: [push, pull_request] jobs: test: runs-on: ubuntu-latest From 1d63d5db5fe61320366f7e2c6c6b8d800b930ac5 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Wed, 28 Oct 2020 10:36:33 -0400 Subject: [PATCH 8/9] Fix a leaky test that set Tempate.error_mode without resetting it (#1339) --- test/integration/template_test.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 8cc3d79..f13d834 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -257,9 +257,8 @@ class TemplateTest < Minitest::Test end def test_nil_value_does_not_raise - Liquid::Template.error_mode = :strict - t = Template.parse("some{{x}}thing") - result = t.render!({ 'x' => nil }, strict_variables: true) + t = Template.parse("some{{x}}thing", error_mode: :strict) + result = t.render!({ 'x' => nil }, strict_variables: true) assert_equal(0, t.errors.count) assert_equal('something', result) From 7754d5aef57134fd9fe6c36a40f9a5514ecf2874 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Wed, 28 Oct 2020 10:37:00 -0400 Subject: [PATCH 9/9] Attempt to strict parse variables before lax parsing in lax error mode (#1338) --- lib/liquid/parser_switching.rb | 12 ++++++++++++ lib/liquid/variable.rb | 2 +- test/integration/assign_test.rb | 5 +++++ test/integration/variable_test.rb | 5 +++++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index 1a8d4ff..78afd58 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -2,6 +2,18 @@ module Liquid module ParserSwitching + def strict_parse_with_error_mode_fallback(markup) + strict_parse_with_error_context(markup) + rescue SyntaxError => e + case parse_context.error_mode + when :strict + raise + when :warn + parse_context.warnings << e + end + lax_parse(markup) + end + def parse_with_selected_parser(markup) case parse_context.error_mode when :strict then strict_parse_with_error_context(markup) diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 77d6b0c..891425c 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -30,7 +30,7 @@ module Liquid @parse_context = parse_context @line_number = parse_context.line_number - parse_with_selected_parser(markup) + strict_parse_with_error_mode_fallback(markup) end def raw diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index b956fd1..c5a8dd9 100644 --- a/test/integration/assign_test.rb +++ b/test/integration/assign_test.rb @@ -48,6 +48,11 @@ class AssignTest < Minitest::Test end end + def test_expression_with_whitespace_in_square_brackets + source = "{% assign r = a[ 'b' ] %}{{ r }}" + assert_template_result('result', source, 'a' => { 'b' => 'result' }) + end + def test_assign_score_exceeding_resource_limit t = Template.parse("{% assign foo = 42 %}{% assign bar = 23 %}") t.resource_limits.assign_score_limit = 1 diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index 168f43a..ad1b28e 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -21,6 +21,11 @@ class VariableTest < Minitest::Test assert_equal(' worked wonderfully ', template.render!('test' => 'worked wonderfully')) end + def test_expression_with_whitespace_in_square_brackets + assert_template_result('result', "{{ a[ 'b' ] }}", 'a' => { 'b' => 'result' }) + assert_template_result('result', "{{ a[ [ 'b' ] ] }}", 'b' => 'c', 'a' => { 'c' => 'result' }) + end + def test_ignore_unknown template = Template.parse(%({{ test }})) assert_equal('', template.render!)