From 0db9c56f348f5325c52b1a86f71d7b9b92092569 Mon Sep 17 00:00:00 2001 From: Mike Angell <53470248+shopmike@users.noreply.github.com> Date: Thu, 26 Sep 2019 00:18:30 +1000 Subject: [PATCH] Disable rendering of tag based on register (#1162) * Disable rendering of tag based on register * Improvements to disable tag * Resolve disbale tag tests * Test disable_tags register * disabled_tags is now always avaiable * Allow multiple tags to be disabled at once * Move disabled check to block_body * Code improvements * Remove redundant nil check * Improve disabled tag error output * Improve disable tag API * Code improvements * Switch disabled? to not mutate output * Fix array handling shortcut in disable_tags --- lib/liquid.rb | 2 ++ lib/liquid/block_body.rb | 13 ++++++- lib/liquid/locales/en.yml | 2 ++ lib/liquid/register.rb | 6 ++++ lib/liquid/registers/disabled_tags.rb | 32 +++++++++++++++++ lib/liquid/tag.rb | 20 +++++++++++ lib/liquid/tags/render.rb | 6 ++++ lib/liquid/template.rb | 18 +++++++++- lib/liquid/variable.rb | 8 +++++ .../registers/disabled_tags_test.rb | 27 ++++++++++++++ test/integration/tags/render_tag_test.rb | 29 +++++++++++---- .../unit/registers/disabled_tags_unit_test.rb | 36 +++++++++++++++++++ 12 files changed, 191 insertions(+), 8 deletions(-) create mode 100644 lib/liquid/register.rb create mode 100644 lib/liquid/registers/disabled_tags.rb create mode 100644 test/integration/registers/disabled_tags_test.rb create mode 100644 test/unit/registers/disabled_tags_unit_test.rb diff --git a/lib/liquid.rb b/lib/liquid.rb index d102530..cfaccee 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -78,8 +78,10 @@ require 'liquid/tokenizer' require 'liquid/parse_context' require 'liquid/partial_cache' require 'liquid/usage' +require 'liquid/register' require 'liquid/static_registers' # Load all the tags of the standard library # Dir["#{__dir__}/liquid/tags/*.rb"].each { |f| require f } +Dir["#{__dir__}/liquid/registers/*.rb"].each { |f| require f } diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index 9400e38..c543d82 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -154,7 +154,13 @@ module Liquid private def render_node(context, output, node) - node.render_to_output_buffer(context, output) + if node.disabled?(context) + output << node.disabled_error_message + return + end + disable_tags(context, node.disabled_tags) do + node.render_to_output_buffer(context, output) + end rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e context.handle_error(e, node.line_number) rescue ::StandardError => e @@ -162,6 +168,11 @@ module Liquid output << context.handle_error(e, line_number) end + def disable_tags(context, tags, &block) + return yield if tags.empty? + context.registers['disabled_tags'].disable(tags, &block) + end + def raise_if_resource_limits_reached(context, length) context.resource_limits.render_length += length return unless context.resource_limits.reached? diff --git a/lib/liquid/locales/en.yml b/lib/liquid/locales/en.yml index c0a9aff..a26320b 100644 --- a/lib/liquid/locales/en.yml +++ b/lib/liquid/locales/en.yml @@ -25,3 +25,5 @@ render: "Syntax error in tag 'render' - Template name must be a quoted string" argument: include: "Argument error in tag 'include' - Illegal template name" + disabled: + tag: "usage is not allowed in this context" diff --git a/lib/liquid/register.rb b/lib/liquid/register.rb new file mode 100644 index 0000000..92d0226 --- /dev/null +++ b/lib/liquid/register.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +module Liquid + class Register + end +end diff --git a/lib/liquid/registers/disabled_tags.rb b/lib/liquid/registers/disabled_tags.rb new file mode 100644 index 0000000..79b6472 --- /dev/null +++ b/lib/liquid/registers/disabled_tags.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +module Liquid + class DisabledTags < Register + def initialize + @disabled_tags = {} + end + + def disabled?(tag) + @disabled_tags.key?(tag) && @disabled_tags[tag] > 0 + end + + def disable(tags) + tags.each(&method(:increment)) + yield + ensure + tags.each(&method(:decrement)) + end + + private + + def increment(tag) + @disabled_tags[tag] ||= 0 + @disabled_tags[tag] += 1 + end + + def decrement(tag) + @disabled_tags[tag] -= 1 + end + end + + Template.add_register('disabled_tags', DisabledTags.new) +end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 1460639..832e32c 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -13,7 +13,15 @@ module Liquid tag end + def disable_tags(*tags) + disabled_tags.push(*tags) + end + private :new + + def disabled_tags + @disabled_tags ||= [] + end end def initialize(tag_name, markup, parse_context) @@ -38,6 +46,14 @@ module Liquid '' end + def disabled?(context) + context.registers['disabled_tags'].disabled?(tag_name) + end + + def disabled_error_message + "#{tag_name} #{options[:locale].t('errors.disabled.tag')}" + end + # For backwards compatibility with custom tags. In a future release, the semantics # of the `render_to_output_buffer` method will become the default and the `render` # method will be removed. @@ -49,5 +65,9 @@ module Liquid def blank? false end + + def disabled_tags + self.class.disabled_tags + end end end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index e6c6223..1403b58 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -4,6 +4,8 @@ module Liquid class Render < Tag SYNTAX = /(#{QuotedString})#{QuotedFragment}*/o + disable_tags "include" + attr_reader :template_name_expr, :attributes def initialize(tag_name, markup, options) @@ -22,6 +24,10 @@ module Liquid end def render_to_output_buffer(context, output) + render_tag(context, output) + end + + def render_tag(context, output) # Though we evaluate this here we will only ever parse it as a string literal. template_name = context.evaluate(@template_name_expr) raise ArgumentError, options[:locale].t("errors.argument.include") unless template_name diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index e77ba8a..2f0bed4 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -92,6 +92,14 @@ module Liquid @tags ||= TagRegistry.new end + def add_register(name, klass) + registers[name.to_s] = klass + end + + def registers + @registers ||= {} + end + def error_mode @error_mode ||= :lax end @@ -191,18 +199,26 @@ module Liquid output = nil + context_register = context.registers.is_a?(StaticRegisters) ? context.registers.static : context.registers + case args.last when Hash options = args.pop output = options[:output] if options[:output] - registers.merge!(options[:registers]) if options[:registers].is_a?(Hash) + options[:registers]&.each do |key, register| + context_register[key] = register + end apply_options_to_context(context, options) when Module, Array context.add_filters(args.pop) end + Template.registers.each do |key, register| + context_register[key] = register + end + # Retrying a render resets resource usage context.resource_limits.reset diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 265748d..5b686e2 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -104,6 +104,14 @@ module Liquid output end + def disabled?(_context) + false + end + + def disabled_tags + [] + end + private def parse_filter_expressions(filter_name, unparsed_args) diff --git a/test/integration/registers/disabled_tags_test.rb b/test/integration/registers/disabled_tags_test.rb new file mode 100644 index 0000000..1fb2458 --- /dev/null +++ b/test/integration/registers/disabled_tags_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'test_helper' + +class DisabledTagsTest < Minitest::Test + include Liquid + + class DisableRaw < Block + disable_tags "raw" + end + + class DisableRawEcho < Block + disable_tags "raw", "echo" + end + + def test_disables_raw + with_custom_tag('disable', DisableRaw) do + assert_template_result 'raw usage is not allowed in this contextfoo', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}' + end + end + + def test_disables_echo_and_raw + with_custom_tag('disable', DisableRawEcho) do + assert_template_result 'raw usage is not allowed in this contextecho usage is not allowed in this context', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}' + end + end +end diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index 154783a..87373a2 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -89,14 +89,12 @@ class RenderTagTest < Minitest::Test end end - def test_includes_and_renders_count_towards_the_same_recursion_limit + def test_sub_contexts_count_towards_the_same_recursion_limit Liquid::Template.file_system = StubFileSystem.new( - 'loop_render' => '{% render "loop_include" %}', - 'loop_include' => '{% include "loop_render" %}' + 'loop_render' => '{% render "loop_render" %}', ) - - assert_raises Liquid::StackLevelError do - Template.parse('{% render "loop_include" %}').render! + assert_raises Liquid::StackLevelError do + Template.parse('{% render "loop_render" %}').render! end end @@ -148,4 +146,23 @@ class RenderTagTest < Minitest::Test Liquid::Template.file_system = StubFileSystem.new('decr' => '{% decrement %}') assert_template_result '-1-2-1', '{% decrement %}{% decrement %}{% render "decr" %}' end + + def test_includes_will_not_render_inside_render_tag + Liquid::Template.file_system = StubFileSystem.new( + 'foo' => 'bar', + 'test_include' => '{% include "foo" %}' + ) + + assert_template_result 'include usage is not allowed in this context', '{% render "test_include" %}' + end + + def test_includes_will_not_render_inside_nested_sibling_tags + Liquid::Template.file_system = StubFileSystem.new( + 'foo' => 'bar', + 'nested_render_with_sibling_include' => '{% render "test_include" %}{% include "foo" %}', + 'test_include' => '{% include "foo" %}' + ) + + assert_template_result 'include usage is not allowed in this contextinclude usage is not allowed in this context', '{% render "nested_render_with_sibling_include" %}' + end end diff --git a/test/unit/registers/disabled_tags_unit_test.rb b/test/unit/registers/disabled_tags_unit_test.rb new file mode 100644 index 0000000..90ac016 --- /dev/null +++ b/test/unit/registers/disabled_tags_unit_test.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'test_helper' + +class DisabledTagsUnitTest < Minitest::Test + include Liquid + + def test_disables_tag_specified + register = DisabledTags.new + register.disable(%w(foo bar)) do + assert_equal true, register.disabled?("foo") + assert_equal true, register.disabled?("bar") + assert_equal false, register.disabled?("unknown") + end + end + + def test_disables_nested_tags + register = DisabledTags.new + register.disable(["foo"]) do + register.disable(["foo"]) do + assert_equal true, register.disabled?("foo") + assert_equal false, register.disabled?("bar") + end + register.disable(["bar"]) do + assert_equal true, register.disabled?("foo") + assert_equal true, register.disabled?("bar") + register.disable(["foo"]) do + assert_equal true, register.disabled?("foo") + assert_equal true, register.disabled?("bar") + end + end + assert_equal true, register.disabled?("foo") + assert_equal false, register.disabled?("bar") + end + end +end