Benchmark marshal load and render against parse and render.

Make it easy to set default resource limits.

History.md

@@ -3,6 +3,8 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
+* Fixed condition with wrong data types, see #423 [Bogdan Gusiev]
+* Add url_encode to standard filters, see #421 [Derrick Reimer, djreimer]
 * Add uniq to standard filters [Florian Weingarten, fw42]
 * Add exception_handler feature, see #397 and #254 [Bogdan Gusiev, bogdan and Florian Weingarten, fw42]
 * Optimize variable parsing to avoid repeated regex evaluation during template rendering #383 [Jason Hiltz-Laforge, jasonhl]

lib/liquid.rb

@@ -54,6 +54,7 @@ require 'liquid/interrupts'
 require 'liquid/strainer'
 require 'liquid/expression'
 require 'liquid/context'
+require 'liquid/parser_switching'
 require 'liquid/tag'
 require 'liquid/block'
 require 'liquid/document'
@@ -66,11 +67,11 @@ require 'liquid/standardfilters'
 require 'liquid/condition'
 require 'liquid/module_ex'
 require 'liquid/utils'
+require 'liquid/token'
 
 # Load all the tags of the standard library
 #
 Dir[File.dirname(__FILE__) + '/liquid/tags/*.rb'].each { |f| require f }
 
 require 'liquid/profiler'
-require 'liquid/profiler/token'
 require 'liquid/profiler/hooks'

lib/liquid/block.rb

@@ -32,7 +32,8 @@ module Liquid
 
               # fetch the tag from registered blocks
               if tag = Template.tags[$1]
-                new_tag = tag.parse($1, $2, tokens, @options)
+                markup = token.is_a?(Token) ? token.child($2) : $2
+                new_tag = tag.parse($1, markup, tokens, @options)
                 new_tag.line_number = token.line_number if token.is_a?(Token)
                 @blank &&= new_tag.blank?
                 @nodelist << new_tag
@@ -103,7 +104,8 @@ module Liquid
 
     def create_variable(token)
       token.scan(ContentOfVariable) do |content|
-        return Variable.new(content.first, @options)
+        markup = token.is_a?(Token) ? token.child(content.first) : content.first
+        return Variable.new(markup, @options)
       end
       raise SyntaxError.new(options[:locale].t("errors.syntax.variable_termination".freeze, :token => token, :tag_end => VariableEnd.inspect))
     end
@@ -144,7 +146,7 @@ module Liquid
         rescue MemoryError => e
           raise e
         rescue ::StandardError => e
-          output << (context.handle_error(e))
+          output << (context.handle_error(e, token))
         end
       end
 

lib/liquid/condition.rb

@@ -15,7 +15,9 @@ module Liquid
       '>'.freeze  => :>,
       '>='.freeze => :>=,
       '<='.freeze => :<=,
-      'contains'.freeze => lambda { |cond, left, right| left && right ? left.include?(right) : false }
+      'contains'.freeze => lambda { |cond, left, right|
+        left && right && left.respond_to?(:include?) ? left.include?(right) : false
+      }
     }
 
     def self.operators

lib/liquid/context.rb

@@ -16,12 +16,14 @@ module Liquid
     attr_reader :scopes, :errors, :registers, :environments, :resource_limits
     attr_accessor :exception_handler
 
-    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = {})
+    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil)
       @environments     = [environments].flatten
       @scopes           = [(outer_scope || {})]
       @registers        = registers
       @errors           = []
-      @resource_limits  = (resource_limits || {}).merge!({ :render_score_current => 0, :assign_score_current => 0 })
+      @resource_limits  = resource_limits || Template.default_resource_limits
+      @resource_limits[:render_score_current] = 0
+      @resource_limits[:assign_score_current] = 0
       @parsed_expression = Hash.new{ |cache, markup| cache[markup] = Expression.parse(markup) }
       squash_instance_assigns_with_environments
 
@@ -91,17 +93,15 @@ module Liquid
       @interrupts.pop
     end
 
-    def handle_error(e)
-      errors.push(e)
 
-      raise if exception_handler && exception_handler.call(e)
-
-      case e
-      when SyntaxError
-        "Liquid syntax error: #{e.message}"
-      else
-        "Liquid error: #{e.message}"
+    def handle_error(e, token=nil)
+      if e.is_a?(Liquid::Error)
+        e.set_line_number_from_token(token)
       end
+
+      errors.push(e)
+      raise if exception_handler && exception_handler.call(e)
+      Liquid::Error.render(e)
     end
 
     def invoke(method, *args)

lib/liquid/errors.rb

@@ -1,5 +1,52 @@
 module Liquid
-  class Error < ::StandardError; end
+  class Error < ::StandardError
+    attr_accessor :line_number
+    attr_accessor :markup_context
+
+    def to_s(with_prefix=true)
+      str = ""
+      str << message_prefix if with_prefix
+      str << super()
+
+      if markup_context
+        str << " "
+        str << markup_context
+      end
+
+      str
+    end
+
+    def set_line_number_from_token(token)
+      return unless token.respond_to?(:line_number)
+      self.line_number = token.line_number
+    end
+
+    def self.render(e)
+      if e.is_a?(Liquid::Error)
+        e.to_s
+      else
+        "Liquid error: #{e.to_s}"
+      end
+    end
+
+    private
+
+    def message_prefix
+      str = ""
+      if is_a?(SyntaxError)
+        str << "Liquid syntax error"
+      else
+        str << "Liquid error"
+      end
+
+      if line_number
+        str << " (line #{line_number})"
+      end
+
+      str << ": "
+      str
+    end
+  end
 
   class ArgumentError < Error; end
   class ContextError < Error; end

lib/liquid/parser_switching.rb

@@ -0,0 +1,31 @@
+module Liquid
+  module ParserSwitching
+    def parse_with_selected_parser(markup)
+      case @options[:error_mode] || Template.error_mode
+      when :strict then strict_parse_with_error_context(markup)
+      when :lax    then lax_parse(markup)
+      when :warn
+        begin
+          return strict_parse_with_error_context(markup)
+        rescue SyntaxError => e
+          e.set_line_number_from_token(markup)
+          @warnings ||= []
+          @warnings << e
+          return lax_parse(markup)
+        end
+      end
+    end
+
+    private
+    def strict_parse_with_error_context(markup)
+      strict_parse(markup)
+    rescue SyntaxError => e
+      e.markup_context = markup_context(markup)
+      raise e
+    end
+
+    def markup_context(markup)
+      "in \"#{markup.strip}\""
+    end
+  end
+end

lib/liquid/standardfilters.rb

@@ -42,6 +42,10 @@ module Liquid
       input.to_s.gsub(HTML_ESCAPE_ONCE_REGEXP, HTML_ESCAPE)
     end
 
+    def url_encode(input)
+      CGI.escape(input) rescue input
+    end
+
     def slice(input, offset, length=nil)
       offset = Integer(offset)
       length = length ? Integer(length) : 1

lib/liquid/tag.rb

@@ -2,6 +2,7 @@ module Liquid
   class Tag
     attr_accessor :options, :line_number
     attr_reader :nodelist, :warnings
+    include ParserSwitching
 
     class << self
       def parse(tag_name, markup, tokens, options)
@@ -37,29 +38,5 @@ module Liquid
     def blank?
       false
     end
-
-    def parse_with_selected_parser(markup)
-      case @options[:error_mode] || Template.error_mode
-      when :strict then strict_parse_with_error_context(markup)
-      when :lax    then lax_parse(markup)
-      when :warn
-        begin
-          return strict_parse_with_error_context(markup)
-        rescue SyntaxError => e
-          @warnings ||= []
-          @warnings << e
-          return lax_parse(markup)
-        end
-      end
-    end
-
-    private
-
-    def strict_parse_with_error_context(markup)
-      strict_parse(markup)
-    rescue SyntaxError => e
-      e.message << " in \"#{markup.strip}\"" 
-      raise e
-    end
   end
 end

lib/liquid/tags/include.rb

@@ -69,7 +69,7 @@ module Liquid
           return cached
         end
         source = read_template_from_file_system(context)
-        partial = Liquid::Template.parse(source)
+        partial = Liquid::Template.parse(source, pass_options)
         cached_partials[template_name] = partial
         context.registers[:cached_partials] = cached_partials
         partial
@@ -88,6 +88,16 @@ module Liquid
           raise ArgumentError, "file_system.read_template_file expects two parameters: (template_name, context)"
         end
       end
+
+      def pass_options
+        dont_pass = @options[:include_options_blacklist]
+        return {locale: @options[:locale]} if dont_pass == true
+        opts = @options.merge(included: true, include_options_blacklist: false)
+        if dont_pass.is_a?(Array)
+          dont_pass.each {|o| opts.delete(o)}
+        end
+        opts
+      end
   end
 
   Template.register_tag('include'.freeze, Include)

lib/liquid/template.rb

@@ -86,6 +86,10 @@ module Liquid
         Strainer.global_filter(mod)
       end
 
+      def default_resource_limits
+        @default_resource_limits ||= {}
+      end
+
       # creates a new <tt>Template</tt> object from liquid source code
       # To enable profiling, pass in <tt>profile: true</tt> as an option.
       # See Liquid::Profiler for more information
@@ -95,15 +99,16 @@ module Liquid
       end
     end
 
-    # creates a new <tt>Template</tt> from an array of tokens. Use <tt>Template.parse</tt> instead
     def initialize
-      @resource_limits = {}
+      @resource_limits = self.class.default_resource_limits.dup
     end
 
     # Parse source code.
     # Returns self for easy chaining
     def parse(source, options = {})
-      @profiling = options.delete(:profile)
+      @options = options
+      @profiling = options[:profile]
+      @line_numbers = options[:line_numbers] || @profiling
       @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options))
       @warnings = nil
       self
@@ -223,7 +228,7 @@ module Liquid
     end
 
     def calculate_line_numbers(raw_tokens)
-      return raw_tokens unless @profiling
+      return raw_tokens unless @line_numbers
 
       current_line = 1
       raw_tokens.map do |token|
@@ -234,7 +239,7 @@ module Liquid
     end
 
     def with_profiling
-      if @profiling
+      if @profiling && !@options[:included]
         @profiler = Profiler.new
         @profiler.start
 
@@ -247,6 +252,5 @@ module Liquid
         yield
       end
     end
-
   end
 end

lib/liquid/token.rb

@@ -1,6 +1,5 @@
 module Liquid
   class Token < String
-
     attr_reader :line_number
 
     def initialize(content, line_number)
@@ -12,5 +11,8 @@ module Liquid
       "<raw>"
     end
 
+    def child(string)
+      Token.new(string, @line_number)
+    end
   end
 end

lib/liquid/variable.rb

@@ -15,30 +15,24 @@ module Liquid
     EasyParse = /\A *(\w+(?:\.\w+)*) *\z/
     attr_accessor :filters, :name, :warnings
     attr_accessor :line_number
+    include ParserSwitching
 
     def initialize(markup, options = {})
       @markup  = markup
       @name    = nil
       @options = options || {}
 
-      case @options[:error_mode] || Template.error_mode
-      when :strict then strict_parse(markup)
-      when :lax    then lax_parse(markup)
-      when :warn
-        begin
-          strict_parse(markup)
-        rescue SyntaxError => e
-          @warnings ||= []
-          @warnings << e
-          lax_parse(markup)
-        end
-      end
+      parse_with_selected_parser(markup)
     end
 
     def raw
       @markup
     end
 
+    def markup_context(markup)
+      "in \"{{#{markup}}}\""
+    end
+
     def lax_parse(markup)
       @filters = []
       if markup =~ /\s*(#{QuotedFragment})(.*)/om
@@ -74,9 +68,6 @@ module Liquid
         @filters << [filtername, filterargs]
       end
       p.consume(:end_of_string)
-    rescue SyntaxError => e
-      e.message << " in \"{{#{markup}}}\""
-      raise e
     end
 
     def parse_filterargs(p)

performance/benchmark.rb

@@ -1,17 +1,14 @@
-require 'benchmark/ips'
+require 'benchmark'
 require File.dirname(__FILE__) + '/theme_runner'
 
 Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
 
-Benchmark.ips do |x|
-  x.time = 60
-  x.warmup = 5
-
-  puts
-  puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)."
-  puts
-
-  x.report("parse:") { profiler.compile }
-  x.report("parse & run:") { profiler.run }
+N = 100
+Benchmark.bmbm do |x|
+  x.report("parse:")                 { N.times { profiler.parse } }
+  x.report("marshal load:")          { N.times { profiler.marshal_load } }
+  x.report("render:")                { N.times { profiler.render } }
+  x.report("marshal load & render:") { N.times { profiler.load_and_render } }
+  x.report("parse & render:")        { N.times { profiler.parse_and_render } }
 end

performance/profile.rb

@@ -3,13 +3,13 @@ require File.dirname(__FILE__) + '/theme_runner'
 
 Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
-profiler.run
+profiler.parse_and_render
 
 [:cpu, :object].each do |profile_type|
   puts "Profiling in #{profile_type.to_s} mode..."
   results = StackProf.run(mode: profile_type) do
     100.times do
-      profiler.run
+      profiler.parse_and_render
     end
   end
   StackProf::Report.new(results).print_text(false, 20)

performance/theme_runner.rb

@@ -32,45 +32,59 @@ class ThemeRunner
 
       [File.read(test), (File.file?(theme_path) ? File.read(theme_path) : nil), test]
     end.compact
-  end
-
-  def compile
-    # Dup assigns because will make some changes to them
-
-    @tests.each do |liquid, layout, template_name|
-
-      tmpl = Liquid::Template.new
-      tmpl.parse(liquid)
-      tmpl = Liquid::Template.new
-      tmpl.parse(layout)
+    @parsed = @tests.map do |liquid, layout, template_name|
+      [Liquid::Template.parse(liquid), Liquid::Template.parse(layout), template_name]
+    end
+    @marshaled = @parsed.map do |liquid, layout, template_name|
+      [Marshal.dump(liquid), Marshal.dump(layout), template_name]
     end
   end
 
-   def run
+  def parse
+    @tests.each do |liquid, layout, template_name|
+      Liquid::Template.parse(liquid)
+      Liquid::Template.parse(layout)
+    end
+  end
+
+  def marshal_load
+    @marshaled.each do |liquid, layout, template_name|
+      Marshal.load(liquid)
+      Marshal.load(layout)
+    end
+  end
+
+  def render
+    @parsed.each do |liquid, layout, template_name|
+      render_once(liquid, layout, template_name)
+    end
+  end
+
+  def load_and_render
+    @marshaled.each do |liquid, layout, template_name|
+      render_once(Marshal.load(liquid), Marshal.load(layout), template_name)
+    end
+  end
+
+  def parse_and_render
+    @tests.each do |liquid, layout, template_name|
+      render_once(Liquid::Template.parse(liquid), Liquid::Template.parse(layout), template_name)
+    end
+  end
+
+  def render_once(template, layout, template_name)
     # Dup assigns because will make some changes to them
     assigns = Database.tables.dup
 
-    @tests.each do |liquid, layout, template_name|
+    assigns['page_title'] = 'Page title'
+    assigns['template'] = File.basename(template_name, File.extname(template_name))
+    template.registers[:file_system] = ThemeRunner::FileSystem.new(File.dirname(template_name))
 
-      # Compute page_tempalte outside of profiler run, uninteresting to profiler
-      page_template = File.basename(template_name, File.extname(template_name))
-      compile_and_render(liquid, layout, assigns, page_template, template_name)
-
-    end
-  end
-
-
-  def compile_and_render(template, layout, assigns, page_template, template_file)
-    tmpl = Liquid::Template.new
-    tmpl.assigns['page_title'] = 'Page title'
-    tmpl.assigns['template'] = page_template
-    tmpl.registers[:file_system] = ThemeRunner::FileSystem.new(File.dirname(template_file))
-
-    content_for_layout = tmpl.parse(template).render!(assigns)
+    content_for_layout = template.render!(assigns)
 
     if layout
       assigns['content_for_layout'] = content_for_layout
-      tmpl.parse(layout).render!(assigns)
+      layout.render!(assigns)
     else
       content_for_layout
     end

test/integration/error_handling_test.rb

@@ -22,6 +22,39 @@ end
 class ErrorHandlingTest < Minitest::Test
   include Liquid
 
+  def test_templates_parsed_with_line_numbers_renders_them_in_errors
+    template = <<-LIQUID
+      Hello,
+
+      {{ errors.standard_error }} will raise a standard error.
+
+      Bla bla test.
+
+      {{ errors.syntax_error }} will raise a syntax error.
+
+      This is an argument error: {{ errors.argument_error }}
+
+      Bla.
+    LIQUID
+
+    expected = <<-TEXT
+      Hello,
+
+      Liquid error (line 3): standard error will raise a standard error.
+
+      Bla bla test.
+
+      Liquid syntax error (line 7): syntax error will raise a syntax error.
+
+      This is an argument error: Liquid error (line 9): argument error
+
+      Bla.
+    TEXT
+
+    output = Liquid::Template.parse(template, line_numbers: true).render('errors' => ErrorDrop.new)
+    assert_equal expected, output
+  end
+
   def test_standard_error
     template = Liquid::Template.parse( ' {{ errors.standard_error }} '  )
     assert_equal ' Liquid error: standard error ', template.render('errors' => ErrorDrop.new)
@@ -59,7 +92,7 @@ class ErrorHandlingTest < Minitest::Test
       end
     end
   end
-  
+
   def test_lax_unrecognized_operator
     template = Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', :error_mode => :lax)
     assert_equal ' Liquid error: Unknown operator =! ', template.render
@@ -71,28 +104,37 @@ class ErrorHandlingTest < Minitest::Test
     err = assert_raises(SyntaxError) do
       Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', :error_mode => :strict)
     end
-    assert_equal 'Unexpected character = in "1 =! 2"', err.message
+    assert_equal 'Liquid syntax error: Unexpected character = in "1 =! 2"', err.message
 
     err = assert_raises(SyntaxError) do
       Liquid::Template.parse('{{%%%}}', :error_mode => :strict)
     end
-    assert_equal 'Unexpected character % in "{{%%%}}"', err.message
+    assert_equal 'Liquid syntax error: Unexpected character % in "{{%%%}}"', err.message
   end
 
   def test_warnings
     template = Liquid::Template.parse('{% if ~~~ %}{{%%%}}{% else %}{{ hello. }}{% endif %}', :error_mode => :warn)
     assert_equal 3, template.warnings.size
-    assert_equal 'Unexpected character ~ in "~~~"', template.warnings[0].message
-    assert_equal 'Unexpected character % in "{{%%%}}"', template.warnings[1].message
-    assert_equal 'Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message
+    assert_equal 'Unexpected character ~ in "~~~"', template.warnings[0].to_s(false)
+    assert_equal 'Unexpected character % in "{{%%%}}"', template.warnings[1].to_s(false)
+    assert_equal 'Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].to_s(false)
     assert_equal '', template.render
   end
 
+  def test_warning_line_numbers
+    template = Liquid::Template.parse("{% if ~~~ %}\n{{%%%}}{% else %}\n{{ hello. }}{% endif %}", :error_mode => :warn, :line_numbers => true)
+    assert_equal 'Liquid syntax error (line 1): Unexpected character ~ in "~~~"', template.warnings[0].message
+    assert_equal 'Liquid syntax error (line 2): Unexpected character % in "{{%%%}}"', template.warnings[1].message
+    assert_equal 'Liquid syntax error (line 3): Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message
+    assert_equal 3, template.warnings.size
+    assert_equal [1,2,3], template.warnings.map(&:line_number)
+  end
+
   # Liquid should not catch Exceptions that are not subclasses of StandardError, like Interrupt and NoMemoryError
   def test_exceptions_propagate
     assert_raises Exception do
-      template = Liquid::Template.parse( ' {{ errors.exception }} '  )
+      template = Liquid::Template.parse('{{ errors.exception }}')
       template.render('errors' => ErrorDrop.new)
     end
   end
-end # ErrorHandlingTest
+end

test/integration/render_profiling_test.rb

@@ -48,6 +48,18 @@ class RenderProfilingTest < Minitest::Test
     assert_equal 2, t.profiler[1].line_number
   end
 
+  def test_profiling_includes_line_numbers_of_included_partials
+    t = Template.parse("{% include 'a_template' %}", :profile => true)
+    t.render!
+
+    included_children = t.profiler[0].children
+
+    # {% assign template_name = 'a_template' %}
+    assert_equal 1, included_children[0].line_number
+    # {{ template_name }}
+    assert_equal 2, included_children[1].line_number
+  end
+
   def test_profiling_times_the_rendering_of_tokens
     t = Template.parse("{% include 'a_template' %}", :profile => true)
     t.render!

test/integration/standard_filter_test.rb

@@ -118,6 +118,11 @@ class StandardFiltersTest < Minitest::Test
     assert_equal '&lt;strong&gt;Hulk&lt;/strong&gt;', @filters.escape_once('&lt;strong&gt;Hulk</strong>')
   end
 
+  def test_url_encode
+    assert_equal 'foo%2B1%40example.com', @filters.url_encode('foo+1@example.com')
+    assert_equal nil, @filters.url_encode(nil)
+  end
+
   def test_truncatewords
     assert_equal 'one two three', @filters.truncatewords('one two three', 4)
     assert_equal 'one two...', @filters.truncatewords('one two three', 2)

test/integration/tags/include_tag_test.rb

@@ -132,7 +132,7 @@ class IncludeTagTest < Minitest::Test
 
     Liquid::Template.file_system = infinite_file_system.new
 
-    assert_raises(Liquid::StackLevelError) do
+    assert_raises(Liquid::StackLevelError, SystemStackError) do
       Template.parse("{% include 'loop' %}").render!
     end
 
@@ -209,4 +209,19 @@ class IncludeTagTest < Minitest::Test
     a.render!
     assert_empty a.errors
   end
+
+  def test_passing_options_to_included_templates
+    assert_raises(Liquid::SyntaxError) do
+      Template.parse("{% include template %}", error_mode: :strict).render!("template" => '{{ "X" || downcase }}')
+    end
+    with_error_mode(:lax) do
+      assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true).render!("template" => '{{ "X" || downcase }}')
+    end
+    assert_raises(Liquid::SyntaxError) do
+      Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale]).render!("template" => '{{ "X" || downcase }}')
+    end
+    with_error_mode(:lax) do
+      assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode]).render!("template" => '{{ "X" || downcase }}')
+    end
+  end
 end # IncludeTagTest

test/unit/condition_unit_test.rb

@@ -80,6 +80,10 @@ class ConditionUnitTest < Minitest::Test
     assert_evalutes_false "0", 'contains', 'not_assigned'
   end
 
+  def test_contains_return_false_on_wrong_data_type
+    assert_evalutes_false "1", 'contains', '0'
+  end
+
   def test_or_condition
     condition = Condition.new('1', '==', '2')