search

Shopify / liquid

Liquid markup language. Safe, customer facing template language for flexible web apps.
https://shopify.github.io/liquid/
MIT License
11.15k stars 1.4k forks source link

The {% comment %} tag parses its body #936

Open davidcornu opened 7 years ago

davidcornu commented 7 years ago

Problem

It would be reasonable to expect any text between {% comment %} and {% endcomment %} to be completely ignored.

However, that is not the case:

❯ irb
irb(main):001:0> require 'liquid'
=> true
irb(main):002:0> Liquid::VERSION
=> "4.0.0"
irb(main):003:0> Liquid::Template.parse("{% comment %}{% if 'foo' %}{% endcomment %}")
Liquid::SyntaxError: Liquid syntax error: Unknown tag 'endcomment'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block.rb:38:in `unknown_tag'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/tags/if.rb:36:in `unknown_tag'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block.rb:68:in `block in parse_body'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block_body.rb:33:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block.rb:58:in `parse_body'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/tags/if.rb:24:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/tag.rb:10:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block_body.rb:27:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block.rb:58:in `parse_body'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block.rb:12:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/tag.rb:10:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/block_body.rb:27:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/document.rb:10:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/document.rb:5:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/template.rb:132:in `parse'
    from /Users/david/.gem/ruby/2.3.3/bundler/gems/liquid-c582b86f16b6/lib/liquid/template.rb:116:in `parse'

Cause

I believe this is because Liquid::Comment inherits from Liquid::Block

https://github.com/Shopify/liquid/blob/27c91203ab4adfde2cc5e1142bce2a267634ef8e/lib/liquid/tags/comment.rb#L2

which parses its body

https://github.com/Shopify/liquid/blob/27c91203ab4adfde2cc5e1142bce2a267634ef8e/lib/liquid/block.rb#L12

My initial thought was that this should have been caught by Liquid::Comment#unknown_tag

https://github.com/Shopify/liquid/blob/27c91203ab4adfde2cc5e1142bce2a267634ef8e/lib/liquid/tags/comment.rb#L7-L8

but the exception is being raised from within Liquid::If#unknown_tag

https://github.com/Shopify/liquid/blob/27c91203ab4adfde2cc5e1142bce2a267634ef8e/lib/liquid/tags/if.rb#L32-L36

which makes sense seen as {% if %} owns its own body.

Fix?

A potential solution might be to override Liquid::Block#parse

https://github.com/Shopify/liquid/blob/27c91203ab4adfde2cc5e1142bce2a267634ef8e/lib/liquid/block.rb#L10-L14

in Liquid::Comment seen as #render always returns an empty string.

davidcornu commented 7 years ago

This particular issue was pointed out in https://github.com/Shopify/liquid/pull/257

image