@tomudding opened this Issue on October 11th 2021

Expected Behavior

I am making a JSONp request to the Overlay API with a specific callback value, as follows:

https://demo.matomo.cloud/?module=API&method=Overlay.getTranslations&idSite=1&format=JSON&jsoncallback=__jsonp1

I expect to get back the following:

__jsonp1([{"oneClick":"1 click","clicks":"%s clicks","clicksFromXLinks":"%1$s clicks from one of %2$s links","link":"Link"}])

Current Behavior

What is actually returned:

_jsonp1([{"oneClick":"1 click","clicks":"%s clicks","clicksFromXLinks":"%1$s clicks from one of %2$s links","link":"Link"}]

The first and last character are missing.

Possible Solution

This issue comes from this specific check and call to substr(). If the array is simple (i.e., one-dimensional), the first and last character are removed to create an output like {...} instead of [{...}].
https://github.com/matomo-org/matomo/blob/465ad185f9b49e02d97268ea55a646ce3ee3b1b4/plugins/API/Renderer/Json.php#L67-L75

However, this does not account for the call to renderDataTable() just before that. Which, when using JSONp with a callback will prepend callback(, and append ).

The substr() will strip the first character of the callback and the closing parentheses. This creates invalid JavaScript, which in turn cannot be used.

The easiest way to fix this would be to add an additional check to the if statement: $this->isJsonp():

 // if $array is a simple associative array, remove the JSON root array that is added by renderDataTable 
 if (!empty($array) 
+    && !this->isJsonp() 
     && Piwik::isAssociativeArray($array) 
     && !Piwik::isMultiDimensionalArray($array) 
 ) { 
     $result = substr($result, 1, strlen($result) - 2); 
 } 

 return $result; 

Steps to Reproduce (for Bugs)

  1. Go to https://demo.matomo.cloud/?module=API&method=Overlay.getTranslations&idSite=1&format=JSON&jsoncallback=__jsonp1 or any other API function which returns a simple array.
  2. Observe incorrect output.

Context

Your Environment

  • Matomo Version: 4.4.1
  • PHP Version: 7.4
@tsteur commented on October 11th 2021 Member

Thanks for reporting this @tomudding and providing links to reproduce and thanks for already pointing to where the problem is. This makes things a lot easier. Must have regressed in https://github.com/matomo-org/matomo/commit/66bcb98556e5b92b7b6187a5a904e13a408d0ed6#diff-287d49d80349087bbbaa1186c9f825689026f878c9bad60f46a735b65011acb2R64

@tomudding commented on October 12th 2021

I'd like to point out one thing from my bug report which is actually wrong based on how the code currently functions and what will make properly fixing this a bit more difficult.

I said I expected:

callback([{'simple': true}])

but what I actually expected and should have expected based on the comment in the code (assuming it worked as intended) was:

callback({'simple': true})

Because without a callback I get:

{'simple': true}

This already applies to multidimensional arrays (caught in the first if), they are (as one would expect) like:

// no callback
{'simple': false, 'sub': {'key': 'value'}}
// with callback
callback({'simple': false, 'sub': {'key': 'value'}})

Hence my suggested fix of adding a check for $this->isJsonp() in the second if is actually incorrect.

The rendering of responses is not explained in the documentation, which already caused some confusion. So not stripping the [] may cause more confusion with regards to the other possible outcomes. Especially if this goes undocumented.

This Issue was closed on October 12th 2021
Powered by GitHub Issue Mirror