diff --git a/README.md b/README.md index 3734a290..93a726a9 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,7 @@ Liquid::Environment.default.error_mode = :strict Liquid::Environment.default.error_mode = :strict # Raises a SyntaxError when invalid syntax is used Liquid::Environment.default.error_mode = :warn # Adds strict errors to template.errors but continues as normal Liquid::Environment.default.error_mode = :lax # The default mode, accepts almost anything. +Liquid::Environment.default.error_mode = :rigid # Uses Parser.new instead of Expression.parse for stricter parsing ``` If you want to set the error mode only on specific templates you can pass `:error_mode` as an option to `parse`: diff --git a/Rakefile b/Rakefile index 6ccd2c86..ae41567a 100755 --- a/Rakefile +++ b/Rakefile @@ -33,7 +33,7 @@ task :rubocop do end end -desc('runs test suite with both strict and lax parsers') +desc('runs test suite with all parsers (lax, strict, and rigid)') task :test do ENV['LIQUID_PARSER_MODE'] = 'lax' Rake::Task['base_test'].invoke @@ -42,6 +42,10 @@ task :test do Rake::Task['base_test'].reenable Rake::Task['base_test'].invoke + ENV['LIQUID_PARSER_MODE'] = 'rigid' + Rake::Task['base_test'].reenable + Rake::Task['base_test'].invoke + if RUBY_ENGINE == 'ruby' || RUBY_ENGINE == 'truffleruby' ENV['LIQUID_PARSER_MODE'] = 'lax' Rake::Task['integration_test'].reenable @@ -50,6 +54,10 @@ task :test do ENV['LIQUID_PARSER_MODE'] = 'strict' Rake::Task['integration_test'].reenable Rake::Task['integration_test'].invoke + + ENV['LIQUID_PARSER_MODE'] = 'rigid' + Rake::Task['integration_test'].reenable + Rake::Task['integration_test'].invoke end end diff --git a/bin/example.liquid b/bin/example.liquid new file mode 100644 index 00000000..c4a93aa3 --- /dev/null +++ b/bin/example.liquid @@ -0,0 +1,5 @@ + + {% tablerow i in (1..10) limit: foo=>bar %} + {{ i }} + {% endtablerow %} +
diff --git a/bin/render b/bin/render new file mode 100755 index 00000000..004c103f --- /dev/null +++ b/bin/render @@ -0,0 +1,30 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require 'bundler/setup' +require 'liquid' + +class VirtualFileSystem + def initialize + snippet_1 = '

{{ greating | default: "Hello" }}, {{ name | default: "world" }}!

