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.

Sign up for GitHub

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

fix(SetupChecks): Validate URL before parsing #49379

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(SetupChecks): Validate URL before parsing #49379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions lib/public/SetupCheck/CheckServerResponseTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,17 @@ private function getRequestOptions(bool $ignoreSSL, bool $httpErrors): array {
* @since 31.0.0
*/
private function normalizeUrl(string $url, bool $removeWebroot): string {
if ($removeWebroot) {
$segments = parse_url($url);
if (!isset($segments['scheme']) || !isset($segments['host'])) {
throw new \InvalidArgumentException('URL is missing scheme or host');
if (filter_var($url, FILTER_VALIDATE_URL)) { // reasonable URL?
if (!$removeWebroot) { // no need to do anything else
return rtrim($url, '/');
} else {
$segments = parse_url($url); // parse the url
if (is_array($segments) && isset($segments['scheme']) && isset($segments['host'])) { // if we have the minimum required
$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;
}
}

$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;
}
return rtrim($url, '/');
throw new \InvalidArgumentException("URL ($url) is invalid - Please verify syntax of all URLs / domains / IP addresses in your config");
}
}
124 changes: 109 additions & 15 deletions tests/lib/SetupCheck/CheckServerResponseTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,42 @@ protected function setUp(): void {
);
}

/**
* @dataProvider dataInvalidNormalizeUrl
*/
public function testInvalidNormalizeUrl(string $url, bool $isRootRequest): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Please verify syntax of all URLs'); // only partial match is required
$this->trait->normalizeUrl($url, $isRootRequest);
}

/**
* @dataProvider dataInvalidNormalizeUrl
*/
public static function dataInvalidNormalizeUrl(): array {
return [
// Web-root left alone
'missing IPv6 brackets' => ['https://ff02::1', false, 'https://ff02::1'],
'missing scheme' => ['nextcloud.com', false, 'nextcloud.com'],
'missing host' => ['https://', false, 'https://'],
'missing host with trailing slash' => ['https:///', false, 'https://'],
'missing host with web-root' => ['https:///root', false, 'https:///root'],
'missing host with web-root with trailing slash' => ['https:///root/', false, 'https:///root'],
// Web-root specified for removal
'missing IPv6 brackets' => ['https://ff02::1', true, 'https://ff02::1'],
'missing scheme' => ['nextcloud.com', true, 'nextcloud.com'],
'missing host' => ['https://', true, 'https://'],
'missing host with trailing slash' => ['https:///', true, 'https://'],
'missing host with web-root' => ['https:///root', true, 'https:///root'],
'missing host with web-root with trailing slash' => ['https:///root/', true, 'https:///root'],

// NOTE: We don't currently catch really bogus URLs here that otherwise follow the basic syntax/format.
// e.g. This will probably pass:
// http://https//example.com
// This is okay here since these are admin specified hostnames/etc
];
}

/**
* @dataProvider dataNormalizeUrl
*/
Expand All @@ -57,21 +93,79 @@ public function testNormalizeUrl(string $url, bool $isRootRequest, string $expec

public static function dataNormalizeUrl(): array {
return [
// untouched web-root
'valid and nothing to change' => ['http://example.com/root', false, 'http://example.com/root'],
'valid with port and nothing to change' => ['http://example.com:8081/root', false, 'http://example.com:8081/root'],
'trailing slash' => ['http://example.com/root/', false, 'http://example.com/root'],
'deep web root' => ['http://example.com/deep/webroot', false, 'http://example.com/deep/webroot'],
// removal of the web-root
'remove web root' => ['http://example.com/root/', true, 'http://example.com'],
'remove web root but empty' => ['http://example.com', true, 'http://example.com'],
'remove deep web root' => ['http://example.com/deep/webroot', true, 'http://example.com'],
'remove web root with port' => ['http://example.com:8081/root', true, 'http://example.com:8081'],
'remove web root with port but empty' => ['http://example.com:8081', true, 'http://example.com:8081'],
'remove web root from IP' => ['https://127.0.0.1/root', true, 'https://127.0.0.1'],
'remove web root from IP with port' => ['https://127.0.0.1:8080/root', true, 'https://127.0.0.1:8080'],
'remove web root from IPv6' => ['https://[ff02::1]/root', true, 'https://[ff02::1]'],
'remove web root from IPv6 with port' => ['https://[ff02::1]:8080/root', true, 'https://[ff02::1]:8080'],
// web-root left alone
// hostname without web-root
'nothing to change' => ['http://example.com', false, 'http://example.com'],
'with trailing slash' => ['http://example.com/', false, 'http://example.com'],
'with port and nothing to change' => ['http://example.com:8081', false, 'http://example.com:8081'],
'with port and trailing slash' => ['http://example.com:8081/', false, 'http://example.com:8081'],
// hostname with web-root
'nothing to change' => ['http://example.com/root', false, 'http://example.com/root'],
'with trailing slash' => ['http://example.com/root/', false, 'http://example.com/root'],
'with port and nothing to change' => ['http://example.com:8081/root', false, 'http://example.com:8081/root'],
'with port and trailing slash' => ['http://example.com:8081/root/', false, 'http://example.com:8081/root'],
// hostname with deep web-root
'nothing to change' => ['http://example.com/deep/webroot', false, 'http://example.com/deep/webroot'],
'with trailing slash' => ['http://example.com/deep/webroot/', false, 'http://example.com/deep/webroot'],
'with port and nothing to change' => ['http://example.com:8081/deep/webroot', false, 'http://example.com/deep/webroot'],
'with port and trailing slash' => ['http://example.com:8081/deep/webroot/', false, 'http://example.com/deep/webroot'],
// IPv4 instead of hostname without web-root
'nothing to change' => ['https://127.0.0.1', false, 'https://127.0.0.1'],
'with trailing slash' => ['https://127.0.0.1/', false, 'https://127.0.0.1'],
'with port and nothing to change' => ['https://127.0.0.1:8080', false, 'https://127.0.0.1:8080'],
'with port and trailing slash' => ['https://127.0.0.1:8080/', false, 'https://127.0.0.1:8080'],
// IPv4 instead of hostname with web-root
'nothing to change' => ['https://127.0.0.1/root', false, 'https://127.0.0.1/root'],
'with trailing slash' => ['https://127.0.0.1/root/', false, 'https://127.0.0.1/root'],
'with port and nothing to change' => ['https://127.0.0.1:8080/root', false, 'https://127.0.0.1:8080/root'],
'with port and trailing slash' => ['https://127.0.0.1:8080/root/', false, 'https://127.0.0.1:8080/root'],
// IPv6 instead of hostname without web-root
'nothing to change' => ['https://[ff02::1]', false, 'https://[ff02::1]'],
'with trailing slash' => ['https://[ff02::1]/', false, 'https://[ff02::1]'],
'with port and nothing to change' => ['https://[ff02::1]:8080', false, 'https://[ff02::1]:8080'],
'with port and trailing slash' => ['https://[ff02::1]:8080/', false, 'https://[ff02::1]:8080'],
// IPv6 instead of hostname with web-root
'nothing to change' => ['https://[ff02::1]/root', false, 'https://[ff02::1]/root'],
'with trailing slash' => ['https://[ff02::1]/root/', false, 'https://[ff02::1]/root'],
'with port and nothing to change' => ['https://[ff02::1]:8080/root', false, 'https://[ff02::1]:8080/root'],
'with port and trailing slash' => ['https://[ff02::1]:8080/root/', false, 'https://[ff02::1]:8080/root'],

// web-root specified for removal
// hostname without web-root
'nothing to change' => ['http://example.com', true, 'http://example.com'],
'with trailing slash' => ['http://example.com/', true, 'http://example.com'],
'with port and nothing to change' => ['http://example.com:8081', true, 'http://example.com:8081'],
'with port and trailing slash' => ['http://example.com:8081/', true, 'http://example.com:8081'],
// hostname with web-root
'without trailing slash' => ['http://example.com/root', true, 'http://example.com'],
'with trailing slash' => ['http://example.com/root/', true, 'http://example.com'],
'with port without trailing slash' => ['http://example.com:8081/root', true, 'http://example.com:8081'],
'with port and trailing slash' => ['http://example.com:8081/root/', true, 'http://example.com:8081'],
// hostname with deep web-root
'without trailing slash' => ['http://example.com/deep/webroot', true, 'http://example.com'],
'with trailing slash' => ['http://example.com/deep/webroot/', true, 'http://example.com'],
'with port without trailing slash' => ['http://example.com:8081/deep/webroot', true, 'http://example.com:8081'],
'with port and trailing slash' => ['http://example.com:8081/deep/webroot/', true, 'http://example.com:8081'],
// IPv4 instead of hostname without web-root
'nothing to change' => ['https://127.0.0.1', true, 'https://127.0.0.1'],
'with trailing slash' => ['https://127.0.0.1/', true, 'https://127.0.0.1'],
'with port and nothing to change' => ['https://127.0.0.1:8080', true, 'https://127.0.0.1:8080'],
'with port and trailing slash' => ['https://127.0.0.1:8080/', true, 'https://127.0.0.1:8080'],
// IPv4 instead of hostname with web-root
'without trailing slash' => ['https://127.0.0.1/root', true, 'https://127.0.0.1'],
'with trailing slash' => ['https://127.0.0.1/root/', true, 'https://127.0.0.1'],
'with port' => ['https://127.0.0.1:8080/root', true, 'https://127.0.0.1:8080/root'],
'with port and trailing slash' => ['https://127.0.0.1:8080/root/', true, 'https://127.0.0.1:8080/root'],
// IPv6 instead of hostname without web-root
'nothing to change' => ['https://[ff02::1]', true, 'https://[ff02::1]'],
'with trailing slash' => ['https://[ff02::1]/', true, 'https://[ff02::1]'],
'with port and nothing to change' => ['https://[ff02::1]:8080', true, 'https://[ff02::1]:8080'],
'with port and trailing slash' => ['https://[ff02::1]:8080/', true, 'https://[ff02::1]:8080'],
// IPv6 instead of hostname with web-root
'without trailing slash' => ['https://[ff02::1]/root', true, 'https://[ff02::1]'],
'with trailing slash' => ['https://[ff02::1]/root/', true, 'https://[ff02::1]'],
'with port' => ['https://[ff02::1]:8080/root', true, 'https://[ff02::1]:8080'],
'with port and trailing slash' => ['https://[ff02::1]:8080/root/', true, 'https://[ff02::1]:8080'],
Comment on lines +96 to +168
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot use the same key several time in an associative array, your are overwriting the previous value each time you use the same key.

];
}

Expand Down
Loading