[StrainerTemplate] Isolate filter mods

lib/liquid.rb

@@ -57,6 +57,8 @@ require 'liquid/forloop_drop'
 require 'liquid/extensions'
 require 'liquid/errors'
 require 'liquid/interrupts'
+require 'liquid/filter'
+require 'liquid/filter_template'
 require 'liquid/strainer_factory'
 require 'liquid/strainer_template'
 require 'liquid/expression'

lib/liquid/filter.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module Liquid
+  # A filter in liquid is a class which contain invokable logic from liquid templates.
+  #
+  # Public methods in filter classes are callable.
+  #
+  # The use for liquid filters is to make logic functions available to the web designers.
+  #
+  # Example:
+  #
+  #   class StringFilter < Liquid::Filter
+  #     def upcase(input)
+  #       input.upcase
+  #     end
+  #   end
+  #
+  #   tmpl = Liquid::Template.parse('Result: {{ "test" | upcase }}')
+  #   tmpl.render({}, filters: [StringFilter])
+  #   => "Result: TEST"
+  class Filter
+    class << self
+      def invokable_methods
+        @invokable_methods ||= begin
+          blacklist = Liquid::Filter.public_instance_methods
+          whitelist = public_instance_methods - blacklist
+
+          Set.new(whitelist.map(&:to_s))
+        end
+      end
+    end
+
+    def initialize(context)
+      @context = context
+    end
+
+    private
+
+    attr_reader :context
+  end
+end

lib/liquid/filter_template.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module Liquid
+  # FilterTemplate is the computed class for the filters system.
+  #
+  # Historically Liquid used to include filters as Module to the context strainer.
+  # This lead to the absence of sandbox between filters (one filter could override private methods of another filter).
+  #
+  # With the implementation of Liquid::Filter, it is now possible for the modules from legacy code to be automatically
+  #   wrapped into a Liquid::Filter generated class.
+  #
+  # This should not be considered as the base behaviour, it is preferred to create filters going forward directly as
+  #   classes that are child of Liquid::Filter.
+  class FilterTemplate < Filter
+    class << self
+      def include(mod)
+        super
+
+        @init_module = mod
+      end
+
+      # Override of the `invokable_methods`.
+      # We can't rely on the parent logic as some modules might have been defining methods that shadow Class methods.
+      #
+      # Eg.:
+      # mod = Liquid::StandardFilters
+      # filter = Class.new(FilterTemplate)
+      # filter.include(mod)
+      # mod.public_instance_methods - (filter.public_instance_methods - Class.public_instance_methods)
+      # => [:prepend]
+      def invokable_methods
+        whitelist = @init_module.public_instance_methods
+
+        @invokable_methods ||= Set.new(whitelist.map(&:to_s))
+      end
+    end
+  end
+end

lib/liquid/strainer_template.rb

@@ -14,40 +14,65 @@ module Liquid
     end
 
     class << self
-      def add_filter(filter)
-        return if include?(filter)
-
-        invokable_non_public_methods = (filter.private_instance_methods + filter.protected_instance_methods).select { |m| invokable?(m) }
-        if invokable_non_public_methods.any?
-          raise MethodOverrideError, "Filter overrides registered public methods as non public: #{invokable_non_public_methods.join(', ')}"
+      def add_filter(mod)
+        filter = if mod.is_a?(Class) && mod.ancestors.include?(Liquid::Filter)
+          mod
+        elsif mod.instance_of?(Module)
+          convert_mod_to_filter(mod)
+        else
+          raise(ArgumentError, "wrong argument type Proc (expected Liquid::Filter)")
         end
 
-        include(filter)
+        filter.invokable_methods.each do |method|
+          filter_map[method] = filter
+        end
+      end
 
-        filter_methods.merge(filter.public_instance_methods.map(&:to_s))
+      def fetch_filter(method)
+        filter_map.fetch(method)
       end
 
       def invokable?(method)
-        filter_methods.include?(method.to_s)
+        filter_map.key?(method)
       end
 
       private
 
-      def filter_methods
-        @filter_methods ||= Set.new
+      def filter_map
+        @filter_map ||= {}
+      end
+
+      def convert_mod_to_filter(mod)
+        @filter_classes ||= {}
+        @filter_classes[mod] ||= begin
+          klass = Class.new(FilterTemplate)
+          klass.include(mod)
+          klass
+        end
+      end
+
+      def filter_class_by_methods
+        @filter_class_by_methods ||= {}
       end
     end
 
     def invoke(method, *args)
       if self.class.invokable?(method)
-        send(method, *args)
+        klass = self.class.fetch_filter(method)
+
+        instance = klass.new(@context)
+        instance.public_send(method, *args)
       elsif @context.strict_filters
-        raise Liquid::UndefinedFilter, "undefined filter #{method}"
+        raise(Liquid::UndefinedFilter, "undefined filter #{method}")
       else
         args.first
       end
     rescue ::ArgumentError => e
       raise Liquid::ArgumentError, e.message, e.backtrace
     end
+
+    private
+
+    attr_reader :context
   end
 end