From 0e4da50985db87c27c23e738501a926de495b2b8 Mon Sep 17 00:00:00 2001
From: Michael Go <michael.go@shopify.com>
Date: Thu, 21 Nov 2024 22:01:07 -0400
Subject: [PATCH] keep the expression parse caching per document parsing

---
 lib/liquid/expression.rb      | 20 ++++++++++++--------
 lib/liquid/parse_context.rb   |  5 +++--
 lib/liquid/range_lookup.rb    |  6 +++---
 lib/liquid/variable_lookup.rb |  8 +++++---
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb
index 0d8a44711..1b7d0ca23 100644
--- a/lib/liquid/expression.rb
+++ b/lib/liquid/expression.rb
@@ -22,7 +22,7 @@ class Expression1
     # malicious input as described in https://github.com/Shopify/liquid/issues/1357
     RANGES_REGEX         = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/
 
-    def self.parse(markup, _ss = nil)
+    def self.parse(markup, _ss = nil, _cache = nil)
       return nil unless markup
 
       markup = markup.strip
@@ -72,10 +72,8 @@ class Expression2
     INTEGER_REGEX = /\A(-?\d+)\z/
     FLOAT_REGEX = /\A(-?\d+)\.\d+\z/
 
-    CACHE = LruRedux::Cache.new(10_000) # most themes would have less than 2,000 unique expression
-
     class << self
-      def parse(markup, ss = StringScanner.new(""))
+      def parse(markup, ss = StringScanner.new(""), cache = nil)
         return unless markup
 
         markup = markup.strip # markup can be a frozen string
@@ -87,24 +85,30 @@ def parse(markup, ss = StringScanner.new(""))
           return LITERALS[markup]
         end
 
-        return CACHE[markup] if CACHE.key?(markup)
+        # Cache only exists during parsing
+        if cache
+          return cache[markup] if cache.key?(markup)
 
-        CACHE[markup] = inner_parse(markup, ss)
+          cache[markup] = inner_parse(markup, ss, cache)
+        else
+          inner_parse(markup, ss, nil)
+        end
       end
 
-      def inner_parse(markup, ss)
+      def inner_parse(markup, ss, cache)
         if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
           return RangeLookup.parse(
             Regexp.last_match(1),
             Regexp.last_match(2),
             ss,
+            cache,
           )
         end
 
         if (num = parse_number(markup, ss))
           num
         else
-          VariableLookup.parse(markup, ss)
+          VariableLookup.parse(markup, ss, cache)
         end
       end
 
diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb
index f66e86be3..de183d526 100644
--- a/lib/liquid/parse_context.rb
+++ b/lib/liquid/parse_context.rb
@@ -3,7 +3,7 @@
 module Liquid
   class ParseContext
     attr_accessor :locale, :line_number, :trim_whitespace, :depth
-    attr_reader :partial, :warnings, :error_mode, :environment, :string_scanner
+    attr_reader :partial, :warnings, :error_mode, :environment, :string_scanner, :expression_cache
 
     def initialize(options = Const::EMPTY_HASH)
       @environment = options.fetch(:environment, Environment.default)
@@ -15,6 +15,7 @@ def initialize(options = Const::EMPTY_HASH)
       # constructing new StringScanner in Lexer, Tokenizer, etc is expensive
       # This StringScanner will be shared by all of them
       @string_scanner = StringScanner.new("")
+      @expression_cache = LruRedux::Cache.new(10_000)
 
       self.depth   = 0
       self.partial = false
@@ -42,7 +43,7 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
     end
 
     def parse_expression(markup)
-      Expression.parse(markup, string_scanner)
+      Expression.parse(markup, string_scanner, @expression_cache)
     end
 
     def partial=(value)
diff --git a/lib/liquid/range_lookup.rb b/lib/liquid/range_lookup.rb
index 763717240..bc316fe12 100644
--- a/lib/liquid/range_lookup.rb
+++ b/lib/liquid/range_lookup.rb
@@ -2,9 +2,9 @@
 
 module Liquid
   class RangeLookup
-    def self.parse(start_markup, end_markup, string_scanner)
-      start_obj = Expression.parse(start_markup, string_scanner)
-      end_obj   = Expression.parse(end_markup, string_scanner)
+    def self.parse(start_markup, end_markup, string_scanner, cache = nil)
+      start_obj = Expression.parse(start_markup, string_scanner, cache)
+      end_obj   = Expression.parse(end_markup, string_scanner, cache)
       if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
         new(start_obj, end_obj)
       else
diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb
index f544b3bc4..06c24b764 100644
--- a/lib/liquid/variable_lookup.rb
+++ b/lib/liquid/variable_lookup.rb
@@ -6,11 +6,11 @@ class VariableLookup
 
     attr_reader :name, :lookups
 
-    def self.parse(markup, string_scanner)
-      new(markup, string_scanner)
+    def self.parse(markup, string_scanner, cache = nil)
+      new(markup, string_scanner, cache)
     end
 
-    def initialize(markup, string_scanner = StringScanner.new(""))
+    def initialize(markup, string_scanner = StringScanner.new(""), cache = nil)
       lookups = markup.scan(VariableParser)
 
       name = lookups.shift
@@ -18,6 +18,7 @@ def initialize(markup, string_scanner = StringScanner.new(""))
         name = Expression.parse(
           name[1..-2],
           string_scanner,
+          cache,
         )
       end
       @name = name
@@ -31,6 +32,7 @@ def initialize(markup, string_scanner = StringScanner.new(""))
           lookups[i] = Expression.parse(
             lookup[1..-2],
             string_scanner,
+            cache,
           )
         elsif COMMAND_METHODS.include?(lookup)
           @command_flags |= 1 << i