' + snippet_2 = '{% for i in (1..5) %} > {{ i }}{% endfor %}' + + @templates = { + 'snippet_1' => snippet_1, + 'snippet_2' => snippet_2, + } + end + + def read_template_file(key) + @templates[key] || raise(Liquid::FileSystemError, "No such template '#{key}'") + end +end + +error_mode = :strict +# error_mode = :rigid +file = File.read(ARGV[0]) +template = Liquid::Template.parse(file, error_mode: error_mode) + +template.registers[:file_system] = VirtualFileSystem.new + +puts template.render diff --git a/lib/liquid/environment.rb b/lib/liquid/environment.rb index 31b17b23..987ec13f 100644 --- a/lib/liquid/environment.rb +++ b/lib/liquid/environment.rb @@ -34,7 +34,7 @@ module Liquid # @param file_system The default file system that is used # to load templates from. # @param error_mode [Symbol] The default error mode for all templates - # (either :strict, :warn, or :lax). + # (either :strict, :warn, :lax, or :rigid). # @param exception_renderer [Proc] The exception renderer that is used to # render exceptions. # @yieldparam environment [Environment] The environment instance that is being built. diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 60cdf9e4..277d349e 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -51,6 +51,26 @@ module Liquid end 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 + # + # 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 + end + Expression.parse(markup, @string_scanner, @expression_cache) end diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index 78afd58a..e1047632 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -8,6 +8,8 @@ module Liquid case parse_context.error_mode when :strict raise + when :rigid + raise when :warn parse_context.warnings << e end @@ -17,6 +19,7 @@ module Liquid def parse_with_selected_parser(markup) case parse_context.error_mode when :strict then strict_parse_with_error_context(markup) + when :rigid then strict_parse_with_error_context(markup) when :lax then lax_parse(markup) when :warn begin diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb index b081afe5..31790b0d 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -184,7 +184,7 @@ class PartialCacheUnitTest < Minitest::Test }, ) - [:lax, :warn, :strict].each do |error_mode| + [:lax, :warn, :strict, :rigid].each do |error_mode| Liquid::PartialCache.load( 'my_partial', context: context, @@ -193,7 +193,7 @@ class PartialCacheUnitTest < Minitest::Test end assert_equal( - ["my_partial:lax", "my_partial:warn", "my_partial:strict"], + ["my_partial:lax", "my_partial:warn", "my_partial:strict", "my_partial:rigid"], context.registers[:cached_partials].keys, ) end diff --git a/test/unit/rigid_mode_unit_test.rb b/test/unit/rigid_mode_unit_test.rb new file mode 100644 index 00000000..d1cd615e --- /dev/null +++ b/test/unit/rigid_mode_unit_test.rb @@ -0,0 +1,264 @@ +# frozen_string_literal: true + +require 'test_helper' + +class RigidModeUnitTest < Minitest::Test + include Liquid + + def test_direct_parse_expression_comparison + test_cases = [ + 'foo bar', + 'user.name first', + 'items[0] next', + 'products[0].name extra', + ] + + test_cases.each do |expr| + ctx_strict = ParseContext.new(environment: strict_env) + result = ctx_strict.parse_expression(expr) + refute_nil(result, "Strict mode should parse '#{expr}'") + + ctx_rigid = ParseContext.new(environment: rigid_env) + error = assert_raises(SyntaxError) do + ctx_rigid.parse_expression(expr) + end + + assert_match(/Expected end_of_string but found id/, error.message) + end + end + + def test_comparison_strict_vs_rigid_with_space_separated_lookups + expr = 'product title' + + ctx_lax = ParseContext.new(environment: lax_env) + result_lax = ctx_lax.parse_expression(expr) + assert_equal('product', result_lax.name) + assert_equal(['title'], result_lax.lookups) + + ctx_strict = ParseContext.new(environment: strict_env) + result_strict = ctx_strict.parse_expression(expr) + assert_equal('product', result_strict.name) + assert_equal(['title'], result_strict.lookups) + + ctx_rigid = ParseContext.new(environment: rigid_env) + assert_raises(SyntaxError) do + ctx_rigid.parse_expression(expr) + end + end + + def test_tablerow_limit_with_invalid_expression + template = <<~LIQUID + {% tablerow i in (1..10) limit: foo=>bar %}{{ i }}{% endtablerow %} + LIQUID + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_tablerow_offset_with_invalid_expression + template = <<~LIQUID + {% tablerow i in (1..10) offset: foo=>bar %}{{ i }}{% endtablerow %} + LIQUID + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_cycle_name_with_invalid_expression + template = <<~LIQUID + {% for i in (1..3) %} + {% cycle foo=>bar: "a", "b" %} + {% endfor %} + LIQUID + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_cycle_variable_with_invalid_expression + template = <<~LIQUID + {% for i in (1..3) %} + {% cycle foo=>bar, "a", "b" %} + {% endfor %} + LIQUID + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_case_with_invalid_expression + template = <<~LIQUID + {% case foo=>bar %} + {% when 1 %} + one + {% endcase %} + LIQUID + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_include_template_with_invalid_expression + template = "{% include foo=>bar %}" + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_include_with_invalid_expression + template = '{% include "snippet" with foo=>bar %}' + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_include_attribute_with_invalid_expression + template = '{% include "snippet", key: foo=>bar %}' + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_render_with_invalid_expression + template = '{% render "snippet" with foo=>bar %}' + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_render_attribute_with_invalid_expression + template = '{% render "snippet", key: foo=>bar %}' + + refute_nil(strict_parse(template)) + + error = assert_raises(SyntaxError) do + rigid_parse(template) + end + assert_match(/Unexpected character =/, error.message) + end + + def test_valid_expressions_work_in_rigid_mode + test_cases = { + '{{ foo }}' => { 'foo' => 'bar' }, + '{{ foo.bar }}' => { 'foo' => { 'bar' => 'baz' } }, + '{{ items[0] }}' => { 'items' => ['first', 'second'] }, + '{{ product.variants[0].title }}' => { 'product' => { 'variants' => [{ 'title' => 'Small' }] } }, + '{{ "hello" }}' => {}, + '{{ 42 }}' => {}, + '{{ 3.14 }}' => {}, + } + + test_cases.each do |template_str, data| + t = rigid_parse(template_str) + result = t.render(data) + assert(result.is_a?(String), "Should render successfully for '#{template_str}'") + end + end + + def test_rigid_mode_with_ranges + template = <<~LIQUID + {% for i in (1..3) %}{{ i }}{% endfor %} + LIQUID + + t = rigid_parse(template) + result = t.render + assert_equal("123\n", result) + end + + def test_rigid_mode_with_variable_ranges + template = <<~LIQUID + {% for i in (start..end) %}{{ i }}{% endfor %} + LIQUID + + t = rigid_parse(template) + result = t.render({ 'start' => 1, 'end' => 3 }) + assert_equal("123\n", result) + end + + def test_rigid_mode_valid_filters + template = <<~LIQUID + {{ "hello" | upcase | prepend: "Say: " }} + LIQUID + + t = rigid_parse(template) + result = t.render + assert_equal("Say: HELLO\n", result) + end + + def test_rigid_mode_valid_filter_with_correct_variable_args + template = <<~LIQUID + {{ "hello" | append: world.name }} + LIQUID + + t = rigid_parse(template) + result = t.render({ 'world' => { 'name' => ' world' } }) + assert_equal("hello world\n", result) + end + + def test_empty_expression_handling + ctx_rigid = ParseContext.new(environment: rigid_env) + result = ctx_rigid.parse_expression('') + assert_nil(result) + + result = ctx_rigid.parse_expression(' ') + assert_nil(result) + end + + private + + def rigid_parse(source) + Template.parse(source, environment: rigid_env) + end + + def strict_parse(source) + Template.parse(source, environment: strict_env) + end + + def lax_env + Environment.build(error_mode: :lax) + end + + def rigid_env + Environment.build(error_mode: :rigid) + end + + def strict_env + Environment.build(error_mode: :strict) + end +end