TypeErrors in UrlHelper #20189
Comments
djmetzle
added
Potential Bug
|
This issue has been mentioned on Matomo forums. There might be relevant details there: https://forum.matomo.org/t/mediaanalytics-error-message/48642/4 |
|
Thanks for reporting this @djmetzle, could you provide a sample tracking URL so we can be sure that the issue is being recreated properly? |
bx80
added
the
Waiting for user feedback
|
@bx80 matomo/plugins/Referrers/Columns/Base.php Line 379 in b83ed61 Note that urldecode here will only work with strings, not if an array is returned.That led us to UrlHelper: Line 278 in b83ed61 but there appears to to be a bug in the docblock: Line 275 in b83ed61 However, the test cases for this method assert that this function will return the type array if needed! matomo/tests/PHPUnit/Unit/UrlHelperTest.php Lines 117 to 122 in b83ed61 This seems like a bug. This problem is similar to the issues we've faced previous, basically "array found, but string expected". Specifically, we are using the QueuedTracking plugin, so when these "hard-stop" type errors occur, our data ingestion queue stops processing completely, and we notice immediately that we stop receiving new tracking data. Here are our relevant forum posts as well:
Currently, we are applying several patches in order to continue processing queued tracking data. All three seem to address nearly the same issue:
--- Request.php 2022-11-09 13:25:33.000000000 -0700
+++ Patched.php 2022-11-09 13:31:06.000000000 -0700
@@ -640,7 +640,8 @@
// use headers as default if no data was send with the tracking request
$default = Http::getClientHintsFromServerVariables();
- $clientHints = Common::getRequestVar('uadata', $default, 'json', $this->params);
+ $varType = is_string($this->params['uadata']) ? 'json' : null;
+ $clientHints = Common::getRequestVar('uadata', $default, $varType, $this->params);
return is_array($clientHints) ? $clientHints : [];
}
--- UrlHelper.php 2023-01-10 11:38:01.000000000 -0700
+++ Patched.php 2023-01-10 11:37:50.000000000 -0700
@@ -279,7 +279,7 @@
{
$nameToValue = self::getArrayFromQueryString($urlQuery);
- if (isset($nameToValue[$parameter])) {
+ if (isset($nameToValue[$parameter]) && is_string($nameToValue[$parameter])) {
return $nameToValue[$parameter];
}
return null;
--- RequestProcessor.php
+++ Patched.php
@@ -93,7 +93,8 @@
$rawParams = $request->getRawParams();
- parse_str(parse_url($rawParams['url'] ?? '', PHP_URL_QUERY) ?? '', $params);
+ $rawUrl = is_string($rawParams['url']) ? $rawParams['url'] : '';
+ parse_str(parse_url($rawUrl, PHP_URL_QUERY) ?? '', $params);
foreach ($providers as $provider) {
if (empty($provider::CLICK_ID_REQUEST_PARAM)) {please advise. |
sgiehl
removed
the
Waiting for user feedback
|
@djmetzle Thanks for tracing this one down. I guess the utm parameters are actually not supposed to be provided as an array. |
sgiehl
added
Bug
|
@sgiehl if it's helpful, we swept, and all uses of that particular method are expecting only strings: Only the Test for that method asserts arrays get returned. In fact, almost every usage appears to expect that the return docblock annotation is accurate. |
|
@djmetzle I want to avoid introducing any regressions with other plugins that might use the method to receive an array. |
|
Got it. Awesome, thank you @sgiehl |
Opening an issue on advice from the forums:
https://forum.matomo.org/t/mediaanalytics-error-message/48642/2
Expected Behavior
No errors process tracking requests with array form GET parameters.
Current Behavior
We're seeing type errors.
Possible Solution
We are currently using a patch to work around the type error from
utmget param arrays:Note that this probably breaks the tests around
UrlHelper::getParameterFromQueryString:matomo/tests/PHPUnit/Unit/UrlHelperTest.php
Lines 117 to 122 in 8d05108
But those tests enforce contradiction with the docblock return type!
matomo/core/UrlHelper.php
Line 275 in 8d05108
This method should always return a string (not an array) in order to meet the documented interface.
Steps to Reproduce (for Bugs)
Tracking hits that include `utm` GET params in array format (which are not correct, but error out the tracker), such as
Fail to be processed.
Context
These are "bad" tracking hits, in the sense that
utm_campaign[]=...are not technically correct, but we should not be failing hard on processing these hits. Specifically, these errors stall out processing while using the QueuedTracking plugin, and stall all processing of the queue, similar to matomo-org/plugin-QueuedTracking#192Your Environment
The text was updated successfully, but these errors were encountered: