Compare commits

...

6 Commits

Author SHA1 Message Date
Dylan Thacker-Smith
f0da48495c Default Block#blank? to false regardless of whether the body is blank 2020-09-03 11:09:24 -04:00
Dylan Thacker-Smith
3b486425b0 Handle BlockBody#blank? at parse time (#1287) 2020-09-03 11:07:13 -04:00
Dylan Thacker-Smith
b08bcf00ac Push interrupts from Continue and Break tags rather than from BlockBody (#1286) 2020-09-03 06:55:24 -04:00
Dylan Thacker-Smith
0740e8b431 Remove unused quirk allowing liquid tags to close a block it is nested in (#1284) 2020-09-03 06:51:56 -04:00
Dylan Thacker-Smith
5532df880f Handle disabled tags errors like other liquid errors (#1275) 2020-08-18 11:39:54 -04:00
Dylan Thacker-Smith
2b11efc3ae Fix performance regression from introduction of Template#disable_tags (#1274) 2020-08-18 11:25:51 -04:00
26 changed files with 270 additions and 264 deletions

View File

@@ -63,6 +63,8 @@ require 'liquid/expression'
require 'liquid/context'
require 'liquid/parser_switching'
require 'liquid/tag'
require 'liquid/tag/disabler'
require 'liquid/tag/disableable'
require 'liquid/block'
require 'liquid/block_body'
require 'liquid/document'
@@ -86,4 +88,3 @@ require 'liquid/template_factory'
# 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 }

View File

@@ -4,11 +4,6 @@ module Liquid
class Block < Tag
MAX_DEPTH = 100
def initialize(tag_name, markup, options)
super
@blank = true
end
def parse(tokens)
@body = BlockBody.new
while parse_body(@body, tokens)
@@ -20,15 +15,16 @@ module Liquid
@body.render(context)
end
def blank?
@blank
end
def nodelist
@body.nodelist
end
def unknown_tag(tag, _params, _tokens)
Block.raise_unknown_tag(tag, block_name, block_delimiter, parse_context)
end
# @api private
def self.raise_unknown_tag(tag, block_name, block_delimiter, parse_context)
if tag == 'else'
raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_else",
block_name: block_name)
@@ -59,8 +55,6 @@ module Liquid
parse_context.depth += 1
begin
body.parse(tokens, parse_context) do |end_tag_name, end_tag_params|
@blank &&= body.blank?
return false if end_tag_name == block_delimiter
unless end_tag_name
raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed", block_name: block_name)

View File

@@ -54,21 +54,20 @@ module Liquid
end
# @api private
def self.unknown_tag_in_liquid_tag(end_tag_name, end_tag_markup)
yield end_tag_name, end_tag_markup
ensure
Usage.increment("liquid_tag_contains_outer_tag") unless $ERROR_INFO.is_a?(SyntaxError)
def self.unknown_tag_in_liquid_tag(tag, parse_context)
Block.raise_unknown_tag(tag, 'liquid', '%}', parse_context)
end
private def parse_liquid_tag(markup, parse_context, &block)
private def parse_liquid_tag(markup, parse_context)
liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
parse_for_liquid_tag(liquid_tag_tokenizer, parse_context) do |end_tag_name, end_tag_markup|
next unless end_tag_name
self.class.unknown_tag_in_liquid_tag(end_tag_name, end_tag_markup, &block)
parse_for_liquid_tag(liquid_tag_tokenizer, parse_context) do |end_tag_name, _end_tag_markup|
if end_tag_name
BlockBody.unknown_tag_in_liquid_tag(end_tag_name, parse_context)
end
end
end
private def parse_for_document(tokenizer, parse_context, &block)
private def parse_for_document(tokenizer, parse_context)
while (token = tokenizer.shift)
next if token.empty?
case
@@ -87,7 +86,7 @@ module Liquid
end
if tag_name == 'liquid'
parse_liquid_tag(markup, parse_context, &block)
parse_liquid_tag(markup, parse_context)
next
end
@@ -131,6 +130,26 @@ module Liquid
@blank
end
# Remove blank strings in the block body for a control flow tag (e.g. `if`, `for`, `case`, `unless`)
# with a blank body.
#
# For example, in a conditional assignment like the following
#
# ```
# {% if size > max_size %}
# {% assign size = max_size %}
# {% endif %}
# ```
#
# we assume the intention wasn't to output the blank spaces in the `if` tag's block body, so this method
# will remove them to reduce the render output size.
#
# Note that it is now preferred to use the `liquid` tag for this use case.
def remove_blank_strings
raise "remove_blank_strings only support being called on a blank block body" unless @blank
@nodelist.reject! { |node| node.instance_of?(String) }
end
def render(context)
render_to_output_buffer(context, +'')
end
@@ -142,23 +161,14 @@ module Liquid
while (node = @nodelist[idx])
previous_output_size = output.bytesize
case node
when String
if node.instance_of?(String)
output << node
when Variable
else
render_node(context, output, node)
when Block
render_node(context, node.blank? ? +'' : output, node)
break if context.interrupt? # might have happened in a for-block
when Continue, Break
# If we get an Interrupt that means the block must stop processing. An
# Interrupt is any command that stops block execution such as {% break %}
# or {% continue %}
context.push_interrupt(node.interrupt)
break
else # Other non-Block tags
render_node(context, output, node)
break if context.interrupt? # might have happened through an include
# or {% continue %}. These tags may also occur through Block or Include tags.
break if context.interrupt? # might have happened in a for-block
end
idx += 1
@@ -171,13 +181,7 @@ module Liquid
private
def render_node(context, output, node)
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
node.render_to_output_buffer(context, output)
rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e
context.handle_error(e, node.line_number)
rescue ::StandardError => e
@@ -185,11 +189,6 @@ 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?

View File

@@ -44,6 +44,7 @@ module Liquid
@interrupts = []
@filters = []
@global_filter = nil
@disabled_tags = {}
end
# rubocop:enable Metrics/ParameterLists
@@ -144,6 +145,7 @@ module Liquid
subcontext.strainer = nil
subcontext.errors = errors
subcontext.warnings = warnings
subcontext.disabled_tags = @disabled_tags
end
end
@@ -208,9 +210,24 @@ module Liquid
end
end
def with_disabled_tags(tag_names)
tag_names.each do |name|
@disabled_tags[name] = @disabled_tags.fetch(name, 0) + 1
end
yield
ensure
tag_names.each do |name|
@disabled_tags[name] -= 1
end
end
def tag_disabled?(tag_name)
@disabled_tags.fetch(tag_name, 0) > 0
end
protected
attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters
attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags
private

View File

@@ -53,5 +53,6 @@ module Liquid
UndefinedDropMethod = Class.new(Error)
UndefinedFilter = Class.new(Error)
MethodOverrideError = Class.new(Error)
DisabledError = Class.new(Error)
InternalError = Class.new(Error)
end

View File

@@ -1,25 +1,21 @@
# frozen_string_literal: true
module Liquid
class BlockBody
def render_node_with_profiling(context, output, node)
module BlockBodyProfilingHook
def render_node(context, output, node)
Profiler.profile_node_render(node) do
render_node_without_profiling(context, output, node)
super
end
end
alias_method :render_node_without_profiling, :render_node
alias_method :render_node, :render_node_with_profiling
end
BlockBody.prepend(BlockBodyProfilingHook)
class Include < Tag
def render_to_output_buffer_with_profiling(context, output)
module IncludeProfilingHook
def render_to_output_buffer(context, output)
Profiler.profile_children(context.evaluate(@template_name_expr).to_s) do
render_to_output_buffer_without_profiling(context, output)
super
end
end
alias_method :render_to_output_buffer_without_profiling, :render_to_output_buffer
alias_method :render_to_output_buffer, :render_to_output_buffer_with_profiling
end
Include.prepend(IncludeProfilingHook)
end

View File

@@ -1,32 +0,0 @@
# 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

View File

@@ -13,15 +13,13 @@ module Liquid
tag
end
def disable_tags(*tags)
disabled_tags.push(*tags)
def disable_tags(*tag_names)
@disabled_tags ||= []
@disabled_tags.concat(tag_names)
prepend(Disabler)
end
private :new
def disabled_tags
@disabled_tags ||= []
end
end
def initialize(tag_name, markup, parse_context)
@@ -46,14 +44,6 @@ 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.
@@ -65,9 +55,5 @@ module Liquid
def blank?
false
end
def disabled_tags
self.class.disabled_tags
end
end
end

View File

@@ -0,0 +1,22 @@
# frozen_string_literal: true
module Liquid
class Tag
module Disableable
def render_to_output_buffer(context, output)
if context.tag_disabled?(tag_name)
output << disabled_error(context)
return
end
super
end
def disabled_error(context)
# raise then rescue the exception so that the Context#exception_renderer can re-raise it
raise DisabledError, "#{tag_name} #{parse_context[:locale].t('errors.disabled.tag')}"
rescue DisabledError => exc
context.handle_error(exc, line_number)
end
end
end
end

View File

@@ -0,0 +1,21 @@
# frozen_string_literal: true
module Liquid
class Tag
module Disabler
module ClassMethods
attr_reader :disabled_tags
end
def self.prepended(base)
base.extend(ClassMethods)
end
def render_to_output_buffer(context, output)
context.with_disabled_tags(self.class.disabled_tags) do
super
end
end
end
end
end

View File

@@ -11,8 +11,11 @@ module Liquid
# {% endfor %}
#
class Break < Tag
def interrupt
BreakInterrupt.new
INTERRUPT = BreakInterrupt.new.freeze
def render_to_output_buffer(context, output)
context.push_interrupt(INTERRUPT)
output
end
end

View File

@@ -25,10 +25,9 @@ module Liquid
end
def render_to_output_buffer(context, output)
previous_output_size = output.bytesize
super
context.scopes.last[@to] = output
context.resource_limits.assign_score += (output.bytesize - previous_output_size)
capture_output = render(context)
context.scopes.last[@to] = capture_output
context.resource_limits.assign_score += capture_output.bytesize
output
end

View File

@@ -21,6 +21,14 @@ module Liquid
def parse(tokens)
body = BlockBody.new
body = @blocks.last.attachment while parse_body(body, tokens)
@blank = @blocks.all? { |condition| condition.attachment.blank? }
if @blank
@blocks.each { |condition| condition.attachment.remove_blank_strings }
end
end
def blank?
@blank
end
def nodelist

View File

@@ -11,8 +11,11 @@ module Liquid
# {% endfor %}
#
class Continue < Tag
def interrupt
ContinueInterrupt.new
INTERRUPT = ContinueInterrupt.new.freeze
def render_to_output_buffer(context, output)
context.push_interrupt(INTERRUPT)
output
end
end

View File

@@ -59,8 +59,18 @@ module Liquid
end
def parse(tokens)
return unless parse_body(@for_block, tokens)
parse_body(@else_block, tokens)
if parse_body(@for_block, tokens)
parse_body(@else_block, tokens)
end
@blank = @for_block.blank? && (@else_block.nil? || @else_block.blank?)
if @blank
@for_block.remove_blank_strings
@else_block&.remove_blank_strings
end
end
def blank?
@blank
end
def nodelist

View File

@@ -31,6 +31,14 @@ module Liquid
def parse(tokens)
while parse_body(@blocks.last.attachment, tokens)
end
@blank = @blocks.all? { |condition| condition.attachment.blank? }
if @blank
@blocks.each { |condition| condition.attachment.remove_blank_strings }
end
end
def blank?
@blank
end
def unknown_tag(tag, markup, tokens)

View File

@@ -16,6 +16,8 @@ module Liquid
# {% include 'product' for products %}
#
class Include < Tag
prepend Tag::Disableable
SYNTAX = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o
Syntax = SYNTAX

View File

@@ -80,14 +80,6 @@ module Liquid
tags[name.to_s] = klass
end
attr_accessor :registers
Template.registers = {}
private :registers=
def add_register(name, klass)
registers[name.to_sym] = klass
end
# Pass a module with filter methods which should be available
# to all liquid views. Good for registering the standard library
def register_filter(mod)
@@ -194,10 +186,6 @@ module Liquid
context.add_filters(args.pop)
end
Template.registers.each do |key, register|
context_register[key] = register unless context_register.key?(key)
end
# Retrying a render resets resource usage
context.resource_limits.reset

View File

@@ -1,27 +0,0 @@
# 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

View File

@@ -0,0 +1,51 @@
# frozen_string_literal: true
require 'test_helper'
class TagDisableableTest < Minitest::Test
include Liquid
class DisableRaw < Block
disable_tags "raw"
end
class DisableRawEcho < Block
disable_tags "raw", "echo"
end
class DisableableRaw < Liquid::Raw
prepend Liquid::Tag::Disableable
end
class DisableableEcho < Liquid::Echo
prepend Liquid::Tag::Disableable
end
def test_disables_raw
with_disableable_tags do
with_custom_tag('disable', DisableRaw) do
output = Template.parse('{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}').render
assert_equal('Liquid error: raw usage is not allowed in this contextfoo', output)
end
end
end
def test_disables_echo_and_raw
with_disableable_tags do
with_custom_tag('disable', DisableRawEcho) do
output = Template.parse('{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}').render
assert_equal('Liquid error: raw usage is not allowed in this contextLiquid error: echo usage is not allowed in this context', output)
end
end
end
private
def with_disableable_tags
with_custom_tag('raw', DisableableRaw) do
with_custom_tag('echo', DisableableEcho) do
yield
end
end
end
end

View File

@@ -82,15 +82,13 @@ class LiquidTagTest < Minitest::Test
end
def test_nested_liquid_tag
assert_usage_increment("liquid_tag_contains_outer_tag", times: 0) do
assert_template_result('good', <<~LIQUID)
{%- if true %}
{%- liquid
echo "good"
%}
{%- endif -%}
LIQUID
end
assert_template_result('good', <<~LIQUID)
{%- if true %}
{%- liquid
echo "good"
%}
{%- endif -%}
LIQUID
end
def test_cannot_open_blocks_living_past_a_liquid_tag
@@ -102,14 +100,12 @@ class LiquidTagTest < Minitest::Test
LIQUID
end
def test_quirk_can_close_blocks_created_before_a_liquid_tag
assert_usage_increment("liquid_tag_contains_outer_tag") do
assert_template_result("42", <<~LIQUID)
{%- if true -%}
42
{%- liquid endif -%}
LIQUID
end
def test_cannot_close_blocks_created_before_a_liquid_tag
assert_match_syntax_error("syntax error (line 3): 'endif' is not a valid delimiter for liquid tags. use %}", <<~LIQUID)
{%- if true -%}
42
{%- liquid endif -%}
LIQUID
end
def test_liquid_tag_in_raw

View File

@@ -127,7 +127,10 @@ class RenderTagTest < Minitest::Test
'test_include' => '{% include "foo" %}'
)
assert_template_result('include usage is not allowed in this context', '{% render "test_include" %}')
exc = assert_raises(Liquid::DisabledError) do
Liquid::Template.parse('{% render "test_include" %}').render!
end
assert_equal('Liquid error: include usage is not allowed in this context', exc.message)
end
def test_includes_will_not_render_inside_nested_sibling_tags
@@ -137,7 +140,8 @@ class RenderTagTest < Minitest::Test
'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" %}')
output = Liquid::Template.parse('{% render "nested_render_with_sibling_include" %}').render
assert_equal('Liquid error: include usage is not allowed in this contextLiquid error: include usage is not allowed in this context', output)
end
def test_render_tag_with

