@anonymous-matomo-user opened this Issue on April 5th 2012

There is a bug in parsing Custom Variables, basically I have something like this on my site:

piwikTracker.setCustomVariable(
2,
"Entry",
'Random Title - " Foobar "',
"page"
);
which will be json encoded as:

{"2":Title - " Foobar ""}

what will happen is that in Tracker/Visit.php piwik will get this from the "cvar" parameter, then sanitize and unsanitize it, after which it looks like:

{"2":Title - " Foobar ""}

this gets then passed to json_decode which fails to parse it due to the wrongly replaced double quotes.

to reproduce just use the piwikTracker.setCustomVariable call similar to above that has " in it.

@diosmosis commented on May 27th 2012 Member

Attachment: Patch for this issue.
3090.diff.tar.gz

@mattab commented on April 6th 2012 Member

Thanks for the report

@diosmosis commented on May 27th 2012 Member

I uploaded a patch for this issue. Its small, but modifies getRequestVar, so I think it should be reviewed. Let me know what you think.

@mattab commented on May 28th 2012 Member

Thanks for yourpatch!

  • good idea adding json type
  • QA:
I think there should also be one test to check these use cases:
+           $lookingAtProfile = 'looking at "profile page"'; 
+           $lookingAtProfile = '\'looking at "\profile page"\'';
and even maybe with final backslash
+           $lookingAtProfile = '\\looking at "\profile page"\\';
  • Should the following test really be modified? I feel uneasy modifying existing expectations. I'd prefer to see a test added but no modifications, unless there is one necessary.. which we should raise and check that it's safe
-       $this->assertEqual( Piwik_Common::getRequestVar('test', "&<a href='/039'>#039</a>;}{}}{}{}&<a href='/039'>#039</a>;", 'string'), "&<a href='/039'>#039</a>;}{}}{}{}&<a href='/039'>#039</a>;");
+       $this->assertEqual( Piwik_Common::getRequestVar('test', "&<a href='/039'>#039</a>;}{}}&<a href='/0'>#0</a> 39;{}{}&<a href='/039'>#039</a>;", 'string'), "&<a href='/039'>#039</a>;}{}}&amp;<a href='/0'>#0</a> 39;{}{}&<a href='/039'>#039</a>;");
  • There is a json_decode in: /trunk/plugins/CustomVariables/CustomVariables.php
    which expects the custom variable value to be a JSON array. The code should be OK unchanged, but we will know for sure since it is tested via tests/integration/EcommerceOrderWithItems.test.php
@diosmosis commented on May 29th 2012 Member

(In [6367]) Fixes #3090, added 'json' request parameter type & related tests.

@mattab commented on May 29th 2012 Member

(In [6374]) Refs #3090 test build

@mattab commented on May 29th 2012 Member

(In [6377]) Refs #3090 test build

@mattab commented on May 29th 2012 Member

(In [6378]) Refs #3090 Maybe the problem comes from when the PHP implementation of json_decode is used? since it passes on capedbuzz+ my laptop but not on CI...!

@mattab commented on May 30th 2012 Member

(In [6386]) Refs #3090 test using native json_decode rather than fallback as there might be a bug in the php implementation

@mattab commented on May 30th 2012 Member

(In [6388]) Refs #3090 test build

@mattab commented on May 30th 2012 Member

(In [6392]) Do we really need a stripslashes for JSON data? really surprising?? Refs #3090 - could it be a magic quote issue?

@mattab commented on May 30th 2012 Member

(In [6395]) Refs #3090 if this fix works, for sure it will be ugly ;)

@mattab commented on May 30th 2012 Member

(In [6398]) Refs #3090

  • last change bug is fixed.. it was caused by the JSON containing escaping (since magic run time is on)
  • Also fixing "More info" link
This Issue was closed on May 30th 2012
Powered by GitHub Issue Mirror