Request: allow LiquidDrops to accept arguments about liquid HOT 13 CLOSED

As we are not using eval on the provided input the only security breach that I see in this use case is possibility of SQL Injection, should the developer poorly hook the drop into SQL.

I don't know where this feature is going, let alone the reason why it hasn't been implemented. This said, I'll assume it's due to complexity and/or possible SQL Injection attacks. Nonetheless, we can try and prototype it and evaluate the results.

With that in mind: Challenge 1, fragment regexp. Given the following string

products.in_any_category(stuff, "Shoes/Slippers", "Shorts", "Toys", 'hi', '!"#$%&/(){}[]@', [1,2], {:a => 1, :b => 2}).order(:name)

I need to extract the following:

["products", ["in_any_category", stuff,  "Shoes/Slippers", "Shorts", "Toys", 'hi', '!"#$%&/(){}[]@', [1,2], {:a => 1, :b => 2}], ["order", :name]]

Essentially:

[drop_name, [method_name, args_array], [method_name, args_array]]

from liquid.

By the way, notice the caveats introduced by this syntax: when parsing this fragment you will have to figure out which keys are literal values or context variables (i.e., stuff).

Furthermore, it might be hard to parse structures such as Array and Hash without using some sort of eval.

One way I see around this is to use context variables as parameters for drop methods. For example, when invoking ProductDrop.color we would search the context for the value of color within the context of ProductDrop, effectively allowing us to bypass eval of objects. However, this means that the code would have to be invoked as follows:

template = "{% for product in products.color.order %}"
Liquid::Template.parse(template).render('product' => ProductDrop.new, 'ProductDrop["color"]' => :black, 'ProductDrop["order"]' => 'id DESC')

This would pretty much render null my challenge 1, but is an interesting alternative.

from liquid.

I've read your comments several times, but I haven't wrapped my head around the 2nd idea.

The other thing I haven't wrapped my head around properly even after years of exposure is complicated regexp, which is what your 1st comment would need, right?

Considering the regexp can be written, would it be a security risk if context variables (i.e. stuff) are eval'd?

Also, something else to consider if context variables can be passed as an argument: what will happen if it doesn't exist? Error, or just pass it through as nil?

from liquid.

Regarding the first scenario, provided that you use the current liquid infrastructure, namely the Variable and Context classes, given a string (e.g, stuff) you can perform a variable lookup within the parsing context, without eval.

The only caveats of the first approach riside in the parsing complexity and evaling of non context variables.

Consider the following snippet: {{ for product in products.in(['a', 'b']) }}

You can parse until products and in. But how will you pass an array (that has 'a' and 'b' as elements) without eval? The only way I see around this is to limit the parameters to keys of values that already reside in the liquid template parsing context, passed to the parser like this: Liquid::Template.parse('{{ name }}').render('name' => 'ramon')

Unless I'm missing something my parsing challenge is effectively a non issue, as you cannot accept values outside of context without eval. However, placing them in the parsing context has its own challenges, namely in which level of the context stack will you place them? under which key convention?

Something like this might actually be the only option to implement this.

template = "{% for product in products.color.order %}"
Liquid::Template.parse(template).render('product' => ProductDrop.new, 'ProductDrop["color"]' => :black, 'ProductDrop["order"]' => 'id DESC')

But then again, this is just the first draft of my implementation idea. I might be overlooking something, specially because I don't really know liquid like the back of my hand yet ;)

from liquid.

It's also worth noting that something like this should be checked with tobi first. There might be a couple of good reasons why said feature is not supported yet. No point developing it as an outlier, aka no prospects of integration in the mainline.

from liquid.

Yes. To be honest, it's a bit in over my head. I'm thinking of making codes in the mean time. For example, categories can have codes such as "category_#{category.id}" that they can use.

I will just expect that if they're editing the template, then they're more willing to get a bit dirty.

Thus, getting products of certain categores would be like this:
products.in_category_1_or_category_2

from liquid.

@mon, I had the same problem last week similar to yours and had pulled my hair once on that. I then realize also that Drop as is works fine.

Anyway, Drops has access to the @context variable, which I use to get other required parameters, for the template/drop. Not sure if this works for you, but maybe don't let the user configure the drop for you on a template, but have a separate interface for it, a UI/View perhaps, then just pass it on the render() method?

e.g.

class ArticleDrop < Liquid::Drop
def articles
article_ids = @context['article_ids'] # get the article ids from the @context variable
Article.find(:all, :conditions => {:id => article_ids})
end
end

template = Liquid::Template.parse( ' {% for article in article.articles %} {{ article.title }} {%endfor%} ' )
template.render('article' => ArticleDrop.new, 'article_ids' => [69338, 29647])

Btw, if you'll be on railscamp today, meet w/ me and let's talk about liquid LOL

from liquid.

Btw, I know the code above I posted isn't your actual problem/issue. I saw from your original post, you want more flexibility.

But whatever, hopefully that helps :D

Alternatively, another solution I'm thinking of for you is stop using drops already but creating a custom tag/block already

from liquid.

I think it's already possible to do what you want, with a slightly different syntax. In my application I have an ImagesDrop which can be used to locate assets. It's exposed in the template as 'images':

{{ images['figure.gif'] | ...

This is picked up by ImagesDrop#before_method:

class ImagesDrop < Liquid::Drop
  def initialize(controller)
    @controller = controller
  end

  [...]

  def before_method(name)
    ContentManagement::Asset.find(name, nil, @controller.requested_brand_domain)
  end
end

before_method gets called with name = 'figure.gif' and can do what it wants with it.

from liquid.

didn't know that. quite interesting

from liquid.

Yes that would solve my problem at least! Thank you!

from liquid.

To update you guys, I've been able to make an active record-like chaining. It essentially makes use of a bunch of drops, some in charge of a single object, others a collection of objects.

Take this example example: PagesDrop and PageDrop.

I could call something like this (let's say I'm on a single page's body that is rendered as liquid):

{% for child in page.children.published.named_home.all %}

I could call, for example:

from liquid.

It's a work in progress, but this might be useful to you when it is done: https://github.com/ramontayag/liquid_stream

from liquid.