Attachment: Screenshot visitory log
screenshot-visitors-log.png

(In [6518]) Fixes #3194, make sure smarty escape modifier doesn't double encode escaped text.

please revert as Dangerous to change the escape mechanism used in piwik - i'm pretty sure this would lead to XSS.

in this case there would be another reason for the bug, that the data was tracked incorrectly in the first place - maybe that "In DataTable/Renderer.php formatValueXml calls html_entity_decode/htmlspecialchars with ENT_COMPAT instead of ENT_QUOTES. Is this intentional?"

Replying to matt:

please revert as Dangerous to change the escape mechanism used in piwik - i'm pretty sure this would lead to XSS.

in this case there would be another reason for the bug, that the data was tracked incorrectly in the first place - maybe that "In DataTable/Renderer.php formatValueXml calls html_entity_decode/htmlspecialchars with ENT_COMPAT instead of ENT_QUOTES. Is this intentional?"

This specific bug is caused because the action name has ' in it and the smarty escape modifier will encode it as '. The action name has ' due to getRequestVar sanitizing the action name when tracking.

I suppose, instead of modifying the escape modifier, I could decode the action name in the Live plugin, but other than decoding before escaping, I can't think of a way to solve this issue...

(In [Refs #3194, reverted 6518) smarty escape modifier change.

I suppose, instead of modifying the escape modifier, I could decode the action name in the Live plugin, but other than decoding before escaping, I can't think of a way to solve this issue...

Did you replicate the original issue? I'm wondering if this is due to a tracker bug, or maybe just a browser bug? or were you able to have an example that fails in all browsers?

Actually banym How do the Page names display in the Actions>Pages and Page Titles report? do the report show the names with the html entities?

Thanks for further information!

Replying to matt:

I suppose, instead of modifying the escape modifier, I could decode the action name in the Live plugin, but other than decoding before escaping, I can't think of a way to solve this issue...

Did you replicate the original issue? I'm wondering if this is due to a tracker bug, or maybe just a browser bug? or were you able to have an example that fails in all browsers?

I modified the VisitorGenerator plugin's access log, adding an "'" character to an entry's title. It showed up in the Visitor's log as "'". The HTML returned contained the text "'", so it is not a browser issue.

The action my test created had a name that looked like this: "incredible title''!". So either the bug is with the tracker when it stores action names in their sanitized state, or w/ the admin frontend.

Since decoding first is used in Piwik_Common::sanitizeInputValue, I assumed using it in the smarty escape modifier wouldn't be an issue.

Maybe the bug is in the tracker that should htmldecode before encoding? I thought it would do it already... Maybe that's a bug?

Can you confirm in your test that this page name is displayed correctly in Actions > Page Titles report?

Replying to matt:

Maybe the bug is in the tracker that should htmldecode before encoding? I thought it would do it already... Maybe that's a bug?

Can you confirm in your test that this page name is displayed correctly in Actions > Page Titles report?

The page name is displayed correctly: "incredible title''!"

What a weird bug...

Could you do a special case of decoding the page title before encoding, in the Live plugins templates? what do you think?

Basically, we're double encoding: first in getRequestVar, and then here:

http://dev.piwik.org/trac/changeset/6104/trunk/plugins/Live/templates/visitorLog.tpl

I don't recall the problem fixed by r6104, but could it be changed to:

{$action.pageTitle|unescape|urldecode|escape:'html'|truncate:80:"...":true} 

(In [6631]) Fixes #3194, committed vipsoft's fix: use unescape before escaping action name in visitor log.