Merge pull request #6715 from marc1706/ticket/17385

[ticket/17385] Backport file_downloader with guzzle for versioncheck with redirect support
2025-07-22 18:08:54 +00:00 · 2024-09-29 19:57:33 +02:00 · 2024-09-29 19:57:33 +02:00 · 13b267ca6f
commit 13b267ca6f
parent 6db142b8ef 9ace4dc585
3 changed files with 323 additions and 63 deletions
--- a/phpBB/phpbb/file_downloader.php
+++ b/phpBB/phpbb/file_downloader.php
@ -13,91 +13,116 @@

 namespace phpbb;

+use GuzzleHttp\Client;
+use GuzzleHttp\Exception\RequestException;
+use phpbb\exception\runtime_exception;
+
 class file_downloader
 {
+	const OK = 200;
+	const NOT_FOUND = 404;
+	const REQUEST_TIMEOUT = 408;
+
 	/** @var string Error string */
 	protected $error_string = '';

 	/** @var int Error number */
 	protected $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: 443
+	 * @param int       $timeout         Connection timeout in seconds; default: 6
 	 *
-	 * @return mixed File data as string if file can be read and there is no
-	 *			timeout, false if there were errors or the connection timed out
+	 * @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
 	 *
-	 * @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)
 	{
+		// 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 (!empty($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 +130,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 +140,7 @@ class file_downloader
 	 *
 	 * @return int Error number
 	 */
-	public function get_error_number()
+	public function get_error_number(): int
 	{
 		return $this->error_number;
 	}
--- a/tests/version/fixture/30x.txt
+++ b/tests/version/fixture/30x.txt
@ -0,0 +1,4 @@
+3.0.14
+https://www.phpbb.com/community/viewtopic.php?f=14&t=2313941
+3.3.12
+https://www.phpbb.com/community/viewtopic.php?t=2653732
--- a/tests/version/version_helper_remote_test.php
+++ b/tests/version/version_helper_remote_test.php
@ -1,4 +1,7 @@
 <?php
+
+use GuzzleHttp\Exception\RequestException;
+
 /**
 *
 * This file is part of the phpBB Forum Software package.
@ -17,6 +20,11 @@ class version_helper_remote_test extends \phpbb_test_case
 	protected $cache;
 	protected $version_helper;

+	// Guzzle mock data
+	protected $guzzle_status = 200; // Default to 200 status
+	protected $guzzle_data;
+	protected $guzzle_mock;
+
 	protected function setUp(): void
 	{
 		parent::setUp();
@ -38,9 +46,30 @@ class version_helper_remote_test extends \phpbb_test_case

 		$this->cache->expects($this->any())
 			->method('get')
-			->with($this->anything())
+			->withAnyParameters()
 			->will($this->returnValue(false));
-		$this->file_downloader = new phpbb_mock_file_downloader();
+
+		$this->guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client')
+			->setMethods(['set_data', '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')
+			->setMethods(['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);

@ -202,7 +231,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
@ -213,7 +242,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
@ -223,4 +252,206 @@ class version_helper_remote_test extends \phpbb_test_case

 		$this->assertEquals($expected_return, $return);
 	}
+
+	public function test_version_phpbb_com()
+	{
+		$guzzle_mock = $this->getMockBuilder('\GuzzleHttp\Client')
+			->setMethods(['request'])
+			->getMock();
+
+		$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)
+			->setMethods(['create_client'])
+			->getMock();
+
+		$file_downloader->method('create_client')
+			->willReturn($guzzle_mock);
+
+		$hostname = 'version.phpbb.com';
+
+		$file = $file_downloader->get($hostname, '/phpbb', '30x.txt');
+		$errstr = $file_downloader->get_error_string();
+		$errno = $file_downloader->get_error_number();
+
+		$this->assertNotEquals(
+			0,
+			strlen($file),
+			'Failed asserting that the response is not empty.'
+		);
+
+		$this->assertSame(
+			'',
+			$errstr,
+			'Failed asserting that the error string is empty.'
+		);
+
+		$this->assertSame(
+			0,
+			$errno,
+			'Failed asserting that the error number is 0 (i.e. no error occurred).'
+		);
+
+		$lines = explode("\n", $file);
+
+		$this->assertGreaterThanOrEqual(
+			2,
+			count($lines),
+			'Failed asserting that the version file has at least two lines.'
+		);
+
+		$this->assertStringStartsWith(
+			'3.',
+			$lines[0],
+			"Failed asserting that the first line of the version file starts with '3.'"
+		);
+
+		$this->assertNotSame(
+			false,
+			filter_var($lines[1], FILTER_VALIDATE_URL),
+			'Failed asserting that the second line of the version file is a valid URL.'
+		);
+
+		$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')
+			->setMethods(['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)
+			->setMethods(['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')
+			->setMethods(['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)
+			->setMethods(['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')
+			->setMethods(['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)
+			->setMethods(['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')
+			->setMethods(['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)
+			->setMethods(['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')
+			->setMethods(['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)
+			->setMethods(['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');
+	}
 }