diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 277d349e..d314448c 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -52,26 +52,21 @@ module Liquid def parse_expression(markup) if @error_mode == :rigid - parser = new_parser(markup) - - # Return nil immediately if the markup is empty or contains only - # whitespaces - return if parser.look(:end_of_string) - - expression_string = parser.expression - - # In rigid mode, verify that all tokens have been consumed + # ExpressionParser doesn't use @expression_cache because rigid mode + # must run Lexer and Parser validation on every call to ensure all + # tokens are valid and properly consumed. # - # Extra tokens remaining after the expression indicate invalid syntaxes, - # such as: "product title" (instead of "product.title") - parser.consume(:end_of_string) unless parser.look(:end_of_string) - - # Use Parser for strict token validation, but still return - # Expression objects for compatibility with the rendering pipeline. - markup = expression_string + # The expensive operations (tokenization and validation) cannot be + # cached, while the cheap operation (building Expression objects from + # validated tokens) provides minimal benefit from caching. + # + # Most importantly, caching would skip the validation step entirely, + # which defeats the core purpose of rigid mode: strict validation of + # every expression to catch syntax errors like "product title". + ExpressionParser.parse(markup, self) + else + Expression.parse(markup, @string_scanner, @expression_cache) end - - Expression.parse(markup, @string_scanner, @expression_cache) end def partial=(value) diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 29e45632..4ee164b8 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -42,14 +42,25 @@ class ExpressionTest < Minitest::Test assert_template_result("3..4", "{{ ( 3 .. 4 ) }}") assert_expression_result(1..2, "(1..2)") - assert_match_syntax_error( - "Liquid syntax error (line 1): Invalid expression type 'false' in range expression", - "{{ (false..true) }}", - ) - assert_match_syntax_error( - "Liquid syntax error (line 1): Invalid expression type '(1..2)' in range expression", - "{{ ((1..2)..3) }}", - ) + if Liquid::Environment.default.error_mode == :rigid + assert_match_syntax_error( + 'Invalid expression type in range expression in "{{ (false..true) }}"', + "{{ (false..true) }}", + ) + assert_match_syntax_error( + 'Liquid syntax error (line 1): Invalid expression type in range expression in "{{ ((1..2)..3) }}"', + "{{ ((1..2)..3) }}", + ) + else + assert_match_syntax_error( + "Liquid syntax error (line 1): Invalid expression type 'false' in range expression", + "{{ (false..true) }}", + ) + assert_match_syntax_error( + "Liquid syntax error (line 1): Invalid expression type '(1..2)' in range expression", + "{{ ((1..2)..3) }}", + ) + end end def test_quirky_negative_sign_expression_markup @@ -66,6 +77,7 @@ class ExpressionTest < Minitest::Test def test_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled + skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid cache = {} template = <<~LIQUID @@ -87,6 +99,7 @@ class ExpressionTest < Minitest::Test def test_expression_cache_with_true_boolean skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled + skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid template = <<~LIQUID {% assign x = 1 %} @@ -111,6 +124,7 @@ class ExpressionTest < Minitest::Test def test_expression_cache_with_lru_redux skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled + skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid cache = LruRedux::Cache.new(10) template = <<~LIQUID @@ -132,6 +146,7 @@ class ExpressionTest < Minitest::Test def test_disable_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled + skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid template = <<~LIQUID {% assign x = 1 %}