Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw a better exception if we fail to templatize trigger payload in the rule enforcer #1492

Merged

Conversation

Kami
Copy link
Member

@Kami Kami commented May 18, 2015

Currently, a known limitation is that trigger payload which is matched against in the rules engine can't contain special Jinja2 characters such as "{{", unless you are referencing variable in a datastore inside the payload.

For example, the following would fail:

curl -X POST http://127.0.0.1:9101/v1/webhooks/sample -H "Content-Type: application/json" -d '{"a": "{{ foo.bar }}"}'

(we try to reference attribute "bar" of variable "foo" which is not defined / available inside the template context)

If the payload contains those special characters, they need to be explicitly escaped inside the payload before sending trigger into the system.

Before this change, a really cryptic exception was thrown which made it hard to figure out the root cause. This pull request doesn't solve the root cause (we need to decide how to handle that), but it at least saves user some time debugging what went wrong.

Explicit escaping is obviously not ideal (especially since in a lot of cases, users just want to directly pipe data from 3rd party systems into StackStorm and they don't want to add another step which processes / sanitizes this data), but as long as we support key-value looks in the trigger payload, I can't think of any good solution for that.

Suggestions welcome.

@Kami
Copy link
Member Author

Kami commented May 18, 2015

I encountered this issue while working on st2 ChatOps stuff - in that case a lot of payload contains "{{" character since we are basically piping "raw" data from our database to the rules.

I have a nasty work-around for it right now, but the workaround is anything but ideal, so we need a better, long term solution since it's just a question of time when other users will also encounter this issue.

@lakshmi-kannan
Copy link
Contributor

🐼 :sad:

@lakshmi-kannan
Copy link
Contributor

LGTM. Yep, we need a better solution for this. Move off Jinja perhaps?

Kami added a commit that referenced this pull request May 19, 2015
…le_payload_transforming_failure

Throw a better exception if we fail to templatize trigger payload in the rule enforcer
@Kami Kami merged commit 0503a27 into master May 19, 2015
@Kami Kami deleted the throw_better_exception_on_rule_payload_transforming_failure branch May 19, 2015 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants