diff --git a/lib/liquid.rb b/lib/liquid.rb index 770d2f9..0e198bb 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -74,6 +74,7 @@ require 'liquid/condition' require 'liquid/utils' require 'liquid/tokenizer' require 'liquid/parse_context' +require 'liquid/partial_cache' # Load all the tags of the standard library # diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 2dcc6af..1b15ca7 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -12,17 +12,25 @@ module Liquid # # context['bob'] #=> nil class Context class Context - attr_reader :scopes, :errors, :registers, :environments, :resource_limits + attr_reader :scopes, :errors, :registers, :environments, :resource_limits, :static_registers, :static_environments attr_accessor :exception_renderer, :template_name, :partial, :global_filter, :strict_variables, :strict_filters - def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil) - @environments = [environments].flatten - @scopes = [(outer_scope || {})] - @registers = registers - @errors = [] - @partial = false - @strict_variables = false - @resource_limits = resource_limits || ResourceLimits.new(Template.default_resource_limits) + # rubocop:disable Metrics/ParameterLists + def self.build(environments: {}, outer_scope: {}, registers: {}, rethrow_errors: false, resource_limits: nil, static_registers: {}, static_environments: {}) + new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_registers, static_environments) + end + + def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, static_registers = {}, static_environments = {}) + @environments = [environments].flatten + @static_environments = [static_environments].flatten.map(&:freeze).freeze + @scopes = [(outer_scope || {})] + @registers = registers + @static_registers = static_registers.freeze + @errors = [] + @partial = false + @strict_variables = false + @resource_limits = resource_limits || ResourceLimits.new(Template.default_resource_limits) + @base_scope_depth = 0 squash_instance_assigns_with_environments @this_stack_used = false @@ -36,6 +44,7 @@ module Liquid @filters = [] @global_filter = nil end + # rubocop:enable Metrics/ParameterLists def warnings @warnings ||= [] @@ -89,7 +98,7 @@ module Liquid # Push new local scope on the stack. use Context#stack instead def push(new_scope = {}) @scopes.unshift(new_scope) - raise StackLevelError, "Nesting too deep".freeze if @scopes.length > Block::MAX_DEPTH + check_overflow end # Merge a hash of variables in the current local scope @@ -126,6 +135,25 @@ module Liquid @this_stack_used = old_stack_used end + # Creates a new context inheriting resource limits, filters, environment etc., + # but with an isolated scope. + def new_isolated_subcontext + check_overflow + + Context.build( + resource_limits: resource_limits, + static_environments: static_environments, + static_registers: static_registers + ).tap do |subcontext| + subcontext.base_scope_depth = base_scope_depth + 1 + subcontext.exception_renderer = exception_renderer + subcontext.filters = @filters + subcontext.strainer = nil + subcontext.errors = errors + subcontext.warnings = warnings + end + end + def clear_instance_assigns @scopes[0] = {} end @@ -164,25 +192,13 @@ module Liquid # This was changed from find() to find_index() because this is a very hot # path and find_index() is optimized in MRI to reduce object allocation index = @scopes.find_index { |s| s.key?(key) } - scope = @scopes[index] if index - variable = nil - - if scope.nil? - @environments.each do |e| - variable = lookup_and_evaluate(e, key, raise_on_not_found: raise_on_not_found) - # When lookup returned a value OR there is no value but the lookup also did not raise - # then it is the value we are looking for. - if !variable.nil? || @strict_variables && raise_on_not_found - scope = e - break - end - end + variable = if index + lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found) + else + try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found) end - scope ||= @environments.last || @scopes.last - variable ||= lookup_and_evaluate(scope, key, raise_on_not_found: raise_on_not_found) - variable = variable.to_liquid variable.context = self if variable.respond_to?(:context=) @@ -203,8 +219,38 @@ module Liquid end end + protected + + attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters + private + attr_reader :base_scope_depth + + def try_variable_find_in_environments(key, raise_on_not_found:) + @environments.each do |environment| + found_variable = lookup_and_evaluate(environment, key, raise_on_not_found: raise_on_not_found) + if !found_variable.nil? || @strict_variables && raise_on_not_found + return found_variable + end + end + @static_environments.each do |environment| + found_variable = lookup_and_evaluate(environment, key, raise_on_not_found: raise_on_not_found) + if !found_variable.nil? || @strict_variables && raise_on_not_found + return found_variable + end + end + nil + end + + def check_overflow + raise StackLevelError, "Nesting too deep".freeze if overflow? + end + + def overflow? + base_scope_depth + @scopes.length > Block::MAX_DEPTH + end + def internal_error # raise and catch to set backtrace and cause on exception raise Liquid::InternalError, 'internal' diff --git a/lib/liquid/locales/en.yml b/lib/liquid/locales/en.yml index 48b3b1d..c0a9aff 100644 --- a/lib/liquid/locales/en.yml +++ b/lib/liquid/locales/en.yml @@ -22,5 +22,6 @@ tag_never_closed: "'%{block_name}' tag was never closed" meta_syntax_error: "Liquid syntax error: #{e.message}" table_row: "Syntax Error in 'table_row loop' - Valid syntax: table_row [item] in [collection] cols=3" + render: "Syntax error in tag 'render' - Template name must be a quoted string" argument: include: "Argument error in tag 'include' - Illegal template name" diff --git a/lib/liquid/partial_cache.rb b/lib/liquid/partial_cache.rb new file mode 100644 index 0000000..d0b8845 --- /dev/null +++ b/lib/liquid/partial_cache.rb @@ -0,0 +1,18 @@ +module Liquid + class PartialCache + def self.load(template_name, context:, parse_context:) + cached_partials = (context.registers[:cached_partials] ||= {}) + cached = cached_partials[template_name] + return cached if cached + + file_system = (context.registers[:file_system] ||= Liquid::Template.file_system) + source = file_system.read_template_file(template_name) + parse_context.partial = true + + partial = Liquid::Template.parse(source, parse_context) + cached_partials[template_name] = partial + ensure + parse_context.partial = false + end + end +end diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 24acf9d..fd86ee4 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -46,7 +46,12 @@ module Liquid template_name = context.evaluate(@template_name_expr) raise ArgumentError.new(options[:locale].t("errors.argument.include")) unless template_name - partial = load_cached_partial(template_name, context) + partial = PartialCache.load( + template_name, + context: context, + parse_context: parse_context + ) + context_variable_name = template_name.split('/'.freeze).last variable = if @variable_name_expr @@ -83,35 +88,9 @@ module Liquid output end - private - alias_method :parse_context, :options private :parse_context - def load_cached_partial(template_name, context) - cached_partials = context.registers[:cached_partials] || {} - - if cached = cached_partials[template_name] - return cached - end - source = read_template_from_file_system(context) - begin - parse_context.partial = true - partial = Liquid::Template.parse(source, parse_context) - ensure - parse_context.partial = false - end - cached_partials[template_name] = partial - context.registers[:cached_partials] = cached_partials - partial - end - - def read_template_from_file_system(context) - file_system = context.registers[:file_system] || Liquid::Template.file_system - - file_system.read_template_file(context.evaluate(@template_name_expr)) - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/increment.rb b/lib/liquid/tags/increment.rb index 5af1242..95875aa 100644 --- a/lib/liquid/tags/increment.rb +++ b/lib/liquid/tags/increment.rb @@ -23,6 +23,7 @@ module Liquid def render_to_output_buffer(context, output) value = context.environments.first[@variable] ||= 0 context.environments.first[@variable] = value + 1 + output << value.to_s output end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb new file mode 100644 index 0000000..2e5310b --- /dev/null +++ b/lib/liquid/tags/render.rb @@ -0,0 +1,54 @@ +module Liquid + class Render < Tag + Syntax = /(#{QuotedString})#{QuotedFragment}*/o + + attr_reader :template_name_expr, :attributes + + def initialize(tag_name, markup, options) + super + + raise SyntaxError.new(options[:locale].t("errors.syntax.render".freeze)) unless markup =~ Syntax + + template_name = $1 + + @template_name_expr = Expression.parse(template_name) + + @attributes = {} + markup.scan(TagAttributes) do |key, value| + @attributes[key] = Expression.parse(value) + end + end + + def render_to_output_buffer(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.new(options[:locale].t("errors.argument.include")) unless template_name + + partial = PartialCache.load( + template_name, + context: context, + parse_context: parse_context + ) + + inner_context = context.new_isolated_subcontext + inner_context.template_name = template_name + inner_context.partial = true + @attributes.each do |key, value| + inner_context[key] = context.evaluate(value) + end + partial.render_to_output_buffer(inner_context, output) + + output + end + + class ParseTreeVisitor < Liquid::ParseTreeVisitor + def children + [ + @node.template_name_expr, + ] + @node.attributes.values + end + end + end + + Template.register_tag('render'.freeze, Render) +end diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb new file mode 100644 index 0000000..a31d018 --- /dev/null +++ b/test/integration/tags/render_tag_test.rb @@ -0,0 +1,149 @@ +require 'test_helper' + +class RenderTagTest < Minitest::Test + include Liquid + + def test_render_with_no_arguments + Liquid::Template.file_system = StubFileSystem.new('source' => 'rendered content') + assert_template_result 'rendered content', '{% render "source" %}' + end + + def test_render_tag_looks_for_file_system_in_registers_first + file_system = StubFileSystem.new('pick_a_source' => 'from register file system') + assert_equal 'from register file system', + Template.parse('{% render "pick_a_source" %}').render!({}, registers: { file_system: file_system }) + end + + def test_render_passes_named_arguments_into_inner_scope + Liquid::Template.file_system = StubFileSystem.new('product' => '{{ inner_product.title }}') + assert_template_result 'My Product', '{% render "product", inner_product: outer_product %}', + 'outer_product' => { 'title' => 'My Product' } + end + + def test_render_accepts_literals_as_arguments + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ price }}') + assert_template_result '123', '{% render "snippet", price: 123 %}' + end + + def test_render_accepts_multiple_named_arguments + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ one }} {{ two }}') + assert_template_result '1 2', '{% render "snippet", one: 1, two: 2 %}' + end + + def test_render_does_not_inherit_parent_scope_variables + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ outer_variable }}') + assert_template_result '', '{% assign outer_variable = "should not be visible" %}{% render "snippet" %}' + end + + def test_render_does_not_inherit_variable_with_same_name_as_snippet + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ snippet }}') + assert_template_result '', "{% assign snippet = 'should not be visible' %}{% render 'snippet' %}" + end + + def test_render_sets_the_correct_template_name_for_errors + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') + + with_taint_mode :error do + template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}') + context = Context.new('unsafe' => (+'unsafe').tap(&:taint)) + template.render(context) + + assert_equal [Liquid::TaintedError], template.errors.map(&:class) + assert_equal 'snippet', template.errors.first.template_name + end + end + + def test_render_sets_the_correct_template_name_for_warnings + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') + + with_taint_mode :warn do + template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}') + context = Context.new('unsafe' => (+'unsafe').tap(&:taint)) + template.render(context) + + assert_equal [Liquid::TaintedError], context.warnings.map(&:class) + assert_equal 'snippet', context.warnings.first.template_name + end + end + + def test_render_does_not_mutate_parent_scope + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{% assign inner = 1 %}') + assert_template_result '', "{% render 'snippet' %}{{ inner }}" + end + + def test_nested_render_tag + Liquid::Template.file_system = StubFileSystem.new( + 'one' => "one {% render 'two' %}", + 'two' => 'two' + ) + assert_template_result 'one two', "{% render 'one' %}" + end + + def test_recursively_rendered_template_does_not_produce_endless_loop + Liquid::Template.file_system = StubFileSystem.new('loop' => '{% render "loop" %}') + + assert_raises Liquid::StackLevelError do + Template.parse('{% render "loop" %}').render! + end + end + + def test_includes_and_renders_count_towards_the_same_recursion_limit + Liquid::Template.file_system = StubFileSystem.new( + 'loop_render' => '{% render "loop_include" %}', + 'loop_include' => '{% include "loop_render" %}' + ) + + assert_raises Liquid::StackLevelError do + Template.parse('{% render "loop_include" %}').render! + end + end + + def test_dynamically_choosen_templates_are_not_allowed + Liquid::Template.file_system = StubFileSystem.new('snippet' => 'should not be rendered') + + assert_raises Liquid::SyntaxError do + Liquid::Template.parse("{% assign name = 'snippet' %}{% render name %}") + end + end + + def test_include_tag_caches_second_read_of_same_partial + file_system = StubFileSystem.new('snippet' => 'echo') + assert_equal 'echoecho', + Template.parse('{% render "snippet" %}{% render "snippet" %}') + .render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.file_read_count + end + + def test_render_tag_doesnt_cache_partials_across_renders + file_system = StubFileSystem.new('snippet' => 'my message') + + assert_equal 'my message', + Template.parse('{% include "snippet" %}').render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.file_read_count + + assert_equal 'my message', + Template.parse('{% include "snippet" %}').render!({}, registers: { file_system: file_system }) + assert_equal 2, file_system.file_read_count + end + + def test_render_tag_within_if_statement + Liquid::Template.file_system = StubFileSystem.new('snippet' => 'my message') + assert_template_result 'my message', '{% if true %}{% render "snippet" %}{% endif %}' + end + + def test_break_through_render + Liquid::Template.file_system = StubFileSystem.new('break' => '{% break %}') + assert_template_result '1', '{% for i in (1..3) %}{{ i }}{% break %}{{ i }}{% endfor %}' + assert_template_result '112233', '{% for i in (1..3) %}{{ i }}{% render "break" %}{{ i }}{% endfor %}' + end + + def test_increment_is_isolated_between_renders + Liquid::Template.file_system = StubFileSystem.new('incr' => '{% increment %}') + assert_template_result '010', '{% increment %}{% increment %}{% render "incr" %}' + end + + def test_decrement_is_isolated_between_renders + Liquid::Template.file_system = StubFileSystem.new('decr' => '{% decrement %}') + assert_template_result '-1-2-1', '{% decrement %}{% decrement %}{% render "decr" %}' + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 210333d..27a2434 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -121,3 +121,17 @@ class ErrorDrop < Liquid::Drop raise Exception, 'exception' end end + +class StubFileSystem + attr_reader :file_read_count + + def initialize(values) + @file_read_count = 0 + @values = values + end + + def read_template_file(template_path) + @file_read_count += 1 + @values.fetch(template_path) + end +end diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index fab19b8..9252eb5 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -468,11 +468,79 @@ class ContextUnitTest < Minitest::Test assert_equal 'hi filtered', context.apply_global_filter('hi') end + def test_static_environments_are_read_with_lower_priority_than_environments + context = Context.build( + static_environments: { 'shadowed' => 'static', 'unshadowed' => 'static' }, + environments: { 'shadowed' => 'dynamic' } + ) + + assert_equal 'dynamic', context['shadowed'] + assert_equal 'static', context['unshadowed'] + end + def test_apply_global_filter_when_no_global_filter_exist context = Context.new assert_equal 'hi', context.apply_global_filter('hi') end + def test_new_isolated_subcontext_does_not_inherit_variables + super_context = Context.new + super_context['my_variable'] = 'some value' + subcontext = super_context.new_isolated_subcontext + + assert_nil subcontext['my_variable'] + end + + def test_new_isolated_subcontext_inherits_static_environment + super_context = Context.build(static_environments: { 'my_environment_value' => 'my value' }) + subcontext = super_context.new_isolated_subcontext + + assert_equal 'my value', subcontext['my_environment_value'] + end + + def test_new_isolated_subcontext_inherits_resource_limits + resource_limits = ResourceLimits.new({}) + super_context = Context.new({}, {}, {}, false, resource_limits) + subcontext = super_context.new_isolated_subcontext + assert_equal resource_limits, subcontext.resource_limits + end + + def test_new_isolated_subcontext_inherits_exception_renderer + super_context = Context.new + super_context.exception_renderer = ->(_e) { 'my exception message' } + subcontext = super_context.new_isolated_subcontext + assert_equal 'my exception message', subcontext.handle_error(Liquid::Error.new) + end + + def test_new_isolated_subcontext_does_not_inherit_non_static_registers + registers = { + my_register: :my_value + } + super_context = Context.new({}, {}, registers) + subcontext = super_context.new_isolated_subcontext + assert_nil subcontext.registers[:my_register] + end + + def test_new_isolated_subcontext_inherits_static_registers + super_context = Context.build(static_registers: { my_register: :my_value }) + subcontext = super_context.new_isolated_subcontext + assert_equal :my_value, subcontext.static_registers[:my_register] + end + + def test_new_isolated_subcontext_inherits_filters + my_filter = Module.new do + def my_filter(*) + 'my filter result' + end + end + + super_context = Context.new + super_context.add_filters([my_filter]) + subcontext = super_context.new_isolated_subcontext + template = Template.parse('{{ 123 | my_filter }}') + assert_equal 'my filter result', template.render(subcontext) + end + private def assert_no_object_allocations diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb new file mode 100644 index 0000000..29f1144 --- /dev/null +++ b/test/unit/partial_cache_unit_test.rb @@ -0,0 +1,91 @@ +require 'test_helper' + +class PartialCacheUnitTest < Minitest::Test + def test_uses_the_file_system_register_if_present + context = Liquid::Context.build( + registers: { + file_system: StubFileSystem.new('my_partial' => 'my partial body') + } + ) + + partial = Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new + ) + + assert_equal 'my partial body', partial.render + end + + def test_reads_from_the_file_system_only_once_per_file + file_system = StubFileSystem.new('my_partial' => 'some partial body') + context = Liquid::Context.build( + registers: { file_system: file_system } + ) + + 2.times do + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new + ) + end + + assert_equal 1, file_system.file_read_count + end + + def test_cache_state_is_stored_per_context + parse_context = Liquid::ParseContext.new + shared_file_system = StubFileSystem.new( + 'my_partial' => 'my shared value' + ) + context_one = Liquid::Context.build( + registers: { + file_system: shared_file_system + } + ) + context_two = Liquid::Context.build( + registers: { + file_system: shared_file_system + } + ) + + 2.times do + Liquid::PartialCache.load( + 'my_partial', + context: context_one, + parse_context: parse_context + ) + end + + Liquid::PartialCache.load( + 'my_partial', + context: context_two, + parse_context: parse_context + ) + + assert_equal 2, shared_file_system.file_read_count + end + + def test_cache_is_not_broken_when_a_different_parse_context_is_used + file_system = StubFileSystem.new('my_partial' => 'some partial body') + context = Liquid::Context.build( + registers: { file_system: file_system } + ) + + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new(my_key: 'value one') + ) + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new(my_key: 'value two') + ) + + # Technically what we care about is that the file was parsed twice, + # but measuring file reads is an OK proxy for this. + assert_equal 1, file_system.file_read_count + end +end