diff --git a/lib/SetupWizard.php b/lib/SetupWizard.php index 0f4c4f18..7110430d 100644 --- a/lib/SetupWizard.php +++ b/lib/SetupWizard.php @@ -146,23 +146,27 @@ public function isBinaryRunningAtDefaultPort(): bool { return $this->isBinaryRunningAt('http://localhost:7867'); } + public static function toHttps(string $url): string { + // replaces the scheme if there is one, adds it if there is not + return (string)preg_replace('#^(https?://|//)?#i', 'https://', $url, 1); + } + private function getBaseUrl(): string { $base = $this->config->getSystemValueString('overwrite.cli.url', ''); - if (strpos($base, 'https://') !== 0) { - $httpsBase = 'https://' . ltrim($base, 'http://'); - if (isset($this->httpsCache[$httpsBase])) { - return ($this->httpsCache[$httpsBase]) ? $httpsBase : $base; - } + $httpsBase = self::toHttps($base); + if ($base === '' || $httpsBase === $base) { + return $base; + } + + if (!isset($this->httpsCache[$httpsBase])) { try { - $this->client->get($base, ['nextcloud' => ['allow_local_address' => true], 'verify' => false]); + $this->client->get($httpsBase, ['nextcloud' => ['allow_local_address' => true], 'verify' => false]); $this->httpsCache[$httpsBase] = true; - return $base; } catch (\Exception $e) { $this->httpsCache[$httpsBase] = false; - return $base; } } - return $base; + return $this->httpsCache[$httpsBase] ? $httpsBase : $base; } public function isSelfSigned(): bool { diff --git a/tests/lib/SetupWizardBaseUrlTest.php b/tests/lib/SetupWizardBaseUrlTest.php new file mode 100644 index 00000000..194f95c5 --- /dev/null +++ b/tests/lib/SetupWizardBaseUrlTest.php @@ -0,0 +1,96 @@ +requested = []; + + $config = $this->createMock(IConfig::class); + $config->method('getSystemValueString') + ->willReturnCallback(function (string $key, string $default = '') use ($cliUrl) { + return $key === 'overwrite.cli.url' ? $cliUrl : $default; + }); + + $client = $this->createMock(IClient::class); + $client->method('get')->willReturnCallback(function (string $url) use ($httpsReachable) { + $this->requested[] = $url; + if (!$httpsReachable) { + throw new \Exception('connection refused'); + } + return $this->createMock(\OCP\Http\Client\IResponse::class); + }); + + $clientService = $this->createMock(IClientService::class); + $clientService->method('newClient')->willReturn($client); + + return new SetupWizard( + $this->createMock(IQueue::class), + $this->createMock(SelfTest::class), + $clientService, + $config, + $this->createMock(BinaryFinder::class), + ); + } + + public function testProbesTheHttpsUrlNotTheHttpOne(): void { + $wizard = $this->wizard('http://push.example.com', true); + $wizard->getProxiedBase(); + + $this->assertEquals( + ['https://push.example.com'], + $this->requested, + 'the wizard has to probe the https url it built, not the http one it started from' + ); + } + + public function testUsesHttpsWhenReachable(): void { + $wizard = $this->wizard('http://push.example.com', true); + $this->assertEquals('https://push.example.com/push', $wizard->getProxiedBase()); + } + + public function testFallsBackToHttpWhenHttpsIsNotReachable(): void { + $wizard = $this->wizard('http://push.example.com', false); + $this->assertEquals('http://push.example.com/push', $wizard->getProxiedBase()); + } + + public function testAnswersTheSameOnEveryCall(): void { + $wizard = $this->wizard('http://push.example.com', true); + + $first = $wizard->getProxiedBase(); + $second = $wizard->getProxiedBase(); + + $this->assertEquals($first, $second, 'the base url changed between calls'); + $this->assertCount(1, $this->requested, 'the https probe was not cached'); + } + + public function testHttpsUrlIsLeftAlone(): void { + $wizard = $this->wizard('https://push.example.com', true); + $this->assertEquals('https://push.example.com/push', $wizard->getProxiedBase()); + $this->assertCount(0, $this->requested, 'no probe is needed for a url that is already https'); + } + + public function testEmptyUrlIsNotProbed(): void { + $wizard = $this->wizard('', true); + $this->assertEquals('/push', $wizard->getProxiedBase()); + $this->assertCount(0, $this->requested); + } +} diff --git a/tests/lib/SetupWizardTest.php b/tests/lib/SetupWizardTest.php new file mode 100644 index 00000000..ad420ae9 --- /dev/null +++ b/tests/lib/SetupWizardTest.php @@ -0,0 +1,37 @@ +assertEquals($expected, SetupWizard::toHttps($input)); + } +}