View File

@@ -176,7 +176,7 @@ class TemplateTest < Minitest::Test
end
def test_resource_limits_hash_in_template_gets_updated_even_if_no_limits_are_set
t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
t = Template.parse("{% for a in (1..100) %}x{% assign foo = 1 %} {% endfor %}")
t.render!
assert(t.resource_limits.assign_score > 0)
assert(t.resource_limits.render_score > 0)
@@ -215,7 +215,7 @@ class TemplateTest < Minitest::Test
def test_default_resource_limits_unaffected_by_render_with_context
context = Context.new
t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
t = Template.parse("{% for a in (1..100) %}x{% assign foo = 1 %} {% endfor %}")
t.render!(context)
assert(context.resource_limits.assign_score > 0)
assert(context.resource_limits.render_score > 0)
@@ -361,48 +361,4 @@ class TemplateTest < Minitest::Test
result = t.render('x' => 1, 'y' => 5)
assert_equal('12345', result)
end
def test_render_uses_correct_disabled_tags_instance
Liquid::Template.file_system = StubFileSystem.new(
'foo' => 'bar',
'test_include' => '{% include "foo" %}'
)
disabled_tags = DisabledTags.new
context = Context.build(registers: { disabled_tags: disabled_tags })
source = "{% render 'test_include' %}"
parse_context = Liquid::ParseContext.new(line_numbers: true, error_mode: :strict)
document = Document.parse(Liquid::Tokenizer.new(source, true), parse_context)
assert_equal("include usage is not allowed in this context", document.render(context))
end
def test_render_sets_context_static_register_when_register_key_does_exist
disabled_tags_for_test = DisabledTags.new
Template.add_register(:disabled_tags, disabled_tags_for_test)
t = Template.parse("{% if true %} Test Template {% endif %}")
context = Context.new
refute(context.registers.key?(:disabled_tags))
t.render(context)
assert(context.registers.key?(:disabled_tags))
assert_equal(disabled_tags_for_test, context.registers[:disabled_tags])
end
def test_render_does_not_override_context_static_register_when_register_key_exists
context = Context.new
context.registers[:random_register] = nil
Template.add_register(:random_register, {})
t = Template.parse("{% if true %} Test Template {% endif %}")
t.render(context)
assert_nil(context.registers[:random_register])
assert(context.registers.key?(:random_register))
end
end

