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

Jump to bottom

Handle invalid variable type sent to Period/Range.php #19914

Closed
tsteur opened this issue Oct 26, 2022 · 3 comments · Fixed by #19926
Closed

Handle invalid variable type sent to Period/Range.php #19914

tsteur opened this issue Oct 26, 2022 · 3 comments · Fixed by #19926
Assignees
@sgiehl
Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone
4.12.4

Comments

@tsteur
Copy link
Member

tsteur commented Oct 26, 2022

This was actually supposed to be fixed in #18794 but seems to be still a problem. It is the same URL as in #18794 . Or is it a regression?

Error: {"message":"strpos(): Argument #1 ($haystack) must be of type string, array given","file":"/core/Period/Range.php","line":535,"request_id":"f2aab","backtrace":" on /core/Period/Range.php(535)\n#0 /core/Period/Range.php(535): strpos(Array, '-')\n#1 /plugins/ImageGraph/ImageGraph.php(91): Piwik\Period\Range::getRelativeToEndDate('day', 'last30', Array, Object(Piwik\Site))\n#2 [internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(Array, Array)\n#3 /core/EventDispatcher.php(147): call_user_func_array(Array, Array)\n#4 /core/Piwik.php(871): Piwik\EventDispatcher->postEvent('API.getReportMe...', Array, false, Array)\n#5 /plugins/API/ProcessedReport.php(220): Piwik\Piwik::postEvent('API.getReportMe...', Array)\n#6 /plugins/API/API.php(272): Piwik\Plugins\API\ProcessedReport->getReportMetadata('1', 'day', Array, false, false)\n#7 /core/Plugin/Controller.php(430): Piwik\Plugins\API\API->getReportMetadata('1', 'day', Array)\n#8 /plugins/VisitsSummary/Controller.php(119): Piwik\Plugin\Controller->getLastUnitGraphAcrossPlugins('VisitsSummary', 'getEvolutionGra...', false, Array, 'This is an over...')\n#9 [internal function]: Piwik\Plugins\VisitsSummary\Controller->getEvolutionGraph()\n#10 /core/FrontController.php(631): call_user_func_array(Array, Array)\n#11 /core/FrontController.php(169): Piwik\FrontController->doDispatch('VisitsSummary', 'getEvolutionGra...', Array)\n#12 /core/dispatch.php(32): Piwik\FrontController->dispatch()\n#13 /index.php(25): require_once('/c...')\n#14 {main}","safemode_backtrace":"#0 [internal function]: Piwik\Plugins\Cloud\Controller->safemode(Array)\n#1 /core/FrontController.php(631): call_user_func_array(Array, Array)\n#2 /core/FrontController.php(169): Piwik\FrontController->doDispatch('Cloud', 'safemode', Array)\n#3 /core/FrontController.php(100): Piwik\FrontController->dispatch('CorePluginsAdmi...', 'safemode', Array)\n#4 /core/FrontController.php(140): Piwik\FrontController::(Array)\n#5 /core/FrontController.php(190): Piwik\FrontController::(Object(TypeError))\n#6 /core/dispatch.php(32): Piwik\FrontController->dispatch()\n#7 /index.php(25): require_once('/c...')\n#8 {main}"}

URL: https://demo.matomo.cloud/index.php?action=getEvolutionGraph&date%5B0%5D=exp&date%5B1%5D=%3D1%E9%8E%88%27%22%5C%28&disableLink=0&forceView=1&idSite=1&module=VisitsSummary&period=day&viewDataTable=graphEvolution&widget=1

GET: {"action":"getEvolutionGraph","date":["exp","=1\u9388'"\("],"disableLink":"0","forceView":"1","idSite":"1","module":"VisitsSummary","period":"day","viewDataTable":"graphEvolution","widget":"1"}
@tsteur tsteur added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Oct 26, 2022
@bx80 bx80 added Regression Indicates a feature used to work in a certain way but it no longer does even though it should. Bug For errors / faults / flaws / inconsistencies etc. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Oct 26, 2022
@bx80 bx80 added this to the 4.12.3 milestone Oct 26, 2022
@bx80
Copy link
Contributor

bx80 commented Oct 26, 2022

Looks like a potential regression, I'll assigned it to the current release.

@peterhashair
Copy link
Contributor

peterhashair commented Oct 27, 2022 •
edited

I guess this could be coupe options here.

matomo/core/Period/Range.php

Line 530 in 037cb42

public static function getRelativeToEndDate($period, $lastN, $endDate, $site)

I can think of is add a check here.

 public static function getRelativeToEndDate(string $period, string $lastN, string $endDate, $site)
     if (!is_string($endDate) || !is_string($period) || !is_int($lastN)) {
            throw new \Exception("Invalid Input");
        }

@justinvelluppillai justinvelluppillai modified the milestones: 4.12.3, 4.12.4 Oct 27, 2022
@sgiehl
Copy link
Member

sgiehl commented Oct 28, 2022

@peterhashair For such cases it is always important to reproduce that locally. That way you may actually see the full set of errors/notices that occur.
Using PHP 7.2 the URL in the issue actually results in:

WARNING API[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/plugins/API/ProcessedReport.php(818): Notice - Array to string conversion - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) #0/plugins/API/ProcessedReport.php(818),#1/plugins/API/ProcessedReport.php(178),#2/plugins/API/API.php(272),#3/core/Plugin/Controller.php(430),#4/plugins/VisitsSummary/Controller.php(120),[internal function]: Piwik\Plugins\VisitsSummary\Controller->getEvolutionGraph(),#6/core/FrontController.php(631),#7/core/FrontController.php(169),#8/core/dispatch.php(32),#9/index.php(25)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Period/Range.php(535): Warning - strpos() expects parameter 1 to be string, array given - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) [internal function]: Piwik\ErrorHandler::errorHandler(),#1/core/Period/Range.php(535),#2/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#4/core/EventDispatcher.php(147),#5/core/Piwik.php(871),#6/plugins/API/ProcessedReport.php(220),#7/plugins/API/API.php(272),#8/core/Plugin/Controller.php(430),#9/plugins/VisitsSummary/Controller.php(120)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Date.php(138): Warning - urldecode() expects parameter 1 to be string, array given - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) [internal function]: Piwik\ErrorHandler::errorHandler(),#1/core/Date.php(138),#2/core/Period/Range.php(539),#3/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#5/core/EventDispatcher.php(147),#6/core/Piwik.php(871),#7/plugins/API/ProcessedReport.php(220),#8/plugins/API/API.php(272),#9/core/Plugin/Controller.php(430)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Date.php(140): Warning - urldecode() expects parameter 1 to be string, array given - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) [internal function]: Piwik\ErrorHandler::errorHandler(),#1/core/Date.php(140),#2/core/Period/Range.php(539),#3/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#5/core/EventDispatcher.php(147),#6/core/Piwik.php(871),#7/plugins/API/ProcessedReport.php(220),#8/plugins/API/API.php(272),#9/core/Plugin/Controller.php(430)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Date.php(142): Warning - urldecode() expects parameter 1 to be string, array given - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) [internal function]: Piwik\ErrorHandler::errorHandler(),#1/core/Date.php(142),#2/core/Period/Range.php(539),#3/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#5/core/EventDispatcher.php(147),#6/core/Piwik.php(871),#7/plugins/API/ProcessedReport.php(220),#8/plugins/API/API.php(272),#9/core/Plugin/Controller.php(430)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Date.php(148): Warning - strpos() expects parameter 1 to be string, array given - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) [internal function]: Piwik\ErrorHandler::errorHandler(),#1/core/Date.php(148),#2/core/Period/Range.php(539),#3/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#5/core/EventDispatcher.php(147),#6/core/Piwik.php(871),#7/plugins/API/ProcessedReport.php(220),#8/plugins/API/API.php(272),#9/core/Plugin/Controller.php(430)
WARNING ImageGraph[2022-10-28 07:48:43 UTC] [e1a71] /srv/matomo/core/Date.php(1144): Notice - Array to string conversion - Matomo 4.12.3 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) #0/core/Date.php(1144),#1/core/Date.php(153),#2/core/Period/Range.php(539),#3/plugins/ImageGraph/ImageGraph.php(94),[internal function]: Piwik\Plugins\ImageGraph\ImageGraph->getReportMetadata(),#5/core/EventDispatcher.php(147),#6/core/Piwik.php(871),#7/plugins/API/ProcessedReport.php(220),#8/plugins/API/API.php(272),#9/core/Plugin/Controller.php(430)
ERROR Piwik\ExceptionHandler[2022-10-28 07:48:43 UTC] [e1a71] Uncaught exception: /srv/matomo/core/Date.php(1144): Das Datumsformat muss YYYY-MM-DD, or 'today' or 'yesterday' oder eine andere, von der Funktion strtotime unterstützte Zeichenkette sein. (Besuchen Sie http://php.net/strtotime für nähere Informationen): Array [Query: ?action=getEvolutionGraph&date%5B0%5D=exp&date%5B1%5D=%3D1%E9%8E%88%27%22%5C%28&disableLink=0&forceView=1&idSite=1&module=VisitsSummary&period=day&viewDataTable=graphEvolution&widget=1, CLI mode: 0]

So the fix for #18794 is actually doing what it should - it results in an invalid date exception that is shown to the user. But there are a couple of warnings/notices on they way, that shouldn't be triggered.
Looking through all the stacktraces actually shows, that adjusting getRelativeToEndDate will not fix all of them. The problem is in Controller::getLastUnitGraphAcrossPlugins, as it directly passes through the input date & period.

@sgiehl sgiehl self-assigned this Oct 28, 2022
@sgiehl sgiehl mentioned this issue Oct 28, 2022
Merged
11 tasks
@elabuwa elabuwa changed the title strpos(): Argument #1 ($haystack) must be of type string, array given in Period/Range.php Handle invalid variable type sent to Period/Range.php Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sgiehl sgiehl

Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
4.12.4
Development

Successfully merging a pull request may close this issue.

Fix param handling for Controller::getLastUnitGraphAcrossPlugins matomo-org/matomo
5 participants
@tsteur @justinvelluppillai @sgiehl @peterhashair @bx80