Can’t use symbols as hash keys for local variables in Liquid::Template#render about liquid HOT 21 CLOSED

Liquid has traditionally used strings for variables names because they could be user defined and before Ruby 2.2 symbols weren't garbage collected. So in the past we would have had at best a memory leak and at worst a DoS attack vector from using symbols internally to represent variable names.

If liquid uses strings internally but accepted symbols from the application code, then this library would need to convert the symbols to strings using Symbol#to_s. In other words, we wouldn't be benefiting from using symbols as Hash keys and would be allocating new string objects for every call to Symbol#to_s.

Ruby 2.1 isn't maintained upstream anymore, so we should consider dropping support for it and switching to using symbols internally in liquid to represent variables. For backwards compatibility, we could convert String variables names passed in from the application code, but String#to_sym doesn't have the performance concerns that Symbol#to_s has.

So not only can we make this library more pleasant to use by supporting the Ruby 1.9 hash syntax, but we should also be able to improve performance by using Symbol hash keys.

from liquid.

Maybe it's time to reconsider this decision? We use liquid in our app and these hash.stringify_keys and key.to_s don't look good in code. And yes - they hit the performance anyways :)

from liquid.

This isn't really a bug, using symbols was never supported. I'd be happy to accept a patch as long as it doesn't have a negative performance impact.

from liquid.

Oh, my bad. I was looking at the wiki page explaining drops and it uses symbols and it wasn’t working for me. Looking at the other examples I guess that’s just a typo—I just updated it.

from liquid.

That's by design. We didn't want to take the performance hit that comes from that.

-- Tobi
CEO Shopify

Written with thumbs.

On Nov 17, 2011, at 11:18 AM, Cody [email protected] wrote:

Here’s a minimal test case:

Liquid::Template.parse('Hi, {{name}}!').render(:name => 'Cody') #=> 'Hi, !'
Liquid::Template.parse('Hi, {{name}}!').render('name' => 'Cody') #=> 'Hi, Cody!'

This problem occurs for me using both 2.3.0 and 2.2.2.

---

Reply to this email directly or view it on GitHub:
#82

from liquid.

The option to opt-in would be awesome.

from liquid.

I just wanted to chime in and say that that keeping symbols out of the picture is a good idea because it allows us to pass private data into Liquid and not have it show up inside of the liquid layout. For example we use symbols when we are passing the controller into the context for the tags to use view_context to do work without having to add more objects to the stack and over-complicate the stack but we don't want that data to show up in the render. It's a cheap way to have private variables in Liquid contexts.

from liquid.

Bump. I just ran into this and it's pretty lame.

from liquid.

@tobi, would it be absurd to enable some sort of "warning mode" that tells the user "any non-string keys will be inaccessible in your templates"?

from liquid.

fwiw:

            <a href="#object-{{ endpoint[1].resource }}">
                <!--
                    liquid is stupid sometimes
                    this code SHOULD be:
                        { site.data.objects[endpoint[1].resource].name }
                    but liquid does not let you use hashes with variables
                -->
                {% for o in site.data.objects %}
                {% if o[0] == endpoint[1].resource %}
                    {{ o[1].name }}
                {% endif %}
                {% endfor %}
                objects
                <!-- end liquid is stupid sometimes -->
            </a>

from liquid.

It's a good idea to include this. I'd hate to run a check on this during
normal operation though because its unnecessary in production
On Mon, Nov 16, 2015 at 2:37 PM Drew DeVault [email protected]
wrote:

fwiw:

        <a href="#object-{{ endpoint[1].resource }}">
            <!--
                liquid is stupid sometimes
                this code SHOULD be:
                    { site.data.objects[endpoint[1].resource].name }
                but liquid does not let you use hashes with variables
            -->
            {% for o in site.data.objects %}
            {% if o[0] == endpoint[1].resource %}
                {{ o[1].name }}
            {% endif %}
            {% endfor %}
            objects
            <!-- end liquid is stupid sometimes -->
        </a>

—
Reply to this email directly or view it on GitHub
#82 (comment).

• tobi
  CEO Shopify

from liquid.

@SirCmpwn I think that should work in Liquid 3+ with the strict syntax mode enabled. Issue #473

from liquid.

Thanks @pushrax. I'll come back to this issue once GitHub Pages updates Liquid (we'll probably all be retired by then, though).

from liquid.

This is such utter nonsense. I spent hours banging my head against the desk just to realize a hash's symbol properties can never be output by this crippled software, but far be it for the software to give any indication of that fact.

from liquid.

I just ran into this as well, and it was pretty difficult to troubleshoot. We use symbols everywhere, so this breaks our style.

That's by design. We didn't want to take the performance hit that comes from that.

Is that still true?
https://blog.gorbikoff.com/ruby-hashkey-showdown-symbol-vs-string/

It looks like rubocop prefers symbols these days as well.
https://github.com/rubocop-hq/ruby-style-guide#symbols-as-keys

from liquid.

Liquid has traditionally used strings for variables names because they could be user defined before Ruby 2.2 symbols weren't garbage collected. So in the past we would have had at best a memory leak and at worst a DoS attack vector from using symbols internally to represent variable names.

You're implying every Ruby program using symbols since forever has been vulnerable to DoS attacks because of an integral language feature. That sounds like complete nonsense. Either your Ruby program is short-lived, in which case this is a non-issue, or it's a long-term process, in which case the symbols are immutable and shared, so it's still a non-issue. I don't know where you get the idea that you can DoS any Ruby program because it defines symbols, unless this is a bad joke.

from liquid.

You're implying every Ruby program using symbols since forever has been vulnerable to DoS attacks because of an integral language feature. That sounds like complete nonsense

That's not what he's saying. The problem is (was) not using symbols per se but converting arbitrary user-supplied input strings to symbols.

https://bugs.ruby-lang.org/issues/7791
http://brakemanscanner.org/docs/warning_types/denial_of_service/index.html

from liquid.

I think maybe the piece that's missing in your understanding is that (in Shopify's use-case) we allow people (i.e. attackers) to send us Liquid source code (i.e. attacker-controlled input) which we will then run for them (on our servers, using our memory). That's a very different scenario than, say, a Jekyll site running locally (or whatever your use-case might be).

Nobody said symbols are always evil. But in a scenario like ours (people send us strings which we will then convert to symbols in our server's memory), they are (used to be) a real problem.

from liquid.

Feels odd that a company would refer to their customers as "attackers", yes they should be referred to in the threat model, but outright calling them an attacker seems wrong, as well, your assessment is outdated.

1. Ruby has frozen string literals to help with memory now
2. Ruby has a symbol GC now to help with the old symbol attack that could happen
   • This has been the case since 2.2.
   • 2.2 is 4 years old.

If you aren't worried about strings, being worried about symbols is odd... So unless you happen to be creating methods for every symbol or string they send in that list of variables... which is highly unoptimized anyways, and also create a ton of symbols (that can't be collected,) and at that point would make this entire debate about symbols negligible... the argument is moot at this point because they will be garbage collected like regular strings will, and if you aren't worried about strings on modern Ruby, you shouldn't be worried about symbols, because when they are no longer used, or useful, they are garbage collected. Unless they are

1. Representing a method
2. Representing a true constant like a class, module, or CONSTANT
3. Used on a regular basis

from liquid.

Feels odd that a company would refer to their customers as "attackers"

I think he just mixed up "e.g." with "i.e."

as well, your assessment is outdated.

You seem to have not read previous comments carefully, because they were talking about the historic reason why we didn't use symbols. My last comment (#82 (comment)) already came to the conclusion that these historic reasons aren't applicable anymore.

from liquid.

I didn't really read all the comments, just came across the last one and decided to reply, so sorry if I replied out of context and exacerbated a debate negatively, that was not my intention. On a side note, I still don't necessarily support symbols because I like the undocumented pretty much private transfer of variables by not supporting symbols.

from liquid.