use StringScanner to improve Expression Parsing and Tokenizer

Gemfile

@@ -24,3 +24,6 @@ group :test do
   gem 'rubocop-shopify', '~> 2.12.0', require: false
   gem 'rubocop-performance', require: false
 end
+
+gem "strscan", ">= 3.1"
+gem "lru_redux"

Rakefile

@@ -71,7 +71,7 @@ end
 
 namespace :benchmark do
   desc "Run the liquid benchmark with lax parsing"
-  task :run do
+  task :lax do
     ruby "./performance/benchmark.rb lax"
   end
 
@@ -80,11 +80,30 @@ 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 unit benchmarks"
-  task :unit do
+  namespace :unit do
-    Dir["./performance/unit/*_benchmark.rb"].each do |file|
+    task :all do
-      puts "🧪 Running #{file}"
+      Dir["./performance/unit/*_benchmark.rb"].each do |file|
-      ruby file
+        puts "🧪 Running #{file}"
+        ruby file
+      end
+    end
+
+    task :lexer do
+      Dir["./performance/unit/lexer_benchmark.rb"].each do |file|
+        puts "🧪 Running #{file}"
+        ruby file
+      end
+    end
+
+    task :expression do
+      Dir["./performance/unit/expression_benchmark.rb"].each do |file|
+        puts "🧪 Running #{file}"
+        ruby file
+      end
     end
   end
 end

lib/liquid/expression.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
+require "lru_redux"
+
 module Liquid
-  class Expression
+  class Expression1
     LITERALS = {
       nil => nil,
       'nil' => nil,
@@ -45,4 +47,106 @@ module Liquid
       end
     end
   end
+
+  class Expression2
+    LITERALS = {
+      nil => nil,
+      'nil' => nil,
+      'null' => nil,
+      '' => nil,
+      'true' => true,
+      'false' => false,
+      'blank' => '',
+      'empty' => ''
+    }.freeze
+
+    DOT = ".".ord
+    ZERO = "0".ord
+    NINE = "9".ord
+    DASH = "-".ord
+
+    # Use an atomic group (?>...) to avoid pathological backtracing from
+    # malicious input as described in https://github.com/Shopify/liquid/issues/1357
+    RANGES_REGEX = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/
+    CACHE = LruRedux::Cache.new(1_000_000) # most themes would have less than 2,000 unique expression
+
+    class << self
+      def string_scanner
+        @ss ||= StringScanner.new("")
+      end
+
+      def parse(markup)
+        return unless markup
+
+        markup = markup.strip # markup can be a frozen string
+
+        return CACHE[markup] if CACHE.key?(markup)
+
+        CACHE[markup] = inner_parse(markup)
+      end
+
+      def inner_parse(markup)
+        if (markup.start_with?('"') && markup.end_with?('"')) ||
+          (markup.start_with?("'") && markup.end_with?("'"))
+          return markup[1..-2]
+        elsif (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
+          return RangeLookup.parse(Regexp.last_match(1), Regexp.last_match(2))
+        end
+
+        return LITERALS[markup] if LITERALS.key?(markup)
+
+        if (num = parse_number(markup))
+          num
+        else
+          VariableLookup.parse(markup)
+        end
+      end
+
+      def parse_number(markup)
+        ss = string_scanner
+        ss.string = markup
+
+        is_integer = true
+        last_dot_pos = nil
+        num_end_pos = nil
+
+        # the first byte must be a digit, a period, or  a dash
+        byte = ss.scan_byte
+
+        return false if byte != DASH && byte != DOT && (byte < ZERO || byte > NINE)
+
+        while (byte = ss.scan_byte)
+          return false if byte != DOT && (byte < ZERO || byte > NINE)
+
+          # we found our number and now we are just scanning the rest of the string
+          next if num_end_pos
+
+          if byte == DOT
+            if is_integer == false
+              num_end_pos = ss.pos - 1
+            else
+              is_integer = false
+              last_dot_pos = ss.pos
+            end
+          end
+        end
+
+        num_end_pos = markup.length if ss.eos?
+
+        return markup.to_i if is_integer
+
+        if num_end_pos
+          # number ends with a number "123.123"
+          markup.byteslice(0, num_end_pos).to_f
+        elsif last_dot_pos
+          markup.byteslice(0, last_dot_pos).to_f
+        else
+          # we should never reach this point
+          false
+        end
+      end
+    end
+  end
+
+  Expression = StringScanner.instance_methods.include?(:scan_byte) ? Expression2 : Expression1
 end

lib/liquid/lexer.rb

@@ -92,6 +92,7 @@ module Liquid
     SINGLE_COMPARISON_TOKENS = [].tap do |table|
       table["<".ord] = COMPARISON_LESS_THAN
       table[">".ord] = COMPARISON_GREATER_THAN
+      table.freeze
     end
 
     TWO_CHARS_COMPARISON_JUMP_TABLE = [].tap do |table|
@@ -103,18 +104,17 @@ module Liquid
         sub_table["=".ord] = COMPARISION_NOT_EQUAL
         sub_table.freeze
       end
+      table.freeze
     end
 
     COMPARISON_JUMP_TABLE = [].tap do |table|
       table["<".ord] = [].tap do |sub_table|
         sub_table["=".ord] = COMPARISON_LESS_THAN_OR_EQUAL
         sub_table[">".ord] = COMPARISON_NOT_EQUAL_ALT
-        RUBY_WHITESPACE.each { |c| sub_table[c.ord] = COMPARISON_LESS_THAN }
         sub_table.freeze
       end
       table[">".ord] = [].tap do |sub_table|
         sub_table["=".ord] = COMPARISON_GREATER_THAN_OR_EQUAL
-        RUBY_WHITESPACE.each { |c| sub_table[c.ord] = COMPARISON_GREATER_THAN }
         sub_table.freeze
       end
       table.freeze
@@ -157,8 +157,15 @@ module Liquid
       table.freeze
     end
 
+    class << self
+      def string_scanner
+        @string_scanner ||= StringScanner.new("")
+      end
+    end
+
     def initialize(input)
-      @ss = StringScanner.new(input)
+      @ss = Lexer2.string_scanner
+      @ss.string = input
     end
 
     # rubocop:disable Metrics/BlockNesting
@@ -233,5 +240,6 @@ module Liquid
     end
   end
 
+  # Remove this once we can depend on strscan >= 3.1.1
   Lexer = StringScanner.instance_methods.include?(:scan_byte) ? Lexer2 : Lexer1
 end

lib/liquid/tokenizer.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
+require "strscan"
+
 module Liquid
-  class Tokenizer
+  class Tokenizer1
     attr_reader :line_number, :for_liquid_tag
 
     def initialize(source, line_numbers = false, line_number: nil, for_liquid_tag: false)
@@ -42,4 +44,154 @@ module Liquid
       tokens
     end
   end
+
+  class Tokenizer2
+    attr_reader :line_number, :for_liquid_tag
+
+    TAG_END = /%\}/
+    TAG_OR_VARIABLE_START = /\{[\{\%]/
+    NEWLINE = /\n/
+
+    OPEN_CURLEY = "{".ord
+    CLOSE_CURLEY = "}".ord
+    PERCENTAGE = "%".ord
+
+    class << self
+      def string_scanner
+        @string_scanner ||= StringScanner.new("")
+      end
+    end
+
+    def initialize(source, line_numbers = false, line_number: nil, for_liquid_tag: false)
+      @line_number    = line_number || (line_numbers ? 1 : nil)
+      @for_liquid_tag = for_liquid_tag
+      @source         = source
+      @offset         = 0
+      @tokens         = []
+      tokenize
+    end
+
+    def shift
+      token = @tokens[@offset]
+
+      return unless token
+
+      @offset += 1
+
+      if @line_number
+        @line_number += @for_liquid_tag ? 1 : token.count("\n")
+      end
+
+      token
+    end
+
+    private
+
+    def tokenize
+      if @for_liquid_tag
+        @tokens = @source.split("\n")
+      else
+        @ss = Tokenizer2.string_scanner
+        @ss.string = @source
+        @tokens << shift_normal until @ss.eos?
+      end
+
+      @ss = nil
+      @source = nil
+    end
+
+    def shift_normal
+      token = next_token
+
+      return unless token
+
+      token
+    end
+
+    def next_token
+      # possible states: :text, :tag, :variable
+      byte_a = @ss.peek_byte
+
+      if byte_a == OPEN_CURLEY
+        @ss.scan_byte
+
+        byte_b = @ss.peek_byte
+
+        if byte_b == PERCENTAGE
+          @ss.scan_byte
+          return next_tag_token
+        elsif byte_b == OPEN_CURLEY
+          @ss.scan_byte
+          return next_variable_token
+        end
+
+        @ss.pos -= 1
+      end
+
+      next_text_token
+    end
+
+    def next_text_token
+      start = @ss.pos
+
+      unless @ss.skip_until(TAG_OR_VARIABLE_START)
+        token = @ss.rest
+        @ss.terminate
+        return token
+      end
+
+      pos = @ss.pos -= 2
+      @source.byteslice(start, pos - start)
+    end
+
+    def next_variable_token
+      start = @ss.pos - 2
+
+      byte_a = byte_b = @ss.scan_byte
+
+      while byte_b
+        byte_a = @ss.scan_byte while byte_a && (byte_a != CLOSE_CURLEY && byte_a != OPEN_CURLEY)
+
+        break unless byte_a
+
+        if @ss.eos?
+          return byte_a == CLOSE_CURLEY ? @source.byteslice(start, @ss.pos - start) : "{{"
+        end
+
+        byte_b = @ss.scan_byte
+
+        if byte_a == CLOSE_CURLEY
+          if byte_b == CLOSE_CURLEY
+            return @source.byteslice(start, @ss.pos - start)
+          elsif byte_b != CLOSE_CURLEY
+            @ss.pos -= 1
+            return @source.byteslice(start, @ss.pos - start)
+          end
+        elsif byte_a == OPEN_CURLEY && byte_b == PERCENTAGE
+          return next_tag_token_with_start(start)
+        end
+
+        byte_a = byte_b
+      end
+
+      "{{"
+    end
+
+    def next_tag_token
+      start = @ss.pos - 2
+      if (len = @ss.skip_until(TAG_END))
+        @source.byteslice(start, len + 2)
+      else
+        "{%"
+      end
+    end
+
+    def next_tag_token_with_start(start)
+      @ss.skip_until(TAG_END)
+      @source.byteslice(start, @ss.pos - start)
+    end
+  end
+
+  # Remove this once we can depend on strscan >= 3.1.1
+  Tokenizer = StringScanner.instance_methods.include?(:scan_byte) ? Tokenizer2 : Tokenizer1
 end

performance/benchmark.rb

@@ -9,14 +9,17 @@ Liquid::Environment.default.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
 
 Benchmark.ips do |x|
-  x.time   = 10
+  x.time   = 20
-  x.warmup = 5
+  x.warmup = 10
 
   puts
   puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)."
   puts
 
-  x.report("parse:") { profiler.compile }
+  phase = ENV["PHASE"] || "all"
-  x.report("render:") { profiler.render }
+
-  x.report("parse & render:") { profiler.run }
+  x.report("tokenize:") { profiler.tokenize } if phase == "all" || phase == "tokenize"
+  x.report("parse:") { profiler.compile } if phase == "all" || phase == "parse"
+  x.report("render:") { profiler.render } if phase == "all" || phase == "render"
+  x.report("parse & render:") { profiler.run } if phase == "all" || phase == "run"
 end

performance/theme_runner.rb

@@ -48,6 +48,14 @@ class ThemeRunner
     end
   end
 
+  # `tokenize` will just test the tokenizen portion of liquid without any templates
+  def tokenize
+    @tests.each do |test_hash|
+      tokenizer = Liquid::Tokenizer.new(test_hash[:liquid], true)
+      while tokenizer.shift; end
+    end
+  end
+
   # `run` is called to benchmark rendering and compiling at the same time
   def run
     each_test do |liquid, layout, assigns, page_template, template_name|

performance/unit/expression_benchmark.rb

@@ -0,0 +1,183 @@
+# frozen_string_literal: true
+
+require "benchmark/ips"
+
+# benchmark liquid lexing
+
+require 'liquid'
+
+RubyVM::YJIT.enable
+
+STRING_MARKUPS = [
+  "\"foo\"",
+  "\"fooooooooooo\"",
+  "\"foooooooooooooooooooooooooooooo\"",
+  "'foo'",
+  "'fooooooooooo'",
+  "'foooooooooooooooooooooooooooooo'",
+]
+
+VARIABLE_MARKUPS = [
+  "article",
+  "article.title",
+  "article.title.size",
+  "very_long_variable_name_2024_11_05",
+  "very_long_variable_name_2024_11_05.size",
+]
+
+NUMBER_MARKUPS = [
+  "0",
+  "35",
+  "1241891024912849",
+  "3.5",
+  "3.51214128409128",
+  "12381902839.123819283910283",
+  "123.456.789",
+  "-123",
+  "-12.33",
+  "-405.231",
+  "-0",
+  "0",
+  "0.0",
+  "0.0000000000000000000000",
+  "0.00000000001",
+]
+
+RANGE_MARKUPS = [
+  "(1..30)",
+  "(1...30)",
+  "(1..30..5)",
+  "(1.0...30.0)",
+  "(1.........30)",
+  "(1..foo)",
+  "(foo..30)",
+  "(foo..bar)",
+  "(foo...bar...100)",
+  "(foo...bar...100.0)",
+]
+
+LITERAL_MARKUPS = [
+  nil,
+  'nil',
+  'null',
+  '',
+  'true',
+  'false',
+  'blank',
+  'empty',
+]
+
+MARKUPS = {
+  "string" => STRING_MARKUPS,
+  "literal" => LITERAL_MARKUPS,
+  "variable" => VARIABLE_MARKUPS,
+  "number" => NUMBER_MARKUPS,
+  "range" => RANGE_MARKUPS,
+}
+
+def compare_objects(object_1, object_2)
+  if object_1.is_a?(Liquid::VariableLookup) && object_2.is_a?(Liquid::VariableLookup)
+    return false if object_1.name != object_2.name
+  elsif object_1 != object_2
+    return false
+  end
+
+  true
+end
+
+def compare_range_lookup(expression_1_result, expression_2_result)
+  return false unless expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup)
+
+  start_obj_1 = expression_1_result.start_obj
+  start_obj_2 = expression_2_result.start_obj
+
+  return false unless compare_objects(start_obj_1, start_obj_2)
+
+  end_obj_1 = expression_1_result.end_obj
+  end_obj_2 = expression_2_result.end_obj
+
+  return false unless compare_objects(end_obj_1, end_obj_2)
+
+  true
+end
+
+MARKUPS.values.flatten.each do |markup|
+  expression_1_result = Liquid::Expression1.parse(markup)
+  expression_2_result = Liquid::Expression2.parse(markup)
+
+  next if expression_1_result == expression_2_result
+
+  if expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup)
+    next if compare_range_lookup(expression_1_result, expression_2_result)
+  end
+
+  warn "Expression1 and Expression2 results are different for markup: #{markup}"
+  warn "expected: #{expression_1_result}"
+  warn "got: #{expression_2_result}"
+  abort
+end
+
+warmed_up = false
+
+MARKUPS.each do |type, markups|
+  Benchmark.ips do |x|
+    if warmed_up
+      x.config(time: 10, warmup: 5)
+      warmed_up = true
+    else
+      x.config(time: 10)
+    end
+
+    x.report("Liquid::Expression1#parse: #{type}") do
+      if Liquid::Expression != Liquid::Expression1
+        Liquid.send(:remove_const, :Expression)
+        Liquid.const_set(:Expression, Liquid::Expression1)
+      end
+
+      markups.each do |markup|
+        Liquid::Expression1.parse(markup)
+      end
+    end
+
+    x.report("Liquid::Expression2#parse: #{type}") do
+      if Liquid::Expression != Liquid::Expression2
+        Liquid.send(:remove_const, :Expression)
+        Liquid.const_set(:Expression, Liquid::Expression2)
+      end
+
+      markups.each do |markup|
+        Liquid::Expression2.parse(markup)
+      end
+    end
+
+    x.compare!
+  end
+end
+
+Benchmark.ips do |x|
+  x.config(time: 10)
+
+  x.report("Liquid::Expression1#parse: all") do
+    if Liquid::Expression != Liquid::Expression1
+      Liquid.send(:remove_const, :Expression)
+      Liquid.const_set(:Expression, Liquid::Expression1)
+    end
+
+    MARKUPS.values.flatten.each do |markup|
+      Liquid::Expression1.parse(markup)
+    end
+  end
+
+  x.report("Liquid::Expression2#parse: all") do
+    if Liquid::Expression != Liquid::Expression2
+      Liquid.send(:remove_const, :Expression)
+      Liquid.const_set(:Expression, Liquid::Expression2)
+    end
+
+    MARKUPS.values.flatten.each do |markup|
+      Liquid::Expression2.parse(markup)
+    end
+  end
+
+  x.compare!
+end

test/integration/expression_test.rb

@@ -13,6 +13,7 @@ class ExpressionTest < Minitest::Test
     assert_template_result("double quoted", '{{"double quoted"}}')
     assert_template_result("spaced", "{{ 'spaced' }}")
     assert_template_result("spaced2", "{{ 'spaced2' }}")
+    assert_template_result("emoji🔥", "{{ 'emoji🔥' }}")
   end
 
   def test_int
@@ -22,6 +23,7 @@ class ExpressionTest < Minitest::Test
   end
 
   def test_float
+    assert_template_result("-17.42", "{{ -17.42  }}")
     assert_template_result("2.5", "{{ 2.5 }}")
     assert_expression_result(1.5, "1.5")
   end

test/integration/parsing_quirks_test.rb

@@ -134,6 +134,14 @@ class ParsingQuirksTest < Minitest::Test
 
   def test_incomplete_expression
     with_error_mode(:lax) do
+      assert_template_result("false", "{{ false - }}")
+      assert_template_result("false", "{{ false > }}")
+      assert_template_result("false", "{{ false < }}")
+      assert_template_result("false", "{{ false = }}")
+      assert_template_result("false", "{{ false ! }}")
+      assert_template_result("false", "{{ false 1 }}")
+      assert_template_result("false", "{{ false a }}")
+
       assert_template_result("false", "{% liquid assign foo = false -\n%}{{ foo }}")
       assert_template_result("false", "{% liquid assign foo = false >\n%}{{ foo }}")
       assert_template_result("false", "{% liquid assign foo = false <\n%}{{ foo }}")

test/integration/tags/raw_tag_test.rb

@@ -16,6 +16,7 @@ class RawTagTest < Minitest::Test
     assert_template_result('>{{ test }}<', '> {%- raw -%}{{ test }}{%- endraw -%} <')
     assert_template_result("> inner  <", "> {%- raw -%} inner {%- endraw %} <")
     assert_template_result("> inner <", "> {%- raw -%} inner {%- endraw -%} <")
+    assert_template_result("{Hello}", "{% raw %}{{% endraw %}Hello{% raw %}}{% endraw %}")
   end
 
   def test_open_tag_in_raw

test/unit/tokenizer_unit_test.rb

@@ -6,6 +6,7 @@ class TokenizerTest < Minitest::Test
   def test_tokenize_strings
     assert_equal([' '], tokenize(' '))
     assert_equal(['hello world'], tokenize('hello world'))
+    assert_equal(['{}'], tokenize('{}'))
   end
 
   def test_tokenize_variables
@@ -34,6 +35,19 @@ class TokenizerTest < Minitest::Test
     assert_equal([], tokenize(nil))
   end
 
+  def test_incomplete_curly_braces
+    assert_equal(["{{.}", " "], tokenize('{{.} '))
+    assert_equal(["{{}", "%}"], tokenize('{{}%}'))
+    assert_equal(["{{}}", "}"], tokenize('{{}}}'))
+  end
+
+  def test_unmatching_start_and_end
+    assert_equal(["{{%}"], tokenize('{{%}'))
+    assert_equal(["{{%%%}}"], tokenize('{{%%%}}'))
+    assert_equal(["{%", "}}"], tokenize('{%}}'))
+    assert_equal(["{%%}", "}"], tokenize('{%%}}'))
+  end
+
   private
 
   def new_tokenizer(source, parse_context: Liquid::ParseContext.new, start_line_number: nil)