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

Special characters in website name not shown correctly #7531

Closed
theBear1 opened this issue Mar 25, 2015 · 6 comments
Closed

Special characters in website name not shown correctly #7531

theBear1 opened this issue Mar 25, 2015 · 6 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@theBear1
Copy link

As requested by Matt herewith a bug report:

I have an '& amp ;' and an '& # 039 ;' in my website name. (written with spaces here because otherwise github shows them correctly)
On the Piwik "All websites dashboard" this look just fine as the & and ' symbols.
all websites dashboard

However in the selected website name field on the main dashboard is shows this just as '&' and ''' which looks very strange.
dashboard screenshot
Also in the Piwik Reports email is looks wrong, as well in the description as in the body except for the "Hello,Please find below your weekly report for......" line that one shows correctly
mail screenshot

Don't know if this is a bug or so, I couldn't find anything about this.
I know this issue is there since very early in v2.xx versions.

I run Piwik on my own server.

Hope it will be fix in one of the next versions, or that someone tells me how to fix (.... not by removing those characters).

@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Mar 26, 2015
@mattab mattab added this to the Piwik 2.13.0 milestone Mar 26, 2015
@mattab
Copy link
Member

mattab commented Mar 26, 2015

Thank you @theBear1 for reporting those 2 issues!

@mnapoli mnapoli self-assigned this Mar 26, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Mar 26, 2015

I was expecting this to be simple to fix but that's actually a big mess :/ The source problem is #6714: strings are stored in the database with HTML entities escaped.

Considering that, let's have a look at the site-selector AngularJS component. It takes site names from different sources:

  1. global JS variable (currentSiteName)
  2. angular directive attribute (i.e. HTML attribute)
  3. calling the HTTP API

In 1. and 2. the value is escaped twice (already escaped in DB + re-escaped by Twig). Which means it is escaped when stored in a AngularJS variable.

However on the other hand 3. returns un-escaped values in JSON (correct behavior).

So the AngularJS site selector ends up with possibly escaped or unescaped values. So just to be sure it re-escapes everything - actually not everything but whatever - (with a manual funny custom implementation from stackoverflow) and insert it into HTML by bypassing Angular's escaping system (yeah…). => depending on the value source, it is either correct or over-escaped

The site-selector has this problem but that's not the only place where this happen:

  • email reports
  • custom alerts table
  • custom alerts history
  • … (yet to be found)

That being said what do we do? I'm guessing fixing #6714 is not gonna happen. Fixing 1. and 2. to bypass Twig's escaping could make sense but 1. is used by other JS components so making it non-escaped anymore might break something else… Also that would make 1. and 2. not consistent with the rest of Piwik.

In the end I think we have to fix it by over-escaping what doesn't need to be escaped so that at least all data sources are consistently over-escaped. That means calling the API, get the site names and iterate over all of them to manually escape them and then bypass Angular's escaping system.

I wanted to explain those details because:

  • I am going to fix this bug with a dirty hack so I want to justify why (any other suggestion to fix this is welcome btw)
  • I want to weigh in on Escape/sanitize strings on output rather than on input #6714 and similar issues: bad foundations leak everywhere, refactoring them doesn't bring user value but not fixing them continuously brings bugs to users, and consumes time (will maybe be a total of 3-4 hours on this ticket alone, debugging was long)

I'll get started on a PR!

@mnapoli
Copy link
Contributor

mnapoli commented Mar 27, 2015

PR: #7552

@mnapoli mnapoli changed the title special characters in website name not shown correctly Special characters in website name not shown correctly Mar 27, 2015
@mattab
Copy link
Member

mattab commented Mar 27, 2015

@mnapoli fyi there is an issue about this particular inconsistently as we've been hit few times before... it's #4231
(the tricky thing that we didn't fix that one was that it we would take few risks of introducing XSS in some places where the site name is used without escaping and if we missed those while changing the sanitise...)

@theBear1
Copy link
Author

Just installed v2.13.0

The first issue is now solved, the second I don't know yet.

However on the Piwik "All websites dashboard" it is now wrong !!
all websites dashboard v2 13

Please solve this one too.

Thanks

@mattab
Copy link
Member

mattab commented May 1, 2015

@theBear1 thanks for the report - created #7806

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.
Projects
None yet
Development

No branches or pull requests

3 participants