From 87e3234b58969d779e33a5fa7ceda80aa1ad0305 Mon Sep 17 00:00:00 2001
From: Dylan Thacker-Smith <Dylan.Smith@shopify.com>
Date: Thu, 9 Jul 2015 14:33:10 -0400
Subject: [PATCH] Add a lax warning mode to see where we can make the lax
 parser stricter.

---
 lib/liquid/parser_switching.rb    |  2 +-
 lib/liquid/variable.rb            | 28 +++++++++++++++++++---------
 test/integration/variable_test.rb | 28 ++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb
index 3aa664a..bf157f2 100644
--- a/lib/liquid/parser_switching.rb
+++ b/lib/liquid/parser_switching.rb
@@ -3,7 +3,7 @@ module Liquid
     def parse_with_selected_parser(markup)
       case parse_context.error_mode
       when :strict then strict_parse_with_error_context(markup)
-      when :lax    then lax_parse(markup)
+      when :lax, :lax_warn then lax_parse(markup)
       when :warn
         begin
           return strict_parse_with_error_context(markup)
diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb
index 5c62ffd..f011dfc 100644
--- a/lib/liquid/variable.rb
+++ b/lib/liquid/variable.rb
@@ -10,7 +10,7 @@ module Liquid
   #   {{ user | link }}
   #
   class Variable
-    FilterParser = /(?:\s+|#{QuotedFragment}|#{ArgumentSeparator})+/o
+    FilterParser = /\s*(?:#{FilterSeparator}|(['"\|]+?))\s*((?:\s+|#{QuotedFragment}|#{ArgumentSeparator})+)/o
     attr_accessor :filters, :name, :line_number
     attr_reader :parse_context
     alias_method :options, :parse_context
@@ -35,16 +35,19 @@ module Liquid
 
     def lax_parse(markup)
       @filters = []
-      return unless markup =~ /(#{QuotedFragment})(.*)/om
+      return unless markup =~ /\A\s*([\s,\|'"]+?)??\s*(#{QuotedFragment})\s*(?:([^\|]+?)??\s*(#{FilterSeparator}.*))?\s*\z/om
 
-      name_markup = $1
-      filter_markup = $2
+      add_syntax_warning("variable prefixed with ignored characters: #{$1.inspect}") if $1
+      name_markup = $2
+      add_syntax_warning("variable filter seperator prefixed with ignored characters: #{$3.inspect}") if $3
+      filters_markup = $4
       @name = Expression.parse(name_markup)
-      if filter_markup =~ /#{FilterSeparator}\s*(.*)/om
-        filters = $1.scan(FilterParser)
-        filters.each do |f|
-          next unless f =~ /\w+/
-          filtername = Regexp.last_match(0)
+      if filters_markup
+        filters_markup.scan(FilterParser) do |sep, f|
+          add_syntax_warning("unterminated quote or multiple pipe characters used as a filter seperator: #{sep.inspect}") if sep
+          next unless f =~ /\A\s*(\W+)??(\w+)/
+          add_syntax_warning("ignored characters before filter name: #{$1.inspect}") if $1
+          filtername = $2
           filterargs = f.scan(/(?:#{FilterArgumentSeparator}|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o).flatten
           @filters << parse_filter_expressions(filtername, filterargs)
         end
@@ -81,6 +84,13 @@ module Liquid
 
     private
 
+    def add_syntax_warning(warning)
+      return unless parse_context.error_mode == :lax_warn
+      error = SyntaxError.new(warning)
+      error.line_number = parse_context.line_number
+      parse_context.warnings << error
+    end
+
     def parse_filter_expressions(filter_name, unparsed_args)
       filter_args = []
       keyword_args = {}
diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb
index 58df833..38bf644 100644
--- a/test/integration/variable_test.rb
+++ b/test/integration/variable_test.rb
@@ -89,4 +89,32 @@ class VariableTest < Minitest::Test
   def test_multiline_variable
     assert_equal 'worked', Template.parse("{{\ntest\n}}").render!('test' => 'worked')
   end
+
+  def test_lax_warnings_for_variable_ignored_prefix
+    ignored_chars = %(|,'")
+    template = Liquid::Template.parse("{{ #{ignored_chars}test }}", error_mode: :lax_warn)
+    assert_equal "works", template.render!('test' => 'works')
+    assert_equal [Liquid::SyntaxError.new("variable prefixed with ignored characters: #{ignored_chars.inspect}")], template.warnings
+  end
+
+  def test_lax_warnings_for_ignored_variable_filter_prefix
+    ignored_chars = ",wat? lax!"
+    template = Liquid::Template.parse("{{ test#{ignored_chars} | noop }}", error_mode: :lax_warn)
+    assert_equal "works", template.render!('test' => 'works')
+    assert_equal [Liquid::SyntaxError.new("variable filter seperator prefixed with ignored characters: #{ignored_chars.inspect}")], template.warnings
+  end
+
+  def test_lax_warnings_for_weird_filter_chars
+    template = Liquid::Template.parse("{{ test | upcase \" prepend: 'it ' || append: ' surprisingly' }}", error_mode: :lax_warn)
+    assert_equal "it WORKS surprisingly", template.render!('test' => 'works')
+    expected_warnings = ['"', '||'].map{ |sep| Liquid::SyntaxError.new("unterminated quote or multiple pipe characters used as a filter seperator: #{sep.inspect}") }
+    assert_equal expected_warnings, template.warnings
+  end
+
+  def test_lax_warnings_for_ignored_filter_name_prefix
+    ignored_chars = "'': '"
+    template = Liquid::Template.parse("{{ test | '': 'prepend ' }}", error_mode: :lax_warn)
+    assert_equal "prepend wat", template.render!('test' => 'wat')
+    assert_equal [Liquid::SyntaxError.new("ignored characters before filter name: #{ignored_chars.inspect}")], template.warnings
+  end
 end