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

Fixes #7969 escape goal names and patterns on output in goals controller #8231

Merged
merged 2 commits into from Jul 12, 2015

Conversation

diosmosis
Copy link
Member

As title. Previously, goal names and patterns were escaped in the Goals controller by calling the Common::sanitizeInputValue() method, and then outputted w/ |raw in twig. This PR removes the manual escaping in the Goals controller and removes the |raw filters in related twig files. It also makes sure jqplot evolution graphs escape series and metric names before displaying tooltips.

This fixes #7969, because goal data is used in the JSON jqplot graph data, and since it was escaped in the PHP, it ended up escaped in the JSON. Then jqplot escaped it again.

Fixes #7969

@diosmosis diosmosis added Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jun 26, 2015
@diosmosis diosmosis self-assigned this Jun 26, 2015
@diosmosis diosmosis added this to the 2.14.1 milestone Jun 26, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Jun 26, 2015

Awesome 👍 An update script might be needed to unsanitize existing values in database?

@diosmosis
Copy link
Member Author

Goal names are stored unsanitized. And they were sanitized in PHP, and outputted w/o escaping in twig. Different from every other part of Piwik, and yet, still wrong. Hooray for inconsistency :)

@mnapoli
Copy link
Contributor

mnapoli commented Jun 26, 2015

Haha ok I wasn't expecting that :)

@mattab
Copy link
Member

mattab commented Jul 12, 2015

This is how to kill some technical debt, nice one @diosmosis

mattab pushed a commit that referenced this pull request Jul 12, 2015
Fixes #7969 escape goal names and patterns on output in goals controller
@mattab mattab merged commit e98f405 into master Jul 12, 2015
@mattab mattab deleted the 7969_goals_output_escaping branch July 17, 2015 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants