Introduce :rigid parsing mode

History.md

@@ -1,5 +1,8 @@
 # Liquid Change Log
 
+## 5.9.0
+* Introduce `:rigid` error mode for stricter, safer parsing of all tags [CP Clermont, Guilherme Carreiro]
+
 ## 5.8.7
 * Expose body content in the `Doc` tag [James Meng]
 

README.md

@@ -99,14 +99,14 @@ Setting the error mode of Liquid lets you specify how strictly you want your tem
 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.
 
-Liquid also comes with a stricter parser that can be used when editing templates to give better error messages
+Liquid also comes with different parsers that can be used when editing templates to give better error messages
 when templates are invalid. You can enable this new parser like this:
 
 ```ruby
-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  # Raises a SyntaxError when invalid syntax is used in all tags
+Liquid::Environment.default.error_mode = :strict # Raises a SyntaxError when invalid syntax is used in some tags
+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.
 ```
 
 If you want to set the error mode only on specific templates you can pass `:error_mode` as an option to `parse`:

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 lax, strict, and rigid parsers')
 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
 
@@ -80,8 +88,13 @@ namespace :benchmark do
     ruby "./performance/benchmark.rb strict"
   end
 
-  desc "Run the liquid benchmark with both lax and strict parsing"
-  task run: [:lax, :strict]
+  desc "Run the liquid benchmark with rigid parsing"
+  task :rigid do
+    ruby "./performance/benchmark.rb rigid"
+  end
+
+  desc "Run the liquid benchmark with lax, strict, and rigid parsing"
+  task run: [:lax, :strict, :rigid]
 
   desc "Run unit benchmarks"
   namespace :unit do

bin/render

@@ -0,0 +1,46 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'bundler/setup'
+require 'liquid'
+
+class VirtualFileSystem
+  def initialize
+    snippet_1 = <<~LIQUID
+      <h1>
+        {{- greating | default: 'Hello' }}, {{ name | default: 'world' -}}!
+      </h1>
+    LIQUID
+    snippet_2 = <<~LIQUID
+      {%- for i in (1..5) -%}
+        > {{ i }}
+      {%- endfor -%}
+    LIQUID
+
+    @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
+
+def source
+  File.read(ARGV[0])
+rescue StandardError
+  'Usage: bin/render example/server/templates/index.liquid'
+end
+
+def assigns
+  {
+    'date' => Time.now,
+  }
+end
+
+puts Liquid::Template
+  .parse(source, error_mode: :rigid)
+  .tap { |t| t.registers[:file_system] = VirtualFileSystem.new }
+  .render(assigns)

lib/liquid/condition.rb

@@ -48,8 +48,8 @@ def self.operators
       @@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

lib/liquid/environment.rb

@@ -34,7 +34,7 @@ class << self
       # @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 :rigid, :strict, :warn, or :lax).
       # @param exception_renderer [Proc] The exception renderer that is used to
       #   render exceptions.
       # @yieldparam environment [Environment] The environment instance that is being built.

lib/liquid/expression.rb

@@ -28,6 +28,10 @@ class Expression
     FLOAT_REGEX = /\A(-?\d+)\.\d+\z/
 
     class << self
+      def safe_parse(parser, ss = StringScanner.new(""), cache = nil)
+        parse(parser.expression, ss, cache)
+      end
+
       def parse(markup, ss = StringScanner.new(""), cache = nil)
         return unless markup
 

lib/liquid/locales/en.yml

@@ -20,6 +20,7 @@
       invalid_template_encoding: "Invalid template encoding"
       render: "Syntax error in tag 'render' - Template name must be a quoted string"
       table_row: "Syntax Error in 'table_row loop' - Valid syntax: table_row [item] in [collection] cols=3"
+      table_row_invalid_attribute: "Invalid attribute '%{attribute}' in tablerow loop. Valid attributes are cols, limit, offset, and range"
       tag_never_closed: "'%{block_name}' tag was never closed"
       tag_termination: "Tag '%{token}' was not properly terminated with regexp: %{tag_end}"
       unexpected_else: "%{block_name} tag does not expect 'else' tag"

lib/liquid/parse_context.rb

@@ -50,7 +50,22 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
       )
     end
 
-    def parse_expression(markup)
+    def safe_parse_expression(parser)
+      Expression.safe_parse(parser, @string_scanner, @expression_cache)
+    end
+
+    def parse_expression(markup, safe: false)
+      if !safe && @error_mode == :rigid
+        # parse_expression is a widely used API. To maintain backward
+        # compatibility while raising awareness about rigid parser standards,
+        # the safe flag supports API users make a deliberate decision.
+        #
+        # In rigid mode, markup MUST come from a string returned by the parser
+        # (e.g., parser.expression). We're not calling the parser here to
+        # prevent redundant parser overhead.
+        raise Liquid::InternalError, "unsafe parse_expression cannot be used in rigid mode"
+      end
+
       Expression.parse(markup, @string_scanner, @expression_cache)
     end
 

lib/liquid/parser_switching.rb

@@ -2,10 +2,22 @@
 
 module Liquid
   module ParserSwitching
+    # Do not use this.
+    #
+    # It's basically doing the same thing the {#parse_with_selected_parser},
+    # except this will try the strict parser regardless of the error mode,
+    # and fall back to the lax parser if the error mode is lax or warn,
+    # except when in rigid mode where it uses the rigid parser.
+    #
+    # @deprecated Use {#parse_with_selected_parser} instead.
     def strict_parse_with_error_mode_fallback(markup)
+      return rigid_parse_with_error_context(markup) if rigid_mode?
+
       strict_parse_with_error_context(markup)
     rescue SyntaxError => e
       case parse_context.error_mode
+      when :rigid
+        raise
       when :strict
         raise
       when :warn
@@ -16,20 +28,33 @@ def strict_parse_with_error_mode_fallback(markup)
 
     def parse_with_selected_parser(markup)
       case parse_context.error_mode
+      when :rigid  then rigid_parse_with_error_context(markup)
       when :strict then strict_parse_with_error_context(markup)
       when :lax    then lax_parse(markup)
       when :warn
         begin
-          strict_parse_with_error_context(markup)
+          rigid_parse_with_error_context(markup)
         rescue SyntaxError => e
           parse_context.warnings << e
           lax_parse(markup)
         end
       end
     end
 
+    def rigid_mode?
+      parse_context.error_mode == :rigid
+    end
+
     private
 
+    def rigid_parse_with_error_context(markup)
+      rigid_parse(markup)
+    rescue SyntaxError => e
+      e.line_number    = line_number
+      e.markup_context = markup_context(markup)
+      raise e
+    end
+
     def strict_parse_with_error_context(markup)
       strict_parse(markup)
     rescue SyntaxError => e

lib/liquid/tag.rb

@@ -68,8 +68,12 @@ def blank?
 
     private
 
-    def parse_expression(markup)
-      parse_context.parse_expression(markup)
+    def safe_parse_expression(parser)
+      parse_context.safe_parse_expression(parser)
+    end
+
+    def parse_expression(markup, safe: false)
+      parse_context.parse_expression(markup, safe: safe)
     end
   end
 end

lib/liquid/tags/case.rb

@@ -31,12 +31,7 @@ class Case < Block
     def initialize(tag_name, markup, options)
       super
       @blocks = []
-
-      if markup =~ Syntax
-        @left = parse_expression(Regexp.last_match(1))
-      else
-        raise SyntaxError, options[:locale].t("errors.syntax.case")
-      end
+      parse_with_selected_parser(markup)
     end
 
     def parse(tokens)
@@ -91,9 +86,50 @@ def render_to_output_buffer(context, output)
 
     private
 
+    def rigid_parse(markup)
+      parser = @parse_context.new_parser(markup)
+      @left = safe_parse_expression(parser)
+      parser.consume(:end_of_string)
+    end
+
+    def strict_parse(markup)
+      lax_parse(markup)
+    end
+
+    def lax_parse(markup)
+      if markup =~ Syntax
+        @left = parse_expression(Regexp.last_match(1))
+      else
+        raise SyntaxError, options[:locale].t("errors.syntax.case")
+      end
+    end
+
     def record_when_condition(markup)
       body = new_body
 
+      if rigid_mode?
+        parse_rigid_when(markup, body)
+      else
+        parse_lax_when(markup, body)
+      end
+    end
+
+    def parse_rigid_when(markup, body)
+      parser = @parse_context.new_parser(markup)
+
+      loop do
+        expr = safe_parse_expression(parser)
+        block = Condition.new(@left, '==', expr)
+        block.attach(body)
+        @blocks << block
+
+        break unless parser.id?('or') || parser.consume?(:comma)
+      end
+
+      parser.consume(:end_of_string)
+    end
+
+    def parse_lax_when(markup, body)
       while markup
         unless markup =~ WhenSyntax
           raise SyntaxError, options[:locale].t("errors.syntax.case_invalid_when")