diff options
author | Sam <8619576+samjf@users.noreply.github.com> | 2022-04-11 12:32:52 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-11 12:32:52 +0300 |
commit | e6182097f4a9005de69f34f82c3f76b5c6162473 (patch) | |
tree | 5b3dbf39461c64f81aefad15f544b81ca7946862 | |
parent | f8267dfed779e06f7483d9d952700e8c819cafba (diff) |
Add the request hostname to error & exception logs (#18996)
* Add the hostname to the tracker exception log so multi-tenant domains are easier to debug
* Add hostname to error log to help with debugging
* use Url::getHost and add method to Url class instead
* apply PSR12 code formatting
Co-authored-by: sgiehl <stefan@matomo.org>
-rw-r--r-- | core/ExceptionHandler.php | 32 | ||||
-rw-r--r-- | core/Tracker/Response.php | 14 | ||||
-rw-r--r-- | core/Url.php | 57 | ||||
-rw-r--r-- | core/testMinimumPhpVersion.php | 34 | ||||
-rw-r--r-- | tests/PHPUnit/Unit/UrlTest.php | 350 |
5 files changed, 275 insertions, 212 deletions
diff --git a/core/ExceptionHandler.php b/core/ExceptionHandler.php index 4a61495a85..ca549622b5 100644 --- a/core/ExceptionHandler.php +++ b/core/ExceptionHandler.php @@ -1,4 +1,5 @@ <?php + /** * Matomo - free/libre analytics platform * @@ -6,6 +7,7 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later * */ + namespace Piwik; use DI\DependencyException; @@ -25,7 +27,7 @@ class ExceptionHandler { public static function setUp() { - set_exception_handler(array('Piwik\ExceptionHandler', 'handleException')); + set_exception_handler(['Piwik\ExceptionHandler', 'handleException']); } /** @@ -97,7 +99,7 @@ class ExceptionHandler try { echo self::getErrorResponse($exception); - } catch(Exception $e) { + } catch (Exception $e) { // When there are failures while generating the HTML error response itself, // we simply print out the error message instead. echo $exception->getMessage(); @@ -118,11 +120,9 @@ class ExceptionHandler $isHtmlMessage = method_exists($ex, 'isHtmlMessage') && $ex->isHtmlMessage(); if (!$isHtmlMessage && Request::isApiRequest($_GET)) { - $outputFormat = strtolower(Common::getRequestVar('format', 'xml', 'string', $_GET + $_POST)); $response = new ResponseBuilder($outputFormat); return $response->getResponseException($ex); - } elseif (!$isHtmlMessage) { $message = Common::sanitizeInputValue($message); } @@ -150,8 +150,20 @@ class ExceptionHandler // be written to the application log instead $writeErrorLog = !($ex instanceof \Piwik\Exception\NotSupportedBrowserException); - $result = Piwik_GetErrorMessagePage($message, $debugTrace, true, true, $logoHeaderUrl, - $logoFaviconUrl, null, $writeErrorLog); + $hostname = Url::getRFCValidHostname(); + $hostStr = $hostname ? "[$hostname] " : '- '; + + $result = Piwik_GetErrorMessagePage( + $message, + $debugTrace, + true, + true, + $logoHeaderUrl, + $logoFaviconUrl, + null, + $hostStr, + $writeErrorLog + ); try { /** @@ -163,7 +175,7 @@ class ExceptionHandler * @param string &$result The HTML of the error page. * @param Exception $ex The Exception displayed in the error page. */ - Piwik::postEvent('FrontController.modifyErrorPage', array(&$result, $ex)); + Piwik::postEvent('FrontController.modifyErrorPage', [&$result, $ex]); } catch (ContainerDoesNotExistException $ex) { // this can happen when an error occurs before the Piwik environment is created } @@ -171,17 +183,17 @@ class ExceptionHandler return $result; } - private static function logException($exception, $loglevel=Log::ERROR) + private static function logException($exception, $loglevel = Log::ERROR) { try { switch ($loglevel) { - case(Log::DEBUG): + case (Log::DEBUG): StaticContainer::get(LoggerInterface::class)->debug('Uncaught exception: {exception}', [ 'exception' => $exception, 'ignoreInScreenWriter' => true, ]); break; - case(Log::ERROR): + case (Log::ERROR): default: StaticContainer::get(LoggerInterface::class)->error('Uncaught exception: {exception}', [ 'exception' => $exception, diff --git a/core/Tracker/Response.php b/core/Tracker/Response.php index 9e1a63c846..e99feb8b38 100644 --- a/core/Tracker/Response.php +++ b/core/Tracker/Response.php @@ -1,4 +1,5 @@ <?php + /** * Matomo - free/libre analytics platform * @@ -6,6 +7,7 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later * */ + namespace Piwik\Tracker; use Exception; @@ -15,6 +17,7 @@ use Piwik\Profiler; use Piwik\Timer; use Piwik\Tracker; use Piwik\Tracker\Db as TrackerDb; +use Piwik\Url; class Response { @@ -89,7 +92,8 @@ class Response Common::printDebug("End of the page."); - if ($tracker->isDebugModeEnabled() + if ( + $tracker->isDebugModeEnabled() && $tracker->isDatabaseConnected() && TrackerDb::isProfilingEnabled() ) { @@ -190,7 +194,7 @@ class Response // Base64 image string $img = base64_decode($customImage); $size = getimagesizefromstring($img); - } else if (is_file($customImage) && is_readable($customImage)) { + } elseif (is_file($customImage) && is_readable($customImage)) { // Image file $img = file_get_contents($customImage); $size = getimagesize($customImage); // imagesize is used to get the mime type @@ -198,7 +202,7 @@ class Response // Must have valid image data and a valid mime type to proceed if ($img && $size && isset($size['mime']) && in_array($size['mime'], $supportedMimeTypes)) { - Common::sendHeader('Content-Type: '.$size['mime']); + Common::sendHeader('Content-Type: ' . $size['mime']); echo $img; return true; } @@ -229,6 +233,8 @@ class Response protected function logExceptionToErrorLog($e) { - error_log(sprintf("Error in Matomo (tracker): %s", str_replace("\n", " ", $this->getMessageFromException($e)))); + $hostname = Url::getRFCValidHostname(); + $hostStr = $hostname ? "[$hostname]" : '-'; + error_log(sprintf("$hostStr Error in Matomo (tracker): %s", str_replace("\n", " ", $this->getMessageFromException($e)))); } } diff --git a/core/Url.php b/core/Url.php index c027538cdb..abe1c82006 100644 --- a/core/Url.php +++ b/core/Url.php @@ -1,4 +1,5 @@ <?php + /** * Matomo - free/libre analytics platform * @@ -6,6 +7,7 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later * */ + namespace Piwik; use Exception; @@ -129,7 +131,8 @@ class Url $url = ''; // insert extra path info if proxy_uri_header is set and enabled - if (isset(Config::getInstance()->General['proxy_uri_header']) + if ( + isset(Config::getInstance()->General['proxy_uri_header']) && Config::getInstance()->General['proxy_uri_header'] == 1 && !empty($_SERVER['HTTP_X_FORWARDED_URI']) ) { @@ -209,7 +212,8 @@ class Url public static function isValidHost($host = false): bool { // only do trusted host check if it's enabled - if (isset(Config::getInstance()->General['enable_trusted_host_check']) + if ( + isset(Config::getInstance()->General['enable_trusted_host_check']) && Config::getInstance()->General['enable_trusted_host_check'] == 0 ) { return true; @@ -277,12 +281,13 @@ class Url protected static function saveHostsnameInConfig($host, $domain, $key) { - if (Piwik::hasUserSuperUserAccess() + if ( + Piwik::hasUserSuperUserAccess() && file_exists(Config::getLocalConfigPath()) ) { $config = Config::getInstance()->$domain; if (!is_array($host)) { - $host = array($host); + $host = [$host]; } $host = array_filter($host); if (empty($host)) { @@ -340,6 +345,21 @@ class Url } /** + * Returns the valid hostname (according to RFC standards) as a string; else it will return false if it isn't valid. + * If the hostname isn't supplied it will default to using Url::getHost + * Note: this will not verify if the hostname is trusted. + * @param $hostname + * @return false|string + */ + public static function getRFCValidHostname($hostname = null) + { + if (empty($hostname)) { + $hostname = self::getHost(false); + } + return filter_var($hostname, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME); + } + + /** * Sets the host. Useful for CLI scripts, eg. core:archive command * * @param $host string @@ -363,7 +383,7 @@ class Url */ public static function getCurrentHost($default = 'unknown', $checkTrustedHost = true) { - $hostHeaders = array(); + $hostHeaders = []; $config = Config::getInstance()->General; if (isset($config['proxy_host_headers'])) { @@ -371,7 +391,7 @@ class Url } if (!is_array($hostHeaders)) { - $hostHeaders = array(); + $hostHeaders = []; } $host = self::getHost($checkTrustedHost); @@ -390,7 +410,8 @@ class Url public static function getCurrentQueryString() { $url = ''; - if (isset($_SERVER['QUERY_STRING']) + if ( + isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) { $url .= "?" . $_SERVER['QUERY_STRING']; @@ -491,7 +512,8 @@ class Url private static function redirectToUrlNoExit($url) { - if (UrlHelper::isLookLikeUrl($url) + if ( + UrlHelper::isLookLikeUrl($url) || strpos($url, 'index.php') === 0 ) { Common::sendResponseCode(302); @@ -568,13 +590,13 @@ class Url // handle host name mangling $requestUri = isset($_SERVER['SCRIPT_URI']) ? $_SERVER['SCRIPT_URI'] : ''; $parseRequest = @parse_url($requestUri); - $hosts = array(self::getHost(), self::getCurrentHost()); + $hosts = [self::getHost(), self::getCurrentHost()]; if (!empty($parseRequest['host'])) { $hosts[] = $parseRequest['host']; } // drop port numbers from hostnames and IP addresses - $hosts = array_map(array('self', 'getHostSanitized'), $hosts); + $hosts = array_map(['self', 'getHostSanitized'], $hosts); $disableHostCheck = Config::getInstance()->General['enable_trusted_host_check'] == 0; // compare scheme and host @@ -583,7 +605,7 @@ class Url return !empty($host) && ($disableHostCheck || in_array($host, $hosts)) && !empty($parsedUrl['scheme']) - && in_array($parsedUrl['scheme'], array('http', 'https')); + && in_array($parsedUrl['scheme'], ['http', 'https']); } /** @@ -650,12 +672,12 @@ class Url $config = @Config::getInstance()->$domain; if (!isset($config[$key])) { - return array(); + return []; } $hosts = $config[$key]; if (!is_array($hosts)) { - return array(); + return []; } return $hosts; } @@ -735,7 +757,7 @@ class Url */ public static function getLocalHostnames() { - return array('localhost', '127.0.0.1', '::1', '[::1]', '[::]', '0000::1', '0177.0.0.1', '2130706433', '[0:0:0:0:0:ffff:127.0.0.1]'); + return ['localhost', '127.0.0.1', '::1', '[::1]', '[::]', '0000::1', '0177.0.0.1', '2130706433', '[0:0:0:0:0:ffff:127.0.0.1]']; } /** @@ -769,10 +791,10 @@ class Url return 'http'; } - if ((isset($_SERVER['HTTPS']) && ($_SERVER['HTTPS'] == 'on' || $_SERVER['HTTPS'] === true)) + if ( + (isset($_SERVER['HTTPS']) && ($_SERVER['HTTPS'] == 'on' || $_SERVER['HTTPS'] === true)) || (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https') ) { - return 'https'; } return 'http'; @@ -796,7 +818,8 @@ class Url { $host = @$_SERVER['SERVER_NAME']; if (!empty($host)) { - if (strpos($host, ':') === false + if ( + strpos($host, ':') === false && !empty($_SERVER['SERVER_PORT']) && $_SERVER['SERVER_PORT'] != 80 && $_SERVER['SERVER_PORT'] != 443 diff --git a/core/testMinimumPhpVersion.php b/core/testMinimumPhpVersion.php index 70fa5310bf..efeb530f68 100644 --- a/core/testMinimumPhpVersion.php +++ b/core/testMinimumPhpVersion.php @@ -1,4 +1,5 @@ <?php + /** * Matomo - free/libre analytics platform * @@ -61,12 +62,12 @@ if ($minimumPhpInvalid) { $composerInstall = "Download and run <a href=\"https://getcomposer.org/Composer-Setup.exe\"><b>Composer-Setup.exe</b></a>, it will install the latest Composer version and set up your PATH so that you can just call composer from any directory in your command line. " . " <br>Then run this command in a terminal in the matomo directory: <br> $ php composer.phar install "; } - $piwik_errorMessage .= "<p>It appears the <a href='https://getcomposer.org/' rel='noreferrer noopener' target='_blank'>composer</a> tool is not yet installed. You can install Composer in a few easy steps:\n\n". - "<br/>" . $composerInstall. - " This will initialize composer for Matomo and download libraries we use in vendor/* directory.". + $piwik_errorMessage .= "<p>It appears the <a href='https://getcomposer.org/' rel='noreferrer noopener' target='_blank'>composer</a> tool is not yet installed. You can install Composer in a few easy steps:\n\n" . + "<br/>" . $composerInstall . + " This will initialize composer for Matomo and download libraries we use in vendor/* directory." . "\n\n<br/><br/>Then reload this page to access your analytics reports." . "\n\n<br/><br/>For more information check out this FAQ: <a href='https://matomo.org/faq/how-to-install/faq_18271/' rel='noreferrer noopener' target='_blank'>How do I use Matomo from the Git repository?</a>." . - "\n\n<br/><br/>Note: if for some reasons you cannot install composer, instead install the latest Matomo release from ". + "\n\n<br/><br/>Note: if for some reasons you cannot install composer, instead install the latest Matomo release from " . "<a href='https://builds.matomo.org/piwik.zip' rel='noreferrer noopener'>builds.matomo.org</a>.</p>"; } } @@ -83,7 +84,8 @@ if (!function_exists('Piwik_GetErrorMessagePage')) { */ function Piwik_ShouldPrintBackTraceWithMessage() { - if (class_exists('\Piwik\SettingsServer') + if ( + class_exists('\Piwik\SettingsServer') && class_exists('\Piwik\Common') && \Piwik\SettingsServer::isArchivePhpTriggered() && \Piwik\Common::isPhpCliMode() @@ -109,14 +111,23 @@ if (!function_exists('Piwik_GetErrorMessagePage')) { * @param bool $optionalLinkBack If true, displays a link to go back * @param bool|string $logoUrl The URL to the logo to use. * @param bool|string $faviconUrl The URL to the favicon to use. + * @param string $errorLogPrefix String to prepend to the error in log file * @param bool $writeErrorLog If true then a webserver error log will be written, defaults to true * @return string */ - function Piwik_GetErrorMessagePage($message, $optionalTrace = false, $optionalLinks = false, $optionalLinkBack = false, - $logoUrl = false, $faviconUrl = false, $isCli = null, bool $writeErrorLog = true) - { + function Piwik_GetErrorMessagePage( + $message, + $optionalTrace = false, + $optionalLinks = false, + $optionalLinkBack = false, + $logoUrl = false, + $faviconUrl = false, + $isCli = null, + $errorLogPrefix = '', + bool $writeErrorLog = true + ) { if ($writeErrorLog) { - error_log(sprintf("Error in Matomo: %s", str_replace("\n", " ", strip_tags($message)))); + error_log(sprintf("${errorLogPrefix}Error in Matomo: %s", str_replace("\n", " ", strip_tags($message)))); } if (!headers_sent()) { @@ -130,7 +141,8 @@ if (!function_exists('Piwik_GetErrorMessagePage')) { } // We return only an HTML fragment for AJAX requests - if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) + if ( + isset($_SERVER['HTTP_X_REQUESTED_WITH']) && (strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) === 'xmlhttprequest') ) { return "<div class='alert alert-danger'><strong>Error:</strong> $message</div>"; @@ -182,7 +194,7 @@ if (!function_exists('Piwik_GetErrorMessagePage')) { . ' ' . $optionalLinks; - $message = str_replace(array("<br />", "<br>", "<br/>", "</p>"), "\n", $message); + $message = str_replace(["<br />", "<br>", "<br/>", "</p>"], "\n", $message); $message = str_replace("\t", "", $message); $message = strip_tags($message); diff --git a/tests/PHPUnit/Unit/UrlTest.php b/tests/PHPUnit/Unit/UrlTest.php index 336eff2757..c6a7bc82a4 100644 --- a/tests/PHPUnit/Unit/UrlTest.php +++ b/tests/PHPUnit/Unit/UrlTest.php @@ -1,4 +1,5 @@ <?php + /** * Matomo - free/libre analytics platform * @@ -20,14 +21,14 @@ class UrlTest extends \PHPUnit\Framework\TestCase { public function testAllMethods() { - $this->assertEquals(Url::getCurrentQueryStringWithParametersModified(array()), Url::getCurrentQueryString()); + $this->assertEquals(Url::getCurrentQueryStringWithParametersModified([]), Url::getCurrentQueryString()); $this->assertEquals(Url::getCurrentUrl(), Url::getCurrentUrlWithoutQueryString()); $this->assertEquals(Url::getCurrentUrl(), Url::getCurrentScheme() . '://' . Url::getCurrentHost() . Url::getCurrentScriptName()); $_SERVER['QUERY_STRING'] = 'q=test'; $parameters = array_keys(Url::getArrayFromCurrentQueryString()); - $parametersNameToValue = array(); + $parametersNameToValue = []; foreach ($parameters as $name) { $parametersNameToValue[$name] = null; } @@ -39,25 +40,25 @@ class UrlTest extends \PHPUnit\Framework\TestCase */ public function getCurrentHosts() { - return array( - array('localhost IPv4', array('127.0.0.1', null, null, null, '127.0.0.1')), - array('localhost IPv6', array('[::1]', null, null, null, '[::1]')), - array('localhost name', array('localhost', null, null, null, 'localhost')), - - array('IPv4 without proxy', array('128.1.2.3', null, null, null, '128.1.2.3')), - array('IPv6 without proxy', array('[2001::b0b]', null, null, null, '[2001::b0b]')), - array('name without proxy', array('example.com', null, null, null, 'example.com')), - - array('IPv4 with one proxy', array('127.0.0.1', '128.1.2.3', 'HTTP_X_FORWARDED_HOST', null, '128.1.2.3')), - array('IPv6 with one proxy', array('[::1]', '[2001::b0b]', 'HTTP_X_FORWARDED_HOST', null, '[2001::b0b]')), - array('name with one IPv4 proxy', array('192.168.1.10', 'example.com', 'HTTP_X_FORWARDED_HOST', null, 'example.com')), - array('name with one IPv6 proxy', array('[::10]', 'www.example.com', 'HTTP_X_FORWARDED_HOST', null, 'www.example.com')), - array('name with one named proxy', array('dmz.example.com', 'www.example.com', 'HTTP_X_FORWARDED_HOST', null, 'www.example.com')), - - array('IPv4 with multiple proxies', array('127.0.0.1', '128.1.2.3, 192.168.1.10', 'HTTP_X_FORWARDED_HOST', '192.168.1.*', '128.1.2.3')), - array('IPv6 with multiple proxies', array('[::1]', '[2001::b0b], [::ffff:192.168.1.10]', 'HTTP_X_FORWARDED_HOST', '::ffff:192.168.1.0/124', '[2001::b0b]')), - array('name with multiple proxies', array('dmz.example.com', 'www.example.com, dmz.example.com', 'HTTP_X_FORWARDED_HOST', 'dmz.example.com', 'www.example.com')), - ); + return [ + ['localhost IPv4', ['127.0.0.1', null, null, null, '127.0.0.1']], + ['localhost IPv6', ['[::1]', null, null, null, '[::1]']], + ['localhost name', ['localhost', null, null, null, 'localhost']], + + ['IPv4 without proxy', ['128.1.2.3', null, null, null, '128.1.2.3']], + ['IPv6 without proxy', ['[2001::b0b]', null, null, null, '[2001::b0b]']], + ['name without proxy', ['example.com', null, null, null, 'example.com']], + + ['IPv4 with one proxy', ['127.0.0.1', '128.1.2.3', 'HTTP_X_FORWARDED_HOST', null, '128.1.2.3']], + ['IPv6 with one proxy', ['[::1]', '[2001::b0b]', 'HTTP_X_FORWARDED_HOST', null, '[2001::b0b]']], + ['name with one IPv4 proxy', ['192.168.1.10', 'example.com', 'HTTP_X_FORWARDED_HOST', null, 'example.com']], + ['name with one IPv6 proxy', ['[::10]', 'www.example.com', 'HTTP_X_FORWARDED_HOST', null, 'www.example.com']], + ['name with one named proxy', ['dmz.example.com', 'www.example.com', 'HTTP_X_FORWARDED_HOST', null, 'www.example.com']], + + ['IPv4 with multiple proxies', ['127.0.0.1', '128.1.2.3, 192.168.1.10', 'HTTP_X_FORWARDED_HOST', '192.168.1.*', '128.1.2.3']], + ['IPv6 with multiple proxies', ['[::1]', '[2001::b0b], [::ffff:192.168.1.10]', 'HTTP_X_FORWARDED_HOST', '::ffff:192.168.1.0/124', '[2001::b0b]']], + ['name with multiple proxies', ['dmz.example.com', 'www.example.com, dmz.example.com', 'HTTP_X_FORWARDED_HOST', 'dmz.example.com', 'www.example.com']], + ]; } /** @@ -67,8 +68,8 @@ class UrlTest extends \PHPUnit\Framework\TestCase { Url::setHost($test[0]); $_SERVER['HTTP_X_FORWARDED_HOST'] = $test[1]; - Config::getInstance()->General['proxy_host_headers'] = array($test[2]); - Config::getInstance()->General['proxy_ips'] = array($test[3]); + Config::getInstance()->General['proxy_host_headers'] = [$test[2]]; + Config::getInstance()->General['proxy_ips'] = [$test[3]]; Config::getInstance()->General['enable_trusted_host_check'] = 0; $this->assertEquals($test[4], Url::getCurrentHost(), $description); } @@ -76,7 +77,7 @@ class UrlTest extends \PHPUnit\Framework\TestCase /** * @dataProvider getProtocol */ - public function test_getCurrentScheme_ProtoHeaderShouldPrecedenceHttpsHeader($proto) + public function testGetCurrentSchemeProtoHeaderShouldPrecedenceHttpsHeader($proto) { $_SERVER['HTTPS'] = 'on'; $_SERVER['HTTP_X_FORWARDED_PROTO'] = $proto; @@ -89,7 +90,7 @@ class UrlTest extends \PHPUnit\Framework\TestCase /** * @dataProvider getProtocol */ - public function test_getCurrentScheme_shouldDetectSecureFromHttpsHeader() + public function testGetCurrentSchemeShouldDetectSecureFromHttpsHeader() { $_SERVER['HTTPS'] = 'on'; $this->assertEquals('https', Url::getCurrentScheme()); @@ -100,14 +101,14 @@ class UrlTest extends \PHPUnit\Framework\TestCase /** * @dataProvider getProtocol */ - public function test_getCurrentScheme_shouldBeHttpByDefault() + public function testGetCurrentSchemeShouldBeHttpByDefault() { $this->assertEquals('http', Url::getCurrentScheme()); } public function getProtocol() { - return array(array('http'), array('https')); + return [['http'], ['https']]; } /** @@ -115,86 +116,86 @@ class UrlTest extends \PHPUnit\Framework\TestCase */ public function getLocalUrls() { - return array( + return [ // simple cases - array('www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true), - array('www.example.com', 'http://www.example.com/path/index.php?module=X', '/path/index.php', 'http://www.example.com/path/', true), - array('www.example.com', 'http://www.example.com/path/', '/path/index.php', 'http://www.example.com/path/index.php?module=Y', true), - array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', 'http://www.example.com/path/?query', true), - array('localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true), - array('www.example.com', 'http://www.example.com/path/', '/path/', 'http://www.example.com/path2/', true), + ['www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true], + ['www.example.com', 'http://www.example.com/path/index.php?module=X', '/path/index.php', 'http://www.example.com/path/', true], + ['www.example.com', 'http://www.example.com/path/', '/path/index.php', 'http://www.example.com/path/index.php?module=Y', true], + ['www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', 'http://www.example.com/path/?query', true], + ['localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true], + ['www.example.com', 'http://www.example.com/path/', '/path/', 'http://www.example.com/path2/', true], // ignore port - array('www.example.com', 'http://www.example.com:80/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true), - array('www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com:80/path/index.php', true), + ['www.example.com', 'http://www.example.com:80/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true], + ['www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com:80/path/index.php', true], - array('localhost', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true), - array('localhost', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true), - array('localhost', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true), + ['localhost', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true], + ['localhost', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true], + ['localhost', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true], - array('localhost:8080', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true), - array('localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true), - array('localhost:8080', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true), - array('localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true), + ['localhost:8080', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true], + ['localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true], + ['localhost:8080', 'http://localhost/path/index.php', '/path/index.php', 'http://localhost/path/index.php', true], + ['localhost:8080', 'http://localhost:8080/path/index.php', '/path/index.php', 'http://localhost:8080/path/index.php', true], // IPv6 - array('[::1]', 'http://[::1]/path/index.php', '/path/index.php', 'http://[::1]/path/index.php', true), - array('[::1]:8080', 'http://[::1]:8080/path/index.php', '/path/index.php', 'http://[::1]/path/index.php', true), - array('[::1]:8080', 'http://[::1]/path/index.php', '/path/index.php', 'http://[::1]:8080/path/index.php', true), + ['[::1]', 'http://[::1]/path/index.php', '/path/index.php', 'http://[::1]/path/index.php', true], + ['[::1]:8080', 'http://[::1]:8080/path/index.php', '/path/index.php', 'http://[::1]/path/index.php', true], + ['[::1]:8080', 'http://[::1]/path/index.php', '/path/index.php', 'http://[::1]:8080/path/index.php', true], // undefined SCRIPT URI - array('www.example.com', null, '/path/index.php', 'http://www.example.com/path/index.php', true), - array('localhost:8080', null, '/path/index.php', 'http://localhost:8080/path/index.php', true), - array('127.0.0.1:8080', null, '/path/index.php', 'http://127.0.0.1:8080/path/index.php', true), - array('[::1]', null, '/path/index.php', 'http://[::1]/path/index.php', true), - array('[::1]:8080', null, '/path/index.php', 'http://[::1]:8080/path/index.php', true), + ['www.example.com', null, '/path/index.php', 'http://www.example.com/path/index.php', true], + ['localhost:8080', null, '/path/index.php', 'http://localhost:8080/path/index.php', true], + ['127.0.0.1:8080', null, '/path/index.php', 'http://127.0.0.1:8080/path/index.php', true], + ['[::1]', null, '/path/index.php', 'http://[::1]/path/index.php', true], + ['[::1]:8080', null, '/path/index.php', 'http://[::1]:8080/path/index.php', true], // Apache+Rails anomaly in SCRIPT_URI - array('www.example.com', 'http://www.example.com/path/#anchor', 'http://www.example.com/path/index.php', 'http://www.example.com/path/?query', true), + ['www.example.com', 'http://www.example.com/path/#anchor', 'http://www.example.com/path/index.php', 'http://www.example.com/path/?query', true], // mangled HTTP_HOST - array('www.example.com', 'http://example.com/path/#anchor', '/path/index.php', 'http://example.com/path/referrer', true), + ['www.example.com', 'http://example.com/path/#anchor', '/path/index.php', 'http://example.com/path/referrer', true], // suppressed Referrer - array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', null, true), - array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', '', true), + ['www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', null, true], + ['www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', '', true], // mismatched scheme or host - array('www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'ftp://www.example.com/path/index.php', false), - array('www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'http://example.com/path/index.php', false), - array('www.example.com', 'http://www.example.com/path/', '/path/', 'http://crsf.example.com/path/', false), - ); + ['www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'ftp://www.example.com/path/index.php', false], + ['www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'http://example.com/path/index.php', false], + ['www.example.com', 'http://www.example.com/path/', '/path/', 'http://crsf.example.com/path/', false], + ]; } /** * @dataProvider getIsLocalHost */ - public function test_isLocalHost($expectedIsLocal, $host) + public function testIsLocalHost($expectedIsLocal, $host) { $this->assertSame($expectedIsLocal, Url::isLocalHost($host)); } public function getIsLocalHost() { - return array( - array($isLocal = false, '127.0.0.2'), - array($isLocal = false, '192.168.1.1'), - array($isLocal = false, '10.1.1.1'), - array($isLocal = false, '172.30.1.1'), + return [ + [$isLocal = false, '127.0.0.2'], + [$isLocal = false, '192.168.1.1'], + [$isLocal = false, '10.1.1.1'], + [$isLocal = false, '172.30.1.1'], - array($isLocal = true, 'localhost'), - array($isLocal = true, '127.0.0.1'), - array($isLocal = true, '::1'), - array($isLocal = true, '[::1]'), + [$isLocal = true, 'localhost'], + [$isLocal = true, '127.0.0.1'], + [$isLocal = true, '::1'], + [$isLocal = true, '[::1]'], // with port - array($isLocal = false, '172.30.1.1:80'), - array($isLocal = false, '3ffe:1900:4545:3:200:f8ff:fe21:67cf:1005'), - array($isLocal = true, 'localhost:3000'), - array($isLocal = true, '127.0.0.1:213424'), - array($isLocal = true, '::1:345'), - array($isLocal = true, '[::1]:443'), - ); + [$isLocal = false, '172.30.1.1:80'], + [$isLocal = false, '3ffe:1900:4545:3:200:f8ff:fe21:67cf:1005'], + [$isLocal = true, 'localhost:3000'], + [$isLocal = true, '127.0.0.1:213424'], + [$isLocal = true, '::1:345'], + [$isLocal = true, '[::1]:443'], + ]; } /** @@ -206,7 +207,7 @@ class UrlTest extends \PHPUnit\Framework\TestCase $_SERVER['SCRIPT_URI'] = $scripturi; $_SERVER['REQUEST_URI'] = $requesturi; Config::getInstance()->General['enable_trusted_host_check'] = 1; - Config::getInstance()->General['trusted_hosts'] = array($httphost); + Config::getInstance()->General['trusted_hosts'] = [$httphost]; $urlToTest = $testurl; $this->assertEquals($result, Url::isLocalUrl($urlToTest)); } @@ -216,12 +217,11 @@ class UrlTest extends \PHPUnit\Framework\TestCase */ public function getCurrentUrlWithoutFilename() { - return array( - array('http://example.com/', false, 'example.com', '/'), - array('https://example.org/', true, 'example.org', '/'), - array('https://example.net/piwik/', 'on', 'example.net', '/piwik/'), - ); - + return [ + ['http://example.com/', false, 'example.com', '/'], + ['https://example.org/', true, 'example.org', '/'], + ['https://example.net/piwik/', 'on', 'example.net', '/piwik/'], + ]; } /** @@ -240,26 +240,26 @@ class UrlTest extends \PHPUnit\Framework\TestCase $_SERVER['REQUEST_URI'] = $path; $_SERVER['HTTP_HOST'] = $host; - Config::getInstance()->General['trusted_hosts'] = array($host); + Config::getInstance()->General['trusted_hosts'] = [$host]; $url = Url::getCurrentUrlWithoutFilename(); $this->assertEquals($expected, $url); } - public function test_getCurrentScriptName() + public function testGetCurrentScriptName() { $this->resetGlobalVariables(); - $tests = array( - array('/', 'http://example.com/', null), - array('/', '/', null), - array('/index.php', '/index.php', null), - array('/index.php', '/index.php?module=Foo', null), - array('/index.php', '/index.php/route/1', '/route/1'), - array('/index.php', '/index.php#<img src=http://matomo.org', ''), - array('/index.php', '/index.php/route/2?module=Bar', '/route/2'), - array('/path/index.php', '/path/index.php/route/3/?module=Fu&action=Bar#Hash', '/route/3/'), - ); + $tests = [ + ['/', 'http://example.com/', null], + ['/', '/', null], + ['/index.php', '/index.php', null], + ['/index.php', '/index.php?module=Foo', null], + ['/index.php', '/index.php/route/1', '/route/1'], + ['/index.php', '/index.php#<img src=http://matomo.org', ''], + ['/index.php', '/index.php/route/2?module=Bar', '/route/2'], + ['/path/index.php', '/path/index.php/route/3/?module=Fu&action=Bar#Hash', '/route/3/'], + ]; foreach ($tests as $test) { list($expected, $uri, $pathInfo) = $test; @@ -277,25 +277,25 @@ class UrlTest extends \PHPUnit\Framework\TestCase */ public function getValidHostData() { - return array( + return [ // $expected, $host, $trustedHosts, $description - array(true, 'example.com', array('example.com'), 'Naked domain'), - array(true, 'example.net', array('example.com', 'example.net'), 'Multiple domains'), - array(true, 'piwik.example.com', array('piwik.example.com'), 'Fully qualified domain name'), - array(true, 'piwik.example.com', array('example.com'), 'Valid subdomain'), - array(false, 'example.net', array('example.com'), 'Invalid domain'), - array(false, '.example.com', array('piwik.example.com'), 'Invalid subdomain'), - array(false, 'example-com', array('example.com'), 'Regex should match . literally'), - array(false, 'www.attacker.com?example.com', array('example.com'), 'Spoofed host'), - array(false, 'example.com.attacker.com', array('example.com'), 'Spoofed subdomain'), - array(true, 'example.com.', array('example.com'), 'Trailing . on host is actually valid'), - array(true, 'www-dev.example.com', array('example.com'), 'host with dashes is valid'), - array(false, 'www.example.com:8080', array('example.com'), 'host:port is valid'), - array(true, 'www.example.com:8080', array('example.com:8080'), 'host:port is valid'), - array(false, 'www.whatever.com', array('*.whatever.com'), 'regex char is escaped'), - array(false, 'www.whatever.com', array('www.whatever.com/abc'), 'with path starting with /a does not throw error'), - array(false, 'www.whatever.com', array('www.whatever.com/path/here'), 'with path starting with /p does not throw error'), - ); + [true, 'example.com', ['example.com'], 'Naked domain'], + [true, 'example.net', ['example.com', 'example.net'], 'Multiple domains'], + [true, 'piwik.example.com', ['piwik.example.com'], 'Fully qualified domain name'], + [true, 'piwik.example.com', ['example.com'], 'Valid subdomain'], + [false, 'example.net', ['example.com'], 'Invalid domain'], + [false, '.example.com', ['piwik.example.com'], 'Invalid subdomain'], + [false, 'example-com', ['example.com'], 'Regex should match . literally'], + [false, 'www.attacker.com?example.com', ['example.com'], 'Spoofed host'], + [false, 'example.com.attacker.com', ['example.com'], 'Spoofed subdomain'], + [true, 'example.com.', ['example.com'], 'Trailing . on host is actually valid'], + [true, 'www-dev.example.com', ['example.com'], 'host with dashes is valid'], + [false, 'www.example.com:8080', ['example.com'], 'host:port is valid'], + [true, 'www.example.com:8080', ['example.com:8080'], 'host:port is valid'], + [false, 'www.whatever.com', ['*.whatever.com'], 'regex char is escaped'], + [false, 'www.whatever.com', ['www.whatever.com/abc'], 'with path starting with /a does not throw error'], + [false, 'www.whatever.com', ['www.whatever.com/path/here'], 'with path starting with /p does not throw error'], + ]; } /** @@ -324,16 +324,16 @@ class UrlTest extends \PHPUnit\Framework\TestCase public function getQueryParameters() { - return array( - array(array(), ''), - array(array('v1', 'v2'), '0=v1&1=v2'), - array(array('key' => 'val'), 'key=val'), - array(array('key' => 'val', 'k2' => 'v2'), 'key=val&k2=v2'), - array(array('key' => 'val', 'k2' => false), 'key=val'), // remove false values - array(array('key' => 'val', 'k2' => null), 'key=val'), // remove null values - array(array('key' => 'val', 'k2' => array('v1', 'v2')), 'key=val&k2[]=v1&k2[]=v2'), - array(array('key' => 'val', 'k2' => array('k1' => 'v1', 'k2' => 'v2')), 'key=val&k2[]=v1&k2[]=v2'), - ); + return [ + [[], ''], + [['v1', 'v2'], '0=v1&1=v2'], + [['key' => 'val'], 'key=val'], + [['key' => 'val', 'k2' => 'v2'], 'key=val&k2=v2'], + [['key' => 'val', 'k2' => false], 'key=val'], // remove false values + [['key' => 'val', 'k2' => null], 'key=val'], // remove null values + [['key' => 'val', 'k2' => ['v1', 'v2']], 'key=val&k2[]=v1&k2[]=v2'], + [['key' => 'val', 'k2' => ['k1' => 'v1', 'k2' => 'v2']], 'key=val&k2[]=v1&k2[]=v2'], + ]; } /** @@ -346,15 +346,25 @@ class UrlTest extends \PHPUnit\Framework\TestCase public function getHostsFromUrl() { - return array( - array(null, null), - array('http://', null), - array('http://www.example.com', 'www.example.com'), - array('http://www.ExaMplE.cOm', 'www.example.com'), - array('http://www.example.com/test/foo?bar=xy', 'www.example.com'), - array('http://127.0.0.1', '127.0.0.1'), - array('example.com', null), - ); + return [ + [null, null], + ['http://', null], + ['http://www.example.com', 'www.example.com'], + ['http://www.ExaMplE.cOm', 'www.example.com'], + ['http://www.example.com/test/foo?bar=xy', 'www.example.com'], + ['http://127.0.0.1', '127.0.0.1'], + ['example.com', null], + ]; + } + + public function testGetRFCValidHostname() + { + $_SERVER['HTTP_HOST'] = 'demo.matomo.org'; + $this->assertEquals('demo.matomo.org', Url::getRFCValidHostname()); + unset($_SERVER['HTTP_HOST']); + $this->assertEquals('matomo.org', Url::getRFCValidHostname('matomo.org')); + $this->assertEquals(false, Url::getRFCValidHostname('matomo org')); + $this->assertEquals(false, Url::getRFCValidHostname('matomo.org;<script')); } /** @@ -367,36 +377,36 @@ class UrlTest extends \PHPUnit\Framework\TestCase public function getIsHostInUrls() { - return array( - array(false, null, null), - array(false, 'http://', array()), - array(false, 'example.com', array()), - array(false, 'www.example.com', array()), - array(false, 'example.com', array('www.example.com')), // not a domain so no "host" - array(true, 'example.com', array('example.com')), - array(true, 'eXamPle.com', array('exaMple.com')), - array(true, 'eXamPle.com', array('http://exaMple.com')), - array(true, 'eXamPle.com', array('http://piwik.org', 'http://www.exaMple.com', 'http://exaMple.com')), // multiple urls one or more are valid but not first one - array(true, 'example.com', array('http://example.com/test')), // url with path but correct host - array(true, 'example.com', array('http://www.example.com')), // subdomains are allowed - array(false, 'example.com', array('http://wwwexample.com')), // it should not be possible to create a similar host and make redirects work again. we allow only subdomains - array(true, 'example.com', array('http://ftp.exAmple.com/test')), - array(true, 'example.com', array('http://www.exAmple.com/test')), - array(false, 'ftp.example.com', array('http://www.example.com/test')), - array(true, '127.0.0.1', array()), // always trusted host - ); + return [ + [false, null, null], + [false, 'http://', []], + [false, 'example.com', []], + [false, 'www.example.com', []], + [false, 'example.com', ['www.example.com']], // not a domain so no "host" + [true, 'example.com', ['example.com']], + [true, 'eXamPle.com', ['exaMple.com']], + [true, 'eXamPle.com', ['http://exaMple.com']], + [true, 'eXamPle.com', ['http://piwik.org', 'http://www.exaMple.com', 'http://exaMple.com']], // multiple urls one or more are valid but not first one + [true, 'example.com', ['http://example.com/test']], // url with path but correct host + [true, 'example.com', ['http://www.example.com']], // subdomains are allowed + [false, 'example.com', ['http://wwwexample.com']], // it should not be possible to create a similar host and make redirects work again. we allow only subdomains + [true, 'example.com', ['http://ftp.exAmple.com/test']], + [true, 'example.com', ['http://www.exAmple.com/test']], + [false, 'ftp.example.com', ['http://www.example.com/test']], + [true, '127.0.0.1', []], // always trusted host + ]; } public function urlProvider() { - return array( - array('http://example.com/'), - array('https://example.com/'), - array('http://example.com/piwik/'), - array('http://example.com/index.php'), - array('http://example.com/index.php?foo=bar'), - array('http://example.com/index.php/.html?foo=bar', '/.html'), - ); + return [ + ['http://example.com/'], + ['https://example.com/'], + ['http://example.com/piwik/'], + ['http://example.com/index.php'], + ['http://example.com/index.php?foo=bar'], + ['http://example.com/index.php/.html?foo=bar', '/.html'], + ]; } /** @@ -412,14 +422,14 @@ class UrlTest extends \PHPUnit\Framework\TestCase public function urlWithoutQueryStringProvider() { - return array( - array('http://example.com/', 'http://example.com/'), - array('https://example.com/', 'https://example.com/'), - array('http://example.com/piwik/', 'http://example.com/piwik/'), - array('http://example.com/index.php', 'http://example.com/index.php'), - array('http://example.com/index.php?foo=bar', 'http://example.com/index.php'), - array('http://example.com/index.php/.html?foo=bar', 'http://example.com/index.php/.html', '/.html'), - ); + return [ + ['http://example.com/', 'http://example.com/'], + ['https://example.com/', 'https://example.com/'], + ['http://example.com/piwik/', 'http://example.com/piwik/'], + ['http://example.com/index.php', 'http://example.com/index.php'], + ['http://example.com/index.php?foo=bar', 'http://example.com/index.php'], + ['http://example.com/index.php/.html?foo=bar', 'http://example.com/index.php/.html', '/.html'], + ]; } /** @@ -443,7 +453,7 @@ class UrlTest extends \PHPUnit\Framework\TestCase $this->resetGlobalVariables(); // these variables where taken from a bug report - $_SERVER = array( + $_SERVER = [ 'QUERY_STRING' => 'foo=bar', 'PATH_INFO' => '/test.php', // Nginx passed a wrong value here (should be empty) 'SCRIPT_NAME' => '/test.php', @@ -453,7 +463,7 @@ class UrlTest extends \PHPUnit\Framework\TestCase 'SERVER_NAME' => 'example.com', 'HTTP_HOST' => 'example.com', 'PHP_SELF' => '/test.php/test.php', // Nginx passed a wrong value here (should be /test.php) - ); + ]; $expectedUrl = 'http://example.com/test.php?foo=bar'; @@ -462,8 +472,8 @@ class UrlTest extends \PHPUnit\Framework\TestCase private function resetGlobalVariables() { - $names = array('PATH_INFO', 'REQUEST_URI', 'SCRIPT_NAME', 'SCRIPT_FILENAME', 'argv', 'HTTPS', - 'HTTP_HOST', 'QUERY_STRING', 'HTTP_REFERER'); + $names = ['PATH_INFO', 'REQUEST_URI', 'SCRIPT_NAME', 'SCRIPT_FILENAME', 'argv', 'HTTPS', + 'HTTP_HOST', 'QUERY_STRING', 'HTTP_REFERER']; foreach ($names as $name) { unset($_SERVER[$name]); |