diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index ab19071..e93dcba 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -44,21 +44,8 @@ module Liquid # for that def add_filters(filters) filters = [filters].flatten.compact - filters.each do |f| - raise ArgumentError, "Expected module but got: #{f.class}" unless f.is_a?(Module) - Strainer.add_known_filter(f) - end - - # If strainer is already setup then there's no choice but to use a runtime - # extend call. If strainer is not yet created, we can utilize strainers - # cached class based API, which avoids busting the method cache. - if @strainer - filters.each do |f| - strainer.extend(f) - end - else - @filters.concat filters - end + @filters += filters + @strainer = nil end # are there any not handled interrupts? diff --git a/lib/liquid/strainer.rb b/lib/liquid/strainer.rb index e11549e..31a1ec2 100644 --- a/lib/liquid/strainer.rb +++ b/lib/liquid/strainer.rb @@ -8,12 +8,13 @@ module Liquid # The Strainer only allows method calls defined in filters given to it via Strainer.global_filter, # Context#add_filters or Template.register_filter class Strainer #:nodoc: - @@filters = [] - @@known_filters = Set.new - @@known_methods = Set.new + @@global_strainer = Class.new(Strainer) do + @filter_methods = Set.new + end @@strainer_class_cache = Hash.new do |hash, filters| - hash[filters] = Class.new(Strainer) do - filters.each { |f| include f } + hash[filters] = Class.new(@@global_strainer) do + @filter_methods = @@global_strainer.filter_methods.dup + filters.each { |f| add_filter(f) } end end @@ -21,33 +22,32 @@ module Liquid @context = context end - def self.global_filter(filter) - raise ArgumentError, "Passed filter is not a module" unless filter.is_a?(Module) - add_known_filter(filter) - @@filters << filter unless @@filters.include?(filter) + def self.filter_methods + @filter_methods end - def self.add_known_filter(filter) - unless @@known_filters.include?(filter) - @@method_blacklist ||= Set.new(Strainer.instance_methods.map(&:to_s)) - new_methods = filter.instance_methods.map(&:to_s) - new_methods.reject!{ |m| @@method_blacklist.include?(m) } - @@known_methods.merge(new_methods) - @@known_filters.add(filter) + def self.add_filter(filter) + raise ArgumentError, "Expected module but got: #{f.class}" unless filter.is_a?(Module) + unless self.class.include?(filter) + self.send(:include, filter) + @filter_methods.merge(filter.public_instance_methods.map(&:to_s)) end end - def self.strainer_class_cache - @@strainer_class_cache + def self.global_filter(filter) + @@global_strainer.add_filter(filter) + end + + def self.invokable?(method) + @filter_methods.include?(method.to_s) end def self.create(context, filters = []) - filters = @@filters + filters - strainer_class_cache[filters].new(context) + @@strainer_class_cache[filters].new(context) end def invoke(method, *args) - if invokable?(method) + if self.class.invokable?(method) send(method, *args) else args.first @@ -55,9 +55,5 @@ module Liquid rescue ::ArgumentError => e raise Liquid::ArgumentError.new(e.message) end - - def invokable?(method) - @@known_methods.include?(method.to_s) && respond_to?(method) - end end end diff --git a/test/integration/filter_test.rb b/test/integration/filter_test.rb index d84458b..0a45075 100644 --- a/test/integration/filter_test.rb +++ b/test/integration/filter_test.rb @@ -25,6 +25,12 @@ end class FiltersTest < Minitest::Test include Liquid + module OverrideObjectMethodFilter + def tap(input) + "tap overridden" + end + end + def setup @context = Context.new end @@ -105,6 +111,13 @@ class FiltersTest < Minitest::Test output = Variable.new(%! 'hello %{first_name}, %{last_name}' | substitute: first_name: surname, last_name: 'doe' !).render(@context) assert_equal 'hello john, doe', output end + + def test_override_object_method_in_filter + assert_equal "tap overridden", Template.parse("{{var | tap}}").render!({ 'var' => 1000 }, :filters => [OverrideObjectMethodFilter]) + + # tap still treated as a non-existent filter + assert_equal "1000", Template.parse("{{var | tap}}").render!({ 'var' => 1000 }) + end end class FiltersInTemplate < Minitest::Test diff --git a/test/test_helper.rb b/test/test_helper.rb index 438f075..d1b4897 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -49,13 +49,19 @@ module Minitest end def with_global_filter(*globals) - original_filters = Array.new(Liquid::Strainer.class_variable_get(:@@filters)) + original_global_strainer = Liquid::Strainer.class_variable_get(:@@global_strainer) + Liquid::Strainer.class_variable_set(:@@global_strainer, Class.new(Liquid::Strainer) do + @filter_methods = Set.new + end) + Liquid::Strainer.class_variable_get(:@@strainer_class_cache).clear + globals.each do |global| Liquid::Template.register_filter(global) end yield ensure - Liquid::Strainer.class_variable_set(:@@filters, original_filters) + Liquid::Strainer.class_variable_get(:@@strainer_class_cache).clear + Liquid::Strainer.class_variable_set(:@@global_strainer, original_global_strainer) end def with_taint_mode(mode) diff --git a/test/unit/strainer_unit_test.rb b/test/unit/strainer_unit_test.rb index ff2ea35..9fcff9c 100644 --- a/test/unit/strainer_unit_test.rb +++ b/test/unit/strainer_unit_test.rb @@ -31,11 +31,11 @@ class StrainerUnitTest < Minitest::Test def test_strainer_only_invokes_public_filter_methods strainer = Strainer.create(nil) - assert_equal false, strainer.invokable?('__test__') - assert_equal false, strainer.invokable?('test') - assert_equal false, strainer.invokable?('instance_eval') - assert_equal false, strainer.invokable?('__send__') - assert_equal true, strainer.invokable?('size') # from the standard lib + assert_equal false, strainer.class.invokable?('__test__') + assert_equal false, strainer.class.invokable?('test') + assert_equal false, strainer.class.invokable?('instance_eval') + assert_equal false, strainer.class.invokable?('__send__') + assert_equal true, strainer.class.invokable?('size') # from the standard lib end def test_strainer_returns_nil_if_no_filter_method_found @@ -63,9 +63,7 @@ class StrainerUnitTest < Minitest::Test assert_kind_of Strainer, strainer assert_kind_of a, strainer assert_kind_of b, strainer - Strainer.class_variable_get(:@@filters).each do |m| - assert_kind_of m, strainer - end + assert_kind_of Liquid::StandardFilters, strainer end end # StrainerTest