View File

@@ -98,10 +98,17 @@ module Minitest
end
def with_custom_tag(tag_name, tag_class)
Liquid::Template.register_tag(tag_name, tag_class)
yield
ensure
Liquid::Template.tags.delete(tag_name)
old_tag = Liquid::Template.tags[tag_name]
begin
Liquid::Template.register_tag(tag_name, tag_class)
yield
ensure
if old_tag
Liquid::Template.tags[tag_name] = old_tag
else
Liquid::Template.tags.delete(tag_name)
end
end
end
end
end

View File

@@ -563,6 +563,35 @@ class ContextUnitTest < Minitest::Test
assert_equal('my filter result', template.render(subcontext))
end
def test_disables_tag_specified
context = Context.new
context.with_disabled_tags(%w(foo bar)) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
assert_equal false, context.tag_disabled?("unknown")
end
end
def test_disables_nested_tags
context = Context.new
context.with_disabled_tags(["foo"]) do
context.with_disabled_tags(["foo"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal false, context.tag_disabled?("bar")
end
context.with_disabled_tags(["bar"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
context.with_disabled_tags(["foo"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
end
end
assert_equal true, context.tag_disabled?("foo")
assert_equal false, context.tag_disabled?("bar")
end
end
private
def assert_no_object_allocations

View File

@@ -1,36 +0,0 @@
# 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