From 11dc18bfdf2d85ffb4b76e54af4e381fef62e9d2 Mon Sep 17 00:00:00 2001 From: James MacAulay Date: Wed, 23 Sep 2009 15:44:29 -0400 Subject: [PATCH] A better fix for "and"/"or" in strings (now with less side effects) --- lib/liquid/condition.rb | 4 ---- lib/liquid/tags/if.rb | 8 +++----- test/parsing_quirks_test.rb | 13 +++++++++++++ test/statements_test.rb | 12 ++++++------ 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index 06d2034..ade7ac1 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -21,10 +21,6 @@ module Liquid def self.operators @@operators end - - def self.operator_regexp - Regexp.new(@@operators.keys.sort_by(&:length).reverse.map {|k| Regexp.escape(k)}.join('|')) - end attr_reader :attachment attr_accessor :left, :operator, :right diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 304d8f1..608d11a 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -13,8 +13,8 @@ module Liquid # class If < Block SyntaxHelp = "Syntax Error in tag 'if' - Valid syntax: if [expression]" - Syntax = /(#{QuotedFragment})\s*(?:([=!<>a-z_]+)\s*(#{QuotedFragment}))?/ - ExpressionsAndOperators = /and|or|#{QuotedFragment}\s*(?:(?!and|or)(?:[=!<>a-z_]+)\s*#{QuotedFragment})?/ + Syntax = /(#{QuotedFragment})\s*([=!<>a-z_]+)?\s*(#{QuotedFragment})?/ + ExpressionsAndOperators = /(?:and|or|(?:\s*(?!\b(?:and|or)\b)(?:#{QuotedFragment}|\S+)\s*)+)/ def initialize(tag_name, markup, tokens) @@ -51,9 +51,7 @@ module Liquid ElseCondition.new else - # expressions = markup.split(/\b(and|or)\b/).reverse - - expressions = markup.scan(ExpressionsAndOperators).flatten.compact.reverse + expressions = markup.scan(ExpressionsAndOperators).reverse raise(SyntaxError, SyntaxHelp) unless expressions.shift =~ Syntax condition = Condition.new($1, $2, $3) diff --git a/test/parsing_quirks_test.rb b/test/parsing_quirks_test.rb index 5cebd52..2f2aa4a 100644 --- a/test/parsing_quirks_test.rb +++ b/test/parsing_quirks_test.rb @@ -38,4 +38,17 @@ class ParsingQuirksTest < Test::Unit::TestCase end end + def test_meaningless_parens + assigns = {'b' => 'bar', 'c' => 'baz'} + markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" + assert_template_result(' YES ',"{% if #{markup} %} YES {% endif %}", assigns) + end + + def test_unexpected_characters_silently_eat_logic + markup = "true && false" + assert_template_result(' YES ',"{% if #{markup} %} YES {% endif %}") + markup = "false || true" + assert_template_result('',"{% if #{markup} %} YES {% endif %}") + end + end \ No newline at end of file diff --git a/test/statements_test.rb b/test/statements_test.rb index 25e272a..63a4104 100644 --- a/test/statements_test.rb +++ b/test/statements_test.rb @@ -17,31 +17,31 @@ class StatementsTest < Test::Unit::TestCase assert_equal expected, Template.parse(text).render end - def test_zero_gt_zero + def test_true_lq_true text = %| {% if 0 > 0 %} true {% else %} false {% endif %} | expected = %| false | assert_equal expected, Template.parse(text).render end - def test_one_gt_zero + def test_one_lq_zero text = %| {% if 1 > 0 %} true {% else %} false {% endif %} | expected = %| true | assert_equal expected, Template.parse(text).render end - def test_zero_lt_one + def test_zero_lq_one text = %| {% if 0 < 1 %} true {% else %} false {% endif %} | expected = %| true | assert_equal expected, Template.parse(text).render end - def test_zero_lt_or_equal_zero + def test_zero_lq_or_equal_one text = %| {% if 0 <= 0 %} true {% else %} false {% endif %} | expected = %| true | assert_equal expected, Template.parse(text).render end - def test_zero_lt_or_equal_zero_involving_nil + def test_zero_lq_or_equal_one_involving_nil text = %| {% if null <= 0 %} true {% else %} false {% endif %} | expected = %| false | assert_equal expected, Template.parse(text).render @@ -52,7 +52,7 @@ class StatementsTest < Test::Unit::TestCase assert_equal expected, Template.parse(text).render end - def test_zero_gt_or_equal_zero + def test_zero_lqq_or_equal_one text = %| {% if 0 >= 0 %} true {% else %} false {% endif %} | expected = %| true | assert_equal expected, Template.parse(text).render