diff --git a/tests/Unit/UrlTest.php b/tests/Unit/UrlTest.php index db7888de94..da226b0cd4 100644 --- a/tests/Unit/UrlTest.php +++ b/tests/Unit/UrlTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use SMF\Config; +use SMF\IP; use SMF\Url; #[CoversClass(Url::class)] @@ -136,6 +137,146 @@ public function testProxiedLeavesUnroutableHostsAlone(string $url, bool $expecte }); } + #[DataProvider('fetchSafeLiteralProvider')] + public function testIsFetchSafeJudgesLiteralAddresses(string $url, bool $expected): void + { + $this->assertSame($expected, Url::create($url)->isFetchSafe(['http', 'https'])); + } + + #[DataProvider('fetchSafeReservedTldProvider')] + public function testIsFetchSafeRejectsReservedTlds(string $url): void + { + // These never reach the resolver: they are refused on the name alone, + // which is the only reason this case can live in a unit test. + $this->assertFalse(Url::create($url)->isFetchSafe(['http', 'https'])); + } + + #[DataProvider('fetchSafeSchemeProvider')] + public function testIsFetchSafeHonoursTheAllowedSchemes(string $url, array $schemes, bool $expected): void + { + $this->assertSame($expected, Url::create($url)->isFetchSafe($schemes)); + } + + public function testIsFetchSafeWithNoSchemesAllowsAnythingWeCanFetch(): void + { + $this->assertTrue(Url::create('http://93.184.216.34/x')->isFetchSafe()); + $this->assertTrue(Url::create('ftp://93.184.216.34/x')->isFetchSafe()); + $this->assertFalse(Url::create('javascript:alert(1)')->isFetchSafe()); + } + + public function testIsFetchSafeLeavesTheUrlAlone(): void + { + // The whole point of the exercise. When the host was swapped for a + // literal address, the request went out with the wrong SNI name, the + // wrong Host header, and a certificate that could not match. + $url = new Url('https://93.184.216.34:8443/a/b?c=d#e'); + + $url->isFetchSafe(['http', 'https']); + + $this->assertSame('https://93.184.216.34:8443/a/b?c=d#e', (string) $url); + $this->assertSame('93.184.216.34', $url->host); + } + + public function testIsFetchSafeLeavesAnInternationalisedHostInUtf8(): void + { + $url = new Url('https://münchen.smf-unit-tests/straße'); + + $this->withResolvedHosts(['xn--mnchen-3ya.smf-unit-tests' => ['93.184.216.34']], function () use ($url): void { + $this->assertTrue($url->isFetchSafe(['http', 'https'])); + }); + + $this->assertSame('https://münchen.smf-unit-tests/straße', (string) $url); + } + + #[DataProvider('fetchSafeResolutionProvider')] + public function testIsFetchSafeJudgesWhatTheHostResolvesTo(array $ips, bool $expected): void + { + $this->withResolvedHosts(['resolved.smf-unit-tests' => $ips], function () use ($expected): void { + $this->assertSame($expected, Url::create('http://resolved.smf-unit-tests/x')->isFetchSafe(['http', 'https'])); + }); + } + + public function testResolvesToMatchesAnyOfTheKnownAddresses(): void + { + $this->withResolvedHosts(['resolved.smf-unit-tests' => ['93.184.216.34', '93.184.216.35']], function (): void { + $url = Url::create('http://resolved.smf-unit-tests/x'); + + $this->assertTrue($url->resolvesTo(new IP('93.184.216.34'))); + $this->assertTrue($url->resolvesTo(new IP('93.184.216.35'))); + $this->assertFalse($url->resolvesTo(new IP('93.184.216.36'))); + }); + } + + public function testResolvesToComparesAddressesRatherThanTheirWrittenForm(): void + { + // SMF\IP puts v6 addresses through inet_ntop(inet_pton()), so the + // expanded and the compressed spelling are the same address by the + // time we compare them. + $url = new Url('http://[2606:4700:4700::1111]/x'); + + $this->assertTrue($url->resolvesTo(new IP('2606:4700:4700:0000:0000:0000:0000:1111'))); + $this->assertFalse($url->resolvesTo(new IP('2606:4700:4700::1112'))); + } + + public function testGetIPsSurvivesTheRoundTripThroughAscii(): void + { + // getIPs() punycodes the host, looks that up, and then puts the URL + // back into UTF-8 before returning. Both conversions re-parse the URL, + // so the host is spelled one way going in and another coming out, and + // reaching for the answer by host name afterwards reaches for a key + // that was never written. + $url = new Url('https://münchen.smf-unit-tests/x'); + + $this->withResolvedHosts(['xn--mnchen-3ya.smf-unit-tests' => ['93.184.216.34']], function () use ($url): void { + $ips = $url->getIPs(); + + $this->assertCount(1, $ips); + $this->assertSame('93.184.216.34', (string) $ips[0]); + }); + + // And the URL is still spelled the way we wrote it. + $this->assertSame('https://münchen.smf-unit-tests/x', (string) $url); + $this->assertSame('münchen.smf-unit-tests', $url->host); + } + + public function testGetIPsReturnsAnArrayForAnInternationalisedHostThatResolvesToNothing(): void + { + $url = new Url('https://münchen.smf-unit-tests/x'); + + $this->withResolvedHosts(['xn--mnchen-3ya.smf-unit-tests' => []], function () use ($url): void { + $this->assertSame([], $url->getIPs()); + }); + } + + public function testResolvesToWorksOnAnInternationalisedHost(): void + { + // get_ips_for_url() and url_resolves_to() reach getIPs() and + // resolvesTo() without converting the URL first, so this is the shape + // the compatibility layer hands them. + $url = new Url('https://münchen.smf-unit-tests/x'); + + $this->withResolvedHosts(['xn--mnchen-3ya.smf-unit-tests' => ['93.184.216.34']], function () use ($url): void { + $this->assertTrue($url->resolvesTo(new IP('93.184.216.34'))); + $this->assertFalse($url->resolvesTo(new IP('10.0.0.1'))); + }); + } + + public function testProxiedDoesNotConsultTheResolver(): void + { + // proxied() runs for every image in every post, and only decides whether + // to route through proxy.php, which does its own checking. An empty + // answer here would read as "unsafe" to isFetchSafe(), so if this comes + // back proxied then proxied() never asked. + $this->withResolvedHosts(['images.smf-unit-tests' => []], function (): void { + $this->withProxySettings(function (): void { + $this->assertStringStartsWith( + 'https://forum.test-site.com/forum/proxy.php?request=', + (string) Url::create('http://images.smf-unit-tests/pic.png')->proxied(), + ); + }); + }); + } + /*********************** * Public static methods ***********************/ @@ -180,6 +321,79 @@ public static function proxiedProvider(): array ]; } + /** + * Addresses written out in the URL, so no lookup happens at all. + * + * @return array + */ + public static function fetchSafeLiteralProvider(): array + { + return [ + 'ipv4 loopback' => ['http://127.0.0.1/x', false], + 'ipv4 private' => ['http://10.0.0.1/x', false], + 'ipv4 private 192' => ['http://192.168.1.1/x', false], + 'ipv4 link local' => ['http://169.254.169.254/x', false], + 'ipv4 global' => ['http://93.184.216.34/x', true], + 'ipv6 loopback' => ['http://[::1]/x', false], + 'ipv6 unique local' => ['http://[fd00::1]/x', false], + 'ipv6 documentation' => ['http://[2001:db8::1]/x', false], + 'ipv6 global' => ['http://[2606:4700:4700::1111]/x', true], + ]; + } + + /** + * Names that are never in public DNS, per RFC 2606 and RFC 6761. + * + * @return array + */ + public static function fetchSafeReservedTldProvider(): array + { + return [ + 'localhost' => ['http://localhost/x'], + 'local' => ['http://box.local/x'], + 'internal' => ['http://box.internal/x'], + 'test' => ['http://box.test/x'], + 'invalid' => ['http://box.invalid/x'], + 'example' => ['http://box.example/x'], + 'onion' => ['http://box.onion/x'], + ]; + } + + /** + * @return array, bool}> + */ + public static function fetchSafeSchemeProvider(): array + { + return [ + 'http when http is allowed' => ['http://93.184.216.34/x', ['http', 'https'], true], + 'ftp when only http is allowed' => ['ftp://93.184.216.34/x', ['http', 'https'], false], + 'ftp when ftp is allowed' => ['ftp://93.184.216.34/x', ['ftp', 'ftps'], true], + 'no scheme' => ['//93.184.216.34/x', ['http', 'https'], false], + 'no host' => ['mailto:someone@example.com', ['http', 'https'], false], + ]; + } + + /** + * What a name resolves to decides the answer. Every address has to be + * globally routable, because we will not know which one gets used. + * + * @return array, bool}> + */ + public static function fetchSafeResolutionProvider(): array + { + return [ + 'all global' => [['93.184.216.34', '93.184.216.35'], true], + 'all private' => [['10.0.0.1'], false], + 'one private among global' => [['93.184.216.34', '127.0.0.1'], false], + 'global v6' => [['2606:4700:4700::1111'], true], + 'link local v6' => [['fe80::1'], false], + // Nothing came back. dns_get_record() never sees names that only + // exist in the system's hosts file, so this is not proof that the + // name is harmless. + 'nothing at all' => [[], false], + ]; + } + /****************** * Internal methods ******************/ @@ -195,6 +409,38 @@ public static function proxiedProvider(): array * * @param callable $test The assertions to run. */ + /** + * Runs $test with the given hosts already resolved, then puts the cache + * back as it was. + * + * Url::getIPs() remembers what a host resolved to for the rest of the + * request. Seeding that cache is what lets a unit test say what a name + * resolves to without a resolver, without a network, and without a result + * that changes depending on where the suite is run. + * + * @param array> $hosts Host name to addresses. + * @param callable $test The assertions to run. + */ + protected function withResolvedHosts(array $hosts, callable $test): void + { + $property = new \ReflectionProperty(Url::class, 'ips'); + + // Typed, and declared with no default, so it starts out uninitialised + // and reading it before anything has resolved would throw. + $previous = $property->isInitialized() ? $property->getValue() : []; + + $property->setValue(null, array_map( + fn(array $ips): array => array_map(fn(string $ip): IP => new IP($ip), $ips), + $hosts, + )); + + try { + $test(); + } finally { + $property->setValue(null, $previous); + } + } + protected function withProxySettings(callable $test): void { $enabled = Config::$image_proxy_enabled ?? false; diff --git a/tests/Unit/WebFetchResultTest.php b/tests/Unit/WebFetchResultTest.php new file mode 100644 index 0000000000..d9670488e0 --- /dev/null +++ b/tests/Unit/WebFetchResultTest.php @@ -0,0 +1,55 @@ +assertNull($fetcher->result('success')); + $this->assertNull($fetcher->result('body')); + $this->assertNull($fetcher->result()); + } + + /*********************** + * Public static methods + ***********************/ + + /** + * @return array + */ + public static function fetcherProvider(): array + { + return [ + 'curl' => [CurlFetcher::class], + 'socket' => [SocketFetcher::class], + ]; + } +}