Input inline syntax does not escape "=" symbol

Bug #1401039 reported by Nikolay Makhotkin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mistral
Fix Released
Medium
Nikolay Makhotkin

Bug Description

Steps to reproduce:

1. Create workflow containing task with following action:

  action: std.http url="http://google.com/search?q={$.query}"

2. Get error of parsing arguments: "List index out of range in 'parse_cmd_and_input' method"

Revision history for this message
Lakshmi Kannan (lakshmi-9) wrote :

I added some debug print statements and figured out that the bug is caused by this regular expression that is used to split the command string in action into key value pairs.

If you look at this, if there is a '=' sign inside a value, then mistral blows up. For example action: std.echo output='lakshmi=test' would break.

('Cmd_str: %s, PARAMS_PTRN: %s', u'std.echo output="Morpheus"', <_sre.SRE_Pattern object at 0x7fad52d743d0>)
('Key: %s, Value: %s', u'output', u'"Morpheus"')
('Cmd_str: %s, PARAMS_PTRN: %s', u'std.echo output="\'http://google.com/query?q={$.query}\'"', <_sre.SRE_Pattern object at 0x7fad52d743d0>)
('Key: %s, Value: %s', u'output', u'')
('Cmd_str: %s, PARAMS_PTRN: %s', u'std.echo output="\'http://google.com/query?q={$.query}\'"', <_sre.SRE_Pattern object at 0x7fad52d743d0>)
('Key: %s, Value: %s', u'q', u'{$.query}')
}}}

I really think regular expression is not the way to parse the string. I would write a simple lexer that splits based on ' ' (space) first and for each of the strings that is not the action name, split based on first '=' that you encounter in the string. I can write some sample code in my free time to show what I am talking about. I just wanted to post my debug logs in case someone else is looking into fixing it.

Revision history for this message
Renat Akhmerov (rakhmerov) wrote :

Yeah, you may be right. I just always feel like reg exps is a good choice when it comes to parsing strings. But in this case it may be simple to solve this task like you described. I think it's worth trying something out and see if it's simpler and less error-prone.

Revision history for this message
Renat Akhmerov (rakhmerov) wrote :

Lakshmi, would you be interested in trying to fix this issue?

One problem I see in the way you suggested is that we can't just split by whitespaces because we can have something like:

action: my_action param1="This is a really good morning" param2={$.task.task1.result}

So you see how many whitespaces are inside this string. The algo must be a little smarter.

Anyway, your input would be appreciated.

Revision history for this message
Lakshmi Kannan (lakshmi-9) wrote :

Renat, let me know if this is the behavior we want. I am using shlex because the action input resembles CLI arguments that you'll pass in command line.

Here's snippet from ipython. If you and Nikolai agree, I can commit this patch.

In [12]: str = 'k1="v1 v2" k2=v3 k3=v4 k4="v4=v45"'

In [13]: parsed = shlex.split(str)

In [14]: print parsed
['k1=v1 v2', 'k2=v3', 'k3=v4', 'k4=v4=v45']

In [15]: for item in parsed:
    print item.split('=', 1)
   ....:
['k1', 'v1 v2']
['k2', 'v3']
['k3', 'v4']
['k4', 'v4=v45']

In [16]:

Revision history for this message
Renat Akhmerov (rakhmerov) wrote :

Looks good. Go ahead and commit it. I think we should have used shlex from the very beginning.

Revision history for this message
Renat Akhmerov (rakhmerov) wrote :

Make sure to add proper test for "=" sign in the middle.

Revision history for this message
Nikolay Makhotkin (nmakhotkin) wrote :

Yes, the solution is nice. Also make sure all prevoius tests are passed.

Changed in mistral:
assignee: nobody → Nikolay Makhotkin (nmakhotkin)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to mistral (master)

Fix proposed to branch: master
Review: https://review.openstack.org/142452

Changed in mistral:
status: New → In Progress
Changed in mistral:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral (master)

Reviewed: https://review.openstack.org/142452
Committed: https://git.openstack.org/cgit/stackforge/mistral/commit/?id=120e6624ef98e28820183bbc716d9e630623cc2a
Submitter: Jenkins
Branch: master

commit 120e6624ef98e28820183bbc716d9e630623cc2a
Author: Nikolay Mahotkin <email address hidden>
Date: Wed Dec 17 13:58:03 2014 +0300

    Fixing parsing inline syntax parameters

    Closes-bug: #1401039

    Change-Id: Iabb96fad6a86934b8ff49c3d3511adb4e87bf651

Changed in mistral:
status: Fix Committed → Fix Released
Changed in mistral:
milestone: kilo-1 → 2015.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.