diff --git a/README.md b/README.md index cf8fcd34..877fdf69 100644 --- a/README.md +++ b/README.md @@ -73,3 +73,34 @@ This is useful for doing things like enabling strict mode only in the theme edit It is recommended that you enable `:strict` or `:warn` mode on new apps to stop invalid templates from being created. It is also recommended that you use it in the template editors of existing apps to give editors better error messages. + +### Undefined variables and filters + +By default, the renderer doesn't raise or in any other way notify you if some variables or filters are missing, i.e. not passed to the `render` method. +You can improve this situation by passing `strict_variables: true` and/or `strict_filters: true` options to the `render` method. +When one of these options is set to true, all errors about undefined variables and undefined filters will be stored in `errors` array of a `Liquid::Template` instance. +Here are some examples: + +```ruby +template = Liquid::Template.parse("{{x}} {{y}} {{z.a}} {{z.b}}") +template.render({ 'x' => 1, 'z' => { 'a' => 2 } }, { strict_variables: true }) +#=> '1 2 ' # when a variable is undefined, it's rendered as nil +template.errors +#=> [#, #] +``` + +```ruby +template = Liquid::Template.parse("{{x | filter1 | upcase}}") +template.render({ 'x' => 'foo' }, { strict_filters: true }) +#=> '' # when at least one filter in the filter chain is undefined, a whole expression is rendered as nil +template.errors +#=> [#] +``` + +If you want to raise on a first exception instead of pushing all of them in `errors`, you can use `render!` method: + +```ruby +template = Liquid::Template.parse("{{x}} {{y}}") +template.render!({ 'x' => 1}, { strict_variables: true }) +#=> Liquid::UndefinedVariable: Liquid error: undefined variable y +``` diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index 60b28b53..c3753478 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -76,6 +76,9 @@ module Liquid end rescue MemoryError => e raise e + rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e + context.handle_error(e, token.line_number) + output << nil rescue ::StandardError => e output << context.handle_error(e, token.line_number) end diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 225eec54..bab5008d 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -13,7 +13,7 @@ module Liquid # context['bob'] #=> nil class Context class Context attr_reader :scopes, :errors, :registers, :environments, :resource_limits - attr_accessor :exception_handler, :template_name, :partial, :global_filter + attr_accessor :exception_handler, :template_name, :partial, :global_filter, :strict_variables, :strict_filters def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil) @environments = [environments].flatten @@ -207,6 +207,8 @@ module Liquid def lookup_and_evaluate(obj, key) if (value = obj[key]).is_a?(Proc) && obj.respond_to?(:[]=) obj[key] = (value.arity == 0) ? value.call : value.call(self) + elsif strict_variables && obj.respond_to?(:key?) && !obj.key?(key) + raise Liquid::UndefinedVariable, "undefined variable #{key}" else value end diff --git a/lib/liquid/drop.rb b/lib/liquid/drop.rb index 08145327..57a17e0b 100644 --- a/lib/liquid/drop.rb +++ b/lib/liquid/drop.rb @@ -24,8 +24,9 @@ module Liquid attr_writer :context # Catch all for the method - def liquid_method_missing(_method) - nil + def liquid_method_missing(method) + return nil unless @context.strict_variables + raise Liquid::UndefinedDropMethod, "undefined method #{method}" end # called by liquid to invoke a drop diff --git a/lib/liquid/errors.rb b/lib/liquid/errors.rb index 9ee3a662..762828cc 100644 --- a/lib/liquid/errors.rb +++ b/lib/liquid/errors.rb @@ -56,4 +56,7 @@ module Liquid MemoryError = Class.new(Error) ZeroDivisionError = Class.new(Error) FloatDomainError = Class.new(Error) + UndefinedVariable = Class.new(Error) + UndefinedDropMethod = Class.new(Error) + UndefinedFilter = Class.new(Error) end diff --git a/lib/liquid/strainer.rb b/lib/liquid/strainer.rb index a902825d..c79a5863 100644 --- a/lib/liquid/strainer.rb +++ b/lib/liquid/strainer.rb @@ -26,7 +26,7 @@ module Liquid end def self.add_filter(filter) - raise ArgumentError, "Expected module but got: #{f.class}" unless filter.is_a?(Module) + raise ArgumentError, "Expected module but got: #{filter.class}" unless filter.is_a?(Module) unless self.class.include?(filter) send(:include, filter) @filter_methods.merge(filter.public_instance_methods.map(&:to_s)) @@ -48,6 +48,8 @@ module Liquid def invoke(method, *args) if self.class.invokable?(method) send(method, *args) + elsif @context && @context.strict_filters + raise Liquid::UndefinedFilter, "undefined filter #{method}" else args.first end diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index b875e0b3..130d17d5 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -181,12 +181,7 @@ module Liquid registers.merge!(options[:registers]) if options[:registers].is_a?(Hash) - context.add_filters(options[:filters]) if options[:filters] - - context.global_filter = options[:global_filter] if options[:global_filter] - - context.exception_handler = options[:exception_handler] if options[:exception_handler] - + apply_options_to_context(context, options) when Module, Array context.add_filters(args.pop) end @@ -235,5 +230,13 @@ module Liquid yield end end + + def apply_options_to_context(context, options) + context.add_filters(options[:filters]) if options[:filters] + context.global_filter = options[:global_filter] if options[:global_filter] + context.exception_handler = options[:exception_handler] if options[:exception_handler] + context.strict_variables = options[:strict_variables] if options[:strict_variables] + context.strict_filters = options[:strict_filters] if options[:strict_filters] + end end end diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 62d753a0..3ed4e4a4 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -55,9 +55,11 @@ module Liquid object = object.send(key).to_liquid # No key was present with the desired value and it wasn't one of the directly supported - # keywords either. The only thing we got left is to return nil + # keywords either. The only thing we got left is to return nil or + # raise an exception if `strict_variables` option is set to true else - return nil + return nil unless context.strict_variables + raise Liquid::UndefinedVariable, "undefined variable #{key}" end # If we are dealing with a drop here we have to diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index c1e2ef3c..aea86690 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -27,6 +27,12 @@ class ErroneousDrop < Liquid::Drop end end +class DropWithUndefinedMethod < Liquid::Drop + def foo + 'foo' + end +end + class TemplateTest < Minitest::Test include Liquid @@ -225,4 +231,68 @@ class TemplateTest < Minitest::Test assert_equal 'BOB filtered', rendered_template end + + def test_undefined_variables + t = Template.parse("{{x}} {{y}} {{z.a}} {{z.b}} {{z.c.d}}") + result = t.render({ 'x' => 33, 'z' => { 'a' => 32, 'c' => { 'e' => 31 } } }, { strict_variables: true }) + + assert_equal '33 32 ', result + assert_equal 3, t.errors.count + assert_instance_of Liquid::UndefinedVariable, t.errors[0] + assert_equal 'Liquid error: undefined variable y', t.errors[0].message + assert_instance_of Liquid::UndefinedVariable, t.errors[1] + assert_equal 'Liquid error: undefined variable b', t.errors[1].message + assert_instance_of Liquid::UndefinedVariable, t.errors[2] + assert_equal 'Liquid error: undefined variable d', t.errors[2].message + end + + def test_undefined_variables_raise + t = Template.parse("{{x}} {{y}} {{z.a}} {{z.b}} {{z.c.d}}") + + assert_raises UndefinedVariable do + t.render!({ 'x' => 33, 'z' => { 'a' => 32, 'c' => { 'e' => 31 } } }, { strict_variables: true }) + end + end + + def test_undefined_drop_methods + d = DropWithUndefinedMethod.new + t = Template.new.parse('{{ foo }} {{ woot }}') + result = t.render(d, { strict_variables: true }) + + assert_equal 'foo ', result + assert_equal 1, t.errors.count + assert_instance_of Liquid::UndefinedDropMethod, t.errors[0] + end + + def test_undefined_drop_methods_raise + d = DropWithUndefinedMethod.new + t = Template.new.parse('{{ foo }} {{ woot }}') + + assert_raises UndefinedDropMethod do + t.render!(d, { strict_variables: true }) + end + end + + def test_undefined_filters + t = Template.parse("{{a}} {{x | upcase | somefilter1 | somefilter2 | somefilter3}}") + filters = Module.new do + def somefilter3(v) + "-#{v}-" + end + end + result = t.render({ 'a' => 123, 'x' => 'foo' }, { filters: [filters], strict_filters: true }) + + assert_equal '123 ', result + assert_equal 1, t.errors.count + assert_instance_of Liquid::UndefinedFilter, t.errors[0] + assert_equal 'Liquid error: undefined filter somefilter1', t.errors[0].message + end + + def test_undefined_filters_raise + t = Template.parse("{{x | somefilter1 | upcase | somefilter2}}") + + assert_raises UndefinedFilter do + t.render!({ 'x' => 'foo' }, { strict_filters: true }) + end + end end diff --git a/test/unit/strainer_unit_test.rb b/test/unit/strainer_unit_test.rb index 2048a393..d06d30a5 100644 --- a/test/unit/strainer_unit_test.rb +++ b/test/unit/strainer_unit_test.rb @@ -77,4 +77,14 @@ class StrainerUnitTest < Minitest::Test assert_kind_of b, strainer assert_kind_of Liquid::StandardFilters, strainer end + + def test_add_filter_when_wrong_filter_class + c = Context.new + s = c.strainer + wrong_filter = ->(v) { v.reverse } + + assert_raises ArgumentError do + s.class.add_filter(wrong_filter) + end + end end # StrainerTest