From 8a8b51243641b08064a1b57813da711e96e76298 Mon Sep 17 00:00:00 2001 From: Peter Zhang Date: Tue, 12 Apr 2022 01:10:53 +1200 Subject: [Bug]fix prefilght cors OPTIONS request record in the action visits (#19030) * extend request with options and method when options header and method is options do not record in the database. * update function update function * update tests update tests * update tests adjust code only trigger on option request * remove class variable remove server * Update Request.php add check request method * drop option request drop prefight request * update reset update reset * return 204 on prefight return 204 on prefight * Update Tracker.php accept cors * fix typo & add type hint * Update core/Tracker/RequestSet.php * apply PSR12 code formatting * adds test Co-authored-by: sgiehl --- tests/PHPUnit/Integration/TrackerTest.php | 141 ++++++++++++++++++------------ 1 file changed, 83 insertions(+), 58 deletions(-) (limited to 'tests/PHPUnit/Integration') diff --git a/tests/PHPUnit/Integration/TrackerTest.php b/tests/PHPUnit/Integration/TrackerTest.php index 1869984a5f..8f1f3689cb 100644 --- a/tests/PHPUnit/Integration/TrackerTest.php +++ b/tests/PHPUnit/Integration/TrackerTest.php @@ -1,4 +1,5 @@ tracker = new TestTracker(); - $this->request = $this->buildRequest(array('idsite' => 1)); + $this->request = $this->buildRequest(['idsite' => 1]); $this->iniTimeZone = ini_get('date.timezone'); } @@ -55,11 +56,11 @@ class TrackerTest extends IntegrationTestCase public function tearDown(): void { $this->restoreConfigFile(); - - if($this->tracker) { + + if ($this->tracker) { $this->tracker->disconnectDatabase(); } - + if (array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)) { unset($GLOBALS['PIWIK_TRACKER_DEBUG']); } @@ -69,23 +70,23 @@ class TrackerTest extends IntegrationTestCase parent::tearDown(); } - public function test_isInstalled_shouldReturnTrue_AsPiwikIsInstalled() + public function testIsInstalledShouldReturnTrueAsPiwikIsInstalled() { $this->assertTrue($this->tracker->isInstalled()); } - public function test_shouldRecordStatistics_shouldReturnTrue_IfEnabled_WhichItIsByDefault() + public function testShouldRecordStatisticsShouldReturnTrueIfEnabledWhichItIsByDefault() { $this->assertTrue($this->tracker->shouldRecordStatistics()); } - public function test_shouldRecordStatistics_shouldReturnFalse_IfEnabledButNotInstalled() + public function testShouldRecordStatisticsShouldReturnFalseIfEnabledButNotInstalled() { $this->tracker->setIsNotInstalled(); $this->assertFalse($this->tracker->shouldRecordStatistics()); } - public function test_shouldRecordStatistics_shouldReturnFalse_IfDisabledButInstalled() + public function testShouldRecordStatisticsShouldReturnFalseIfDisabledButInstalled() { $oldConfig = Tracker\TrackerConfig::getConfigValue('record_statistics'); Tracker\TrackerConfig::setConfigValue('record_statistics', 0); @@ -95,7 +96,7 @@ class TrackerTest extends IntegrationTestCase Tracker\TrackerConfig::setConfigValue('record_statistics', $oldConfig); // reset } - public function test_loadTrackerEnvironment_shouldSetGlobalsDebugVar_WhichShouldBeDisabledByDefault() + public function testLoadTrackerEnvironmentShouldSetGlobalsDebugVarWhichShouldBeDisabledByDefault() { $this->assertTrue(!array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)); @@ -104,7 +105,7 @@ class TrackerTest extends IntegrationTestCase $this->assertFalse($GLOBALS['PIWIK_TRACKER_DEBUG']); } - public function test_loadTrackerEnvironment_shouldSetGlobalsDebugVar() + public function testLoadTrackerEnvironmentShouldSetGlobalsDebugVar() { $this->assertTrue(!array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)); @@ -119,7 +120,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($GLOBALS['PIWIK_TRACKER_DEBUG']); } - public function test_loadTrackerEnvironment_shouldEnableTrackerMode() + public function testLoadTrackerEnvironmentShouldEnableTrackerMode() { $this->assertTrue(!array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)); @@ -130,7 +131,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue(SettingsServer::isTrackerApiRequest()); } - public function test_loadTrackerEnvironment_shouldNotThrow_whenConfigNotFound() + public function testLoadTrackerEnvironmentShouldNotThrowWhenConfigNotFound() { $this->assertTrue(!array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)); @@ -145,26 +146,26 @@ class TrackerTest extends IntegrationTestCase Tracker::loadTrackerEnvironment(); $this->assertTrue(SettingsServer::isTrackerApiRequest()); - + //always reset on the test itself $this->restoreConfigFile(); } - public function test_isDatabaseConnected_shouldReturnFalse_IfNotConnected() + public function testIsDatabaseConnectedShouldReturnFalseIfNotConnected() { $this->tracker->disconnectDatabase(); $this->assertFalse($this->tracker->isDatabaseConnected()); } - public function test_getDatabase_shouldReturnDbInstance() + public function testGetDatabaseShouldReturnDbInstance() { $db = $this->tracker->getDatabase(); $this->assertInstanceOf('Piwik\\Tracker\\Db', $db); } - public function test_isDatabaseConnected_shouldReturnTrue_WhenDbIsConnected() + public function testIsDatabaseConnectedShouldReturnTrueWhenDbIsConnected() { $db = $this->tracker->getDatabase(); // make sure connected $this->assertNotEmpty($db); @@ -172,7 +173,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($this->tracker->isDatabaseConnected()); } - public function test_disconnectDatabase_shouldDisconnectDb() + public function testDisconnectDatabaseShouldDisconnectDb() { $this->tracker->getDatabase(); // make sure connected $this->assertTrue($this->tracker->isDatabaseConnected()); @@ -182,19 +183,19 @@ class TrackerTest extends IntegrationTestCase $this->assertFalse($this->tracker->isDatabaseConnected()); } - public function test_trackRequest_shouldNotTrackAnything_IfRequestIsEmpty() + public function testTrackRequestShouldNotTrackAnythingIfRequestIsEmpty() { $called = false; Piwik::addAction('Tracker.makeNewVisitObject', function () use (&$called) { $called = true; }); - $this->tracker->trackRequest(new Request(array())); + $this->tracker->trackRequest(new Request([])); $this->assertFalse($called); } - public function test_trackRequest_shouldTrack_IfRequestIsNotEmpty() + public function testTrackRequestShouldTrackIfRequestIsNotEmpty() { $called = false; Piwik::addAction('Tracker.makeNewVisitObject', function () use (&$called) { @@ -206,7 +207,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($called); } - public function test_trackRequest_shouldIncreaseLoggedRequestsCounter() + public function testTrackRequestShouldIncreaseLoggedRequestsCounter() { $this->tracker->trackRequest($this->request); $this->assertSame(1, $this->tracker->getCountOfLoggedRequests()); @@ -215,9 +216,9 @@ class TrackerTest extends IntegrationTestCase $this->assertSame(2, $this->tracker->getCountOfLoggedRequests()); } - public function test_trackRequest_shouldIncreaseLoggedRequestsCounter_EvenIfRequestIsEmpty() + public function testTrackRequestShouldIncreaseLoggedRequestsCounterEvenIfRequestIsEmpty() { - $request = $this->buildRequest(array()); + $request = $this->buildRequest([]); $this->assertTrue($request->isEmptyRequest()); $this->tracker->trackRequest($request); @@ -227,35 +228,35 @@ class TrackerTest extends IntegrationTestCase $this->assertSame(2, $this->tracker->getCountOfLoggedRequests()); } - public function test_trackRequest_shouldActuallyTrack() + public function testTrackRequestShouldActuallyTrack() { - $request = $this->buildRequest(array('idsite' => 1, 'url' => 'http://www.example.com', 'action_name' => 'test', 'rec' => 1)); + $request = $this->buildRequest(['idsite' => 1, 'url' => 'http://www.example.com', 'action_name' => 'test', 'rec' => 1]); $this->tracker->trackRequest($request); $this->assertActionEquals('test', 1); $this->assertActionEquals('example.com', 2); } - public function test_trackRequest_shouldTrackOutlinkWithFragment() + public function testTrackRequestShouldTrackOutlinkWithFragment() { - $request = $this->buildRequest(array('idsite' => 1, 'link' => 'http://example.com/outlink#fragment-here', 'rec' => 1)); + $request = $this->buildRequest(['idsite' => 1, 'link' => 'http://example.com/outlink#fragment-here', 'rec' => 1]); $this->tracker->trackRequest($request); $this->assertActionEquals('http://example.com/outlink#fragment-here', 1); } - public function test_trackRequest_shouldTrackDownloadWithFragment() + public function testTrackRequestShouldTrackDownloadWithFragment() { - $request = $this->buildRequest(array('idsite' => 1, 'download' => 'http://example.com/file.zip#fragment-here&pk_campaign=Campaign param accepted here', 'rec' => 1)); + $request = $this->buildRequest(['idsite' => 1, 'download' => 'http://example.com/file.zip#fragment-here&pk_campaign=Campaign param accepted here', 'rec' => 1]); $this->tracker->trackRequest($request); $this->assertActionEquals('http://example.com/file.zip#fragment-here&pk_campaign=Campaign param accepted here', 1); } - public function test_main_shouldReturnEmptyPiwikResponse_IfNoRequestsAreGiven() + public function testMainShouldReturnEmptyPiwikResponseIfNoRequestsAreGiven() { $requestSet = $this->getEmptyRequestSet(); - $requestSet->setRequests(array()); + $requestSet->setRequests([]); $response = $this->tracker->main($this->getDefaultHandler(), $requestSet); @@ -263,14 +264,14 @@ class TrackerTest extends IntegrationTestCase $this->assertEquals($expected, $response); } - public function test_main_shouldReturnApiResponse_IfRequestsAreGiven() + public function testMainShouldReturnApiResponseIfRequestsAreGiven() { $response = $this->tracker->main($this->getDefaultHandler(), $this->getRequestSetWithRequests()); Fixture::checkResponse($response); } - public function test_main_shouldReturnNotReturnAnyApiResponse_IfImageIsDisabled() + public function testMainShouldReturnNotReturnAnyApiResponseIfImageIsDisabled() { $_GET['send_image'] = '0'; @@ -281,7 +282,21 @@ class TrackerTest extends IntegrationTestCase $this->assertEquals('', $response); } - public function test_main_shouldActuallyTrackNumberOfTrackedRequests() + public function testMainShouldHandPreflightCorsRequestWithoutTracking() + { + $_SERVER['REQUEST_METHOD'] = 'OPTIONS'; + $_SERVER['HTTP_ACCESS_CONTROL_REQUEST_METHOD'] = 'GET'; + + $handler = $this->getMockBuilder(Handler::class)->getMock(); + $handler->expects($this->never())->method('init'); + + $response = $this->tracker->main($handler, $this->getRequestSetWithRequests()); + $this->assertNull($response); + + $this->assertSame(0, $this->tracker->getCountOfLoggedRequests()); + } + + public function testMainShouldActuallyTrackNumberOfTrackedRequests() { $this->assertSame(0, $this->tracker->getCountOfLoggedRequests()); @@ -290,7 +305,7 @@ class TrackerTest extends IntegrationTestCase $this->assertSame(2, $this->tracker->getCountOfLoggedRequests()); } - public function test_main_shouldNotTrackAnythingButStillReturnApiResponse_IfNotInstalledOrShouldNotRecordStats() + public function testMainShouldNotTrackAnythingButStillReturnApiResponseIfNotInstalledOrShouldNotRecordStats() { $this->tracker->setIsNotInstalled(); $response = $this->tracker->main($this->getDefaultHandler(), $this->getRequestSetWithRequests()); @@ -299,27 +314,29 @@ class TrackerTest extends IntegrationTestCase $this->assertSame(0, $this->tracker->getCountOfLoggedRequests()); } - public function test_main_shouldReadValuesFromGETandPOSTifNoRequestSet() + public function testMainShouldReadValuesFromGETandPOSTifNoRequestSet() { - $_GET = array('idsite' => '1'); - $_POST = array('url' => 'http://localhost/post'); + $_GET = ['idsite' => '1']; + $_POST = ['url' => 'http://localhost/post']; $requestSet = $this->getEmptyRequestSet(); $response = $this->tracker->main($this->getDefaultHandler(), $requestSet); - $_GET = array(); - $_POST = array(); + $_GET = []; + $_POST = []; Fixture::checkResponse($response); $this->assertSame(1, $this->tracker->getCountOfLoggedRequests()); $identifiedRequests = $requestSet->getRequests(); $this->assertCount(1, $identifiedRequests); - $this->assertEquals(array('idsite' => '1', 'url' => 'http://localhost/post'), - $identifiedRequests[0]->getParams()); + $this->assertEquals( + ['idsite' => '1', 'url' => 'http://localhost/post'], + $identifiedRequests[0]->getParams() + ); } - public function test_main_shouldPostEndEvent() + public function testMainShouldPostEndEvent() { $called = false; Piwik::addAction('Tracker.end', function () use (&$called) { @@ -331,7 +348,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($called); } - public function test_main_shouldPostEndEvent_EvenIfShouldNotRecordStats() + public function testMainShouldPostEndEventEvenIfShouldNotRecordStats() { $called = false; Piwik::addAction('Tracker.end', function () use (&$called) { @@ -347,7 +364,7 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($called); } - public function test_main_shouldPostEndEvent_EvenIfThereIsAnException() + public function testMainShouldPostEndEventEvenIfThereIsAnException() { $called = false; Piwik::addAction('Tracker.end', function () use (&$called) { @@ -358,7 +375,7 @@ class TrackerTest extends IntegrationTestCase $handler->enableTriggerExceptionInProcess(); $requestSet = new RequestSet(); - $requestSet->setRequests(array($this->buildRequest(1), $this->buildRequest(1))); + $requestSet->setRequests([$this->buildRequest(1), $this->buildRequest(1)]); $this->tracker->main($handler, $requestSet); @@ -366,21 +383,29 @@ class TrackerTest extends IntegrationTestCase $this->assertTrue($called); } - public function test_archiveInvalidation_differentServerAndWebsiteTimezones() + public function testArchiveInvalidationDifferentServerAndWebsiteTimezones() { // Server timezone is UTC ini_set('date.timezone', 'UTC'); // Website timezone is New York - $idSite = Fixture::createWebsite('2014-01-01 00:00:00', 0, false, false, - 1, null, null, 'America/New_York'); + $idSite = Fixture::createWebsite( + '2014-01-01 00:00:00', + 0, + false, + false, + 1, + null, + null, + 'America/New_York' + ); // It's 3 April in UTC but 2 April in New York Date::$now = 1554257039; $this->tracker = new TestTracker(); - $this->request = $this->buildRequest(array('idsite' => $idSite)); + $this->request = $this->buildRequest(['idsite' => $idSite]); $this->request->setParam('rec', 1); $this->request->setCurrentTimestamp(Date::$now); $this->tracker->trackRequest($this->request); @@ -389,7 +414,7 @@ class TrackerTest extends IntegrationTestCase $this->assertEquals([], Option::getLike('report_to_invalidate_2_2019-04-02%')); } - public function test_TrackingNewVisitOfKnownVisitor() + public function testTrackingNewVisitOfKnownVisitor() { Fixture::createWebsite('2015-01-01 00:00:00'); @@ -421,17 +446,17 @@ class TrackerTest extends IntegrationTestCase private function getRequestSetWithRequests() { $requestSet = $this->getEmptyRequestSet(); - $requestSet->setRequests(array( - $this->buildRequest(array('idsite' => '1', 'url' => 'http://localhost')), - $this->buildRequest(array('idsite' => '1', 'url' => 'http://localhost/test')) - )); + $requestSet->setRequests([ + $this->buildRequest(['idsite' => '1', 'url' => 'http://localhost']), + $this->buildRequest(['idsite' => '1', 'url' => 'http://localhost/test']) + ]); return $requestSet; } private function assertActionEquals($expected, $idaction) { - $actionName = Tracker::getDatabase()->fetchOne("SELECT name FROM " . Common::prefixTable('log_action') . " WHERE idaction = ?", array($idaction)); + $actionName = Tracker::getDatabase()->fetchOne("SELECT name FROM " . Common::prefixTable('log_action') . " WHERE idaction = ?", [$idaction]); $this->assertEquals($expected, $actionName); } @@ -460,10 +485,10 @@ class TrackerTest extends IntegrationTestCase { rename($this->getLocalConfigPath(), $this->getLocalConfigPathMoved()); } - + protected function restoreConfigFile() { - if(file_exists($this->getLocalConfigPathMoved())){ + if (file_exists($this->getLocalConfigPathMoved())) { rename($this->getLocalConfigPathMoved(), $this->getLocalConfigPath()); } } -- cgit v1.2.3