From 94bbf6ca32295dac13cfed79a2c6a9b109421a4e Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 7 Oct 2025 13:50:48 -0400 Subject: [PATCH] Make it possible to safe_parse subsets of expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e.g. sometimes you want to only accept strings | lookups. {% render snippetName %} for example. snippetName is a string right now. We don't want safe_parse_expression because this would allow snippetName to be a number, a boolean, etc. But we still want to strict parse this. So what we'll do is use parse_expression(string, safe: true), this is an optional opt-in to say "I know what I'm doing". Usually that's because you're using the output of Parser#something as the input of parse_expression. It is true that Parser#expression is subset of Expression.parse, it is not true of the opposite (e.g. Expression.parse doesn't care about .5 and happily parses that as a global lookup of the variable named "5", Parser#expression throws for that.) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index e5c321dc..9ab350f0 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -48,8 +48,8 @@ module Liquid @@operators end - def self.parse_expression(parse_context, markup) - @@method_literals[markup] || parse_context.parse_expression(markup) + def self.parse_expression(parse_context, markup, safe: false) + @@method_literals[markup] || parse_context.parse_expression(markup, safe: safe) end attr_reader :attachment, :child_condition diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 1c59fe4a..82cf5768 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -51,13 +51,13 @@ module Liquid end def safe_parse_expression(parser) - Expression.safe_parse(parser) + Expression.safe_parse(parser, @string_scanner, @expression_cache) end - def parse_expression(markup) + def parse_expression(markup, safe: false) # todo(guilherme): remove this once rigid mode is fully using safe_parse_expression - # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" if @error_mode == :rigid - puts("🚨 parse_expression used in rigid mode") if @error_mode == :rigid + # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" if !safe && @error_mode == :rigid + puts("🚨 parse_expression used in rigid mode") if !safe && @error_mode == :rigid Expression.parse(markup, @string_scanner, @expression_cache) end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 656d2e47..374ee511 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -72,8 +72,8 @@ module Liquid parse_context.safe_parse_expression(parser) end - def parse_expression(markup) - parse_context.parse_expression(markup) + def parse_expression(markup, safe: false) + parse_context.parse_expression(markup, safe: safe) end end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index c2be5db1..3182983b 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -93,7 +93,7 @@ module Liquid raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') collection_name = p.expression - @collection_name = parse_expression(collection_name) + @collection_name = parse_expression(collection_name, safe: true) @name = "#{@variable_name}-#{collection_name}" @reversed = p.id?('reversed') diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 342374f1..e25d6250 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -81,8 +81,8 @@ module Liquid block.attach(new_body) end - def parse_expression(markup) - Condition.parse_expression(parse_context, markup) + def parse_expression(markup, safe: false) + Condition.parse_expression(parse_context, markup, safe: safe) end def lax_parse(markup) @@ -124,9 +124,9 @@ module Liquid end def parse_comparison(p) - a = parse_expression(p.expression) + a = parse_expression(p.expression, safe: true) if (op = p.consume?(:comparison)) - b = parse_expression(p.expression) + b = parse_expression(p.expression, safe: true) Condition.new(a, op, b) else Condition.new(a) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 6cdbfd6f..b72a235b 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -87,10 +87,11 @@ module Liquid def rigid_parse(markup) p = @parse_context.new_parser(markup) - template_name = p.expression + @template_name_expr = safe_parse_expression(p) with_or_for = p.id?("for") || p.id?("with") || nil + @variable_name_expr = nil if with_or_for - variable_name = p.expression + @variable_name_expr = parse_expression(p.consume(:id), safe: true) end alias_name = nil @@ -98,8 +99,6 @@ module Liquid alias_name = p.consume(:id) end - @template_name_expr = parse_expression(template_name) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil @alias_name = alias_name # optional comma @@ -109,7 +108,7 @@ module Liquid while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = parse_expression(p.expression) + @attributes[key] = safe_parse_expression(p) p.consume?(:comma) # optional comma end end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 89c11063..4f716b24 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -88,10 +88,11 @@ module Liquid def rigid_parse(markup) p = @parse_context.new_parser(markup) - template_name = rigid_template_name(p) + @template_name_expr = parse_expression(rigid_template_name(p), safe: true) + @variable_name_expr = nil with_or_for = p.id?("for") || p.id?("with") || nil if with_or_for - variable_name = p.expression + @variable_name_expr = safe_parse_expression(p) end alias_name = nil @@ -99,8 +100,6 @@ module Liquid alias_name = p.consume(:id) end - @template_name_expr = parse_expression(template_name) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil @alias_name = alias_name @is_for_loop = (with_or_for == FOR) @@ -111,7 +110,7 @@ module Liquid while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = parse_expression(p.expression) + @attributes[key] = safe_parse_expression(p) p.consume?(:comma) # optional comma end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 20957065..a3623bc5 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -65,11 +65,11 @@ module Liquid return if p.look(:end_of_string) - @name = parse_context.parse_expression(p.expression) + @name = parse_context.safe_parse_expression(p) while p.consume?(:pipe) filtername = p.consume(:id) filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << parse_filter_expressions(filtername, filterargs) + @filters << parse_filter_expressions(filtername, filterargs, safe: true) end p.consume(:end_of_string) end @@ -122,15 +122,15 @@ module Liquid private - def parse_filter_expressions(filter_name, unparsed_args) + def parse_filter_expressions(filter_name, unparsed_args, safe: false) filter_args = [] keyword_args = nil unparsed_args.each do |a| - if (matches = a.match(JustTagAttributes)) + if (matches = a.match(JustTagAttributes)) # we'll need to fix this keyword_args ||= {} - keyword_args[matches[1]] = parse_context.parse_expression(matches[2]) + keyword_args[matches[1]] = parse_context.parse_expression(matches[2], safe: false) else - filter_args << parse_context.parse_expression(a) + filter_args << parse_context.parse_expression(a, safe: safe) end end result = [filter_name, filter_args] --- lib/liquid/condition.rb | 4 ++-- lib/liquid/parse_context.rb | 8 ++++---- lib/liquid/tag.rb | 4 ++-- lib/liquid/tags/for.rb | 2 +- lib/liquid/tags/if.rb | 8 ++++---- lib/liquid/tags/include.rb | 9 ++++----- lib/liquid/tags/render.rb | 9 ++++----- lib/liquid/variable.rb | 12 ++++++------ 8 files changed, 27 insertions(+), 29 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index e5c321dc..9ab350f0 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -48,8 +48,8 @@ module Liquid @@operators end - def self.parse_expression(parse_context, markup) - @@method_literals[markup] || parse_context.parse_expression(markup) + def self.parse_expression(parse_context, markup, safe: false) + @@method_literals[markup] || parse_context.parse_expression(markup, safe: safe) end attr_reader :attachment, :child_condition diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 1c59fe4a..82cf5768 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -51,13 +51,13 @@ module Liquid end def safe_parse_expression(parser) - Expression.safe_parse(parser) + Expression.safe_parse(parser, @string_scanner, @expression_cache) end - def parse_expression(markup) + def parse_expression(markup, safe: false) # todo(guilherme): remove this once rigid mode is fully using safe_parse_expression - # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" if @error_mode == :rigid - puts("🚨 parse_expression used in rigid mode") if @error_mode == :rigid + # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" if !safe && @error_mode == :rigid + puts("🚨 parse_expression used in rigid mode") if !safe && @error_mode == :rigid Expression.parse(markup, @string_scanner, @expression_cache) end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 656d2e47..374ee511 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -72,8 +72,8 @@ module Liquid parse_context.safe_parse_expression(parser) end - def parse_expression(markup) - parse_context.parse_expression(markup) + def parse_expression(markup, safe: false) + parse_context.parse_expression(markup, safe: safe) end end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index c2be5db1..3182983b 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -93,7 +93,7 @@ module Liquid raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') collection_name = p.expression - @collection_name = parse_expression(collection_name) + @collection_name = parse_expression(collection_name, safe: true) @name = "#{@variable_name}-#{collection_name}" @reversed = p.id?('reversed') diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 342374f1..e25d6250 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -81,8 +81,8 @@ module Liquid block.attach(new_body) end - def parse_expression(markup) - Condition.parse_expression(parse_context, markup) + def parse_expression(markup, safe: false) + Condition.parse_expression(parse_context, markup, safe: safe) end def lax_parse(markup) @@ -124,9 +124,9 @@ module Liquid end def parse_comparison(p) - a = parse_expression(p.expression) + a = parse_expression(p.expression, safe: true) if (op = p.consume?(:comparison)) - b = parse_expression(p.expression) + b = parse_expression(p.expression, safe: true) Condition.new(a, op, b) else Condition.new(a) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 6cdbfd6f..b72a235b 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -87,10 +87,11 @@ module Liquid def rigid_parse(markup) p = @parse_context.new_parser(markup) - template_name = p.expression + @template_name_expr = safe_parse_expression(p) with_or_for = p.id?("for") || p.id?("with") || nil + @variable_name_expr = nil if with_or_for - variable_name = p.expression + @variable_name_expr = parse_expression(p.consume(:id), safe: true) end alias_name = nil @@ -98,8 +99,6 @@ module Liquid alias_name = p.consume(:id) end - @template_name_expr = parse_expression(template_name) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil @alias_name = alias_name # optional comma @@ -109,7 +108,7 @@ module Liquid while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = parse_expression(p.expression) + @attributes[key] = safe_parse_expression(p) p.consume?(:comma) # optional comma end end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 89c11063..4f716b24 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -88,10 +88,11 @@ module Liquid def rigid_parse(markup) p = @parse_context.new_parser(markup) - template_name = rigid_template_name(p) + @template_name_expr = parse_expression(rigid_template_name(p), safe: true) + @variable_name_expr = nil with_or_for = p.id?("for") || p.id?("with") || nil if with_or_for - variable_name = p.expression + @variable_name_expr = safe_parse_expression(p) end alias_name = nil @@ -99,8 +100,6 @@ module Liquid alias_name = p.consume(:id) end - @template_name_expr = parse_expression(template_name) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil @alias_name = alias_name @is_for_loop = (with_or_for == FOR) @@ -111,7 +110,7 @@ module Liquid while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = parse_expression(p.expression) + @attributes[key] = safe_parse_expression(p) p.consume?(:comma) # optional comma end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 20957065..a3623bc5 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -65,11 +65,11 @@ module Liquid return if p.look(:end_of_string) - @name = parse_context.parse_expression(p.expression) + @name = parse_context.safe_parse_expression(p) while p.consume?(:pipe) filtername = p.consume(:id) filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << parse_filter_expressions(filtername, filterargs) + @filters << parse_filter_expressions(filtername, filterargs, safe: true) end p.consume(:end_of_string) end @@ -122,15 +122,15 @@ module Liquid private - def parse_filter_expressions(filter_name, unparsed_args) + def parse_filter_expressions(filter_name, unparsed_args, safe: false) filter_args = [] keyword_args = nil unparsed_args.each do |a| - if (matches = a.match(JustTagAttributes)) + if (matches = a.match(JustTagAttributes)) # we'll need to fix this keyword_args ||= {} - keyword_args[matches[1]] = parse_context.parse_expression(matches[2]) + keyword_args[matches[1]] = parse_context.parse_expression(matches[2], safe: false) else - filter_args << parse_context.parse_expression(a) + filter_args << parse_context.parse_expression(a, safe: safe) end end result = [filter_name, filter_args]