diff --git a/phpBB/phpbb/file_downloader.php b/phpBB/phpbb/file_downloader.php index 873bccc9e8..bf466c94bc 100644 --- a/phpBB/phpbb/file_downloader.php +++ b/phpBB/phpbb/file_downloader.php @@ -13,91 +13,117 @@ namespace phpbb; +use GuzzleHttp\Client; +use GuzzleHttp\Exception\RequestException ; +use phpbb\exception\runtime_exception; +use function Amp\Promise\rethrow; + class file_downloader { + const OK = 200; + const NOT_FOUND = 404; + const REQUEST_TIMEOUT = 408; + /** @var string Error string */ - protected $error_string = ''; + protected string $error_string = ''; /** @var int Error number */ - protected $error_number = 0; + protected int $error_number = 0; + + /** + * Create new guzzle client + * + * @param string $host + * @param int $port + * @param int $timeout + * + * @return Client + */ + protected function create_client(string $host, int $port = 443, int $timeout = 6): Client + { + // Only set URL scheme if not specified in URL + $url_parts = parse_url($host); + if (!isset($url_parts['scheme'])) + { + $host = (($port === 443) ? 'https://' : 'http://') . $host; + } + + // Initialize Guzzle client + return new Client([ + 'base_uri' => $host, + 'timeout' => $timeout, + ]); + } /** * Retrieve contents from remotely stored file * - * @param string $host File host - * @param string $directory Directory file is in - * @param string $filename Filename of file to retrieve - * @param int $port Port to connect to; default: 80 - * @param int $timeout Connection timeout in seconds; default: 6 + * @param string $host File host + * @param string $directory Directory file is in + * @param string $filename Filename of file to retrieve + * @param int $port Port to connect to; default: 80 + * @param int $timeout Connection timeout in seconds; default: 6 * * @return false|string File data as string if file can be read and there is no - * timeout, false if there were errors or the connection timed out + * timeout, false if there were errors or the connection timed out * - * @throws \phpbb\exception\runtime_exception If data can't be retrieved and no error - * message is returned + * @throws runtime_exception If data can't be retrieved and no error + * message is returned */ - public function get($host, $directory, $filename, $port = 80, $timeout = 6) + public function get(string $host, string $directory, string $filename, int $port = 443, int $timeout = 6): bool|string { + // Initialize Guzzle client + $client = $this->create_client($host, $port, $timeout); + // Set default values for error variables $this->error_number = 0; $this->error_string = ''; - if (function_exists('fsockopen') && - $socket = @fsockopen(($port == 443 ? 'ssl://' : '') . $host, $port, $this->error_number, $this->error_string, $timeout) - ) + try { - @fputs($socket, "GET $directory/$filename HTTP/1.0\r\n"); - @fputs($socket, "HOST: $host\r\n"); - @fputs($socket, "Connection: close\r\n\r\n"); + $response = $client->request('GET', "$directory/$filename"); - $timer_stop = time() + $timeout; - stream_set_timeout($socket, $timeout); - - $file_info = ''; - $get_info = false; - - while (!@feof($socket)) + // Check if the response status code is 200 (OK) + if ($response->getStatusCode() == self::OK) { - if ($get_info) - { - $file_info .= @fread($socket, 1024); - } - else - { - $line = @fgets($socket, 1024); - if ($line == "\r\n") - { - $get_info = true; - } - else if (stripos($line, '404 not found') !== false) - { - throw new \phpbb\exception\runtime_exception('FILE_NOT_FOUND', array($filename)); - } - } - - $stream_meta_data = stream_get_meta_data($socket); - - if ($stream_meta_data['timed_out'] || time() >= $timer_stop) - { - throw new \phpbb\exception\runtime_exception('FSOCK_TIMEOUT'); - } - } - @fclose($socket); - } - else - { - if ($this->error_string) - { - $this->error_string = utf8_convert_message($this->error_string); - return false; + return $response->getBody()->getContents(); } else { - throw new \phpbb\exception\runtime_exception('FSOCK_DISABLED'); + $this->error_number = $response->getStatusCode(); + throw new runtime_exception('FILE_NOT_FOUND', [$filename]); } } + catch (RequestException $exception) + { + if ($exception->hasResponse()) + { + $this->error_number = $exception->getResponse()->getStatusCode(); - return $file_info; + if ($this->error_number == self::NOT_FOUND) + { + throw new runtime_exception('FILE_NOT_FOUND', [$filename]); + } + } + else + { + $this->error_number = self::REQUEST_TIMEOUT; + throw new runtime_exception('FSOCK_TIMEOUT'); + } + + $this->error_string = utf8_convert_message($exception->getMessage()); + return false; + } + catch (runtime_exception $exception) + { + // Rethrow runtime_exceptions, only handle unknown cases below + throw $exception; + } + catch (\Throwable $exception) + { + $this->error_string = utf8_convert_message($exception->getMessage()); + throw new runtime_exception('FSOCK_DISABLED'); + } } /** @@ -105,7 +131,7 @@ class file_downloader * * @return string Error string */ - public function get_error_string() + public function get_error_string(): string { return $this->error_string; } @@ -115,7 +141,7 @@ class file_downloader * * @return int Error number */ - public function get_error_number() + public function get_error_number(): int { return $this->error_number; } diff --git a/tests/version/version_helper_remote_test.php b/tests/version/version_helper_remote_test.php index 02a811b539..be92a3f977 100644 --- a/tests/version/version_helper_remote_test.php +++ b/tests/version/version_helper_remote_test.php @@ -1,4 +1,7 @@ method('get') ->with($this->anything()) ->will($this->returnValue(false)); - $this->file_downloader = new phpbb_mock_file_downloader(); + + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->addMethods(['set_data']) + ->onlyMethods(['request']) + ->getMock(); + $this->guzzle_mock->method('set_data') + ->will($this->returnCallback(function($data) + { + $this->guzzle_data = $data; + } + )); + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function() + { + return new \GuzzleHttp\Psr7\Response($this->guzzle_status, [], $this->guzzle_data); + } + )); + + $this->file_downloader = $this->getMockBuilder('\phpbb\file_downloader') + ->onlyMethods(['create_client']) + ->getMock(); + $this->file_downloader->method('create_client') + ->will($this->returnValue($this->guzzle_mock)); $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); @@ -203,7 +233,7 @@ class version_helper_remote_test extends \phpbb_test_case */ public function test_get_versions($input, $valid_data, $expected_return = '', $expected_exception = '') { - $this->file_downloader->set($input); + $this->guzzle_mock->set_data($input); // version_helper->get_versions() doesn't return a value on VERSIONCHECK_FAIL but only throws exception // so the $return is undefined. Define it here @@ -214,7 +244,7 @@ class version_helper_remote_test extends \phpbb_test_case try { $return = $this->version_helper->get_versions(); } catch (\phpbb\exception\runtime_exception $e) { - $this->assertEquals((string)$e->getMessage(), $expected_exception); + $this->assertEquals($expected_exception, $e->getMessage()); } } else @@ -227,7 +257,23 @@ class version_helper_remote_test extends \phpbb_test_case public function test_version_phpbb_com() { - $file_downloader = new \phpbb\file_downloader(); + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function() + { + return new \GuzzleHttp\Psr7\Response(200, [], file_get_contents(__DIR__ . '/fixture/30x.txt')); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); $hostname = 'version.phpbb.com'; @@ -285,4 +331,138 @@ class version_helper_remote_test extends \phpbb_test_case $this->assertStringContainsString('http', $lines[1]); $this->assertStringContainsString('phpbb.com', $lines[1], '', true); } + + public function test_file_downloader_file_not_found() + { + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function() + { + return new \GuzzleHttp\Psr7\Response(404, [], ''); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); + + $this->expectException(\phpbb\exception\runtime_exception::class); + $this->expectExceptionMessage('FILE_NOT_FOUND'); + + $file_downloader->get('foo.com', 'bar', 'foo.txt'); + } + + public function test_file_downloader_exception_not_found() + { + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function($method, $uri) + { + $request = new \GuzzleHttp\Psr7\Request('GET', $uri); + $response = new \GuzzleHttp\Psr7\Response(404, [], ''); + throw new RequestException('FILE_NOT_FOUND', $request, $response); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); + + $this->expectException(\phpbb\exception\runtime_exception::class); + $this->expectExceptionMessage('FILE_NOT_FOUND'); + + $file_downloader->get('foo.com', 'bar', 'foo.txt'); + } + + public function test_file_downloader_exception_moved() + { + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function($method, $uri) + { + $request = new \GuzzleHttp\Psr7\Request('GET', $uri); + $response = new \GuzzleHttp\Psr7\Response(302, [], ''); + throw new RequestException('FILE_MOVED', $request, $response); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); + + $this->assertFalse($file_downloader->get('foo.com', 'bar', 'foo.txt')); + $this->assertEquals(302, $file_downloader->get_error_number()); + $this->assertEquals('FILE_MOVED', $file_downloader->get_error_string()); + } + + public function test_file_downloader_exception_timeout() + { + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function($method, $uri) + { + $request = new \GuzzleHttp\Psr7\Request('GET', $uri); + throw new RequestException('FILE_NOT_FOUND', $request); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); + + $this->expectException(\phpbb\exception\runtime_exception::class); + $this->expectExceptionMessage('FSOCK_TIMEOUT'); + + $file_downloader->get('foo.com', 'bar', 'foo.txt'); + } + + public function test_file_downloader_exception_other() + { + $this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client') + ->onlyMethods(['request']) + ->getMock(); + + $this->guzzle_mock->method('request') + ->will($this->returnCallback(function($method, $uri) + { + throw new \RuntimeException('FSOCK_NOT_SUPPORTED'); + } + )); + + $file_downloader = $this->getMockBuilder(\phpbb\file_downloader::class) + ->onlyMethods(['create_client']) + ->getMock(); + + $file_downloader->method('create_client') + ->willReturn($this->guzzle_mock); + + $this->expectException(\phpbb\exception\runtime_exception::class); + $this->expectExceptionMessage('FSOCK_DISABLED'); + + $file_downloader->get('foo.com', 'bar', 'foo.txt'); + } }