diff --git a/src/Console/Command/Extract.php b/src/Console/Command/Extract.php index 64f58a936..9596a0782 100644 --- a/src/Console/Command/Extract.php +++ b/src/Console/Command/Extract.php @@ -160,7 +160,7 @@ private static function dumpPhar(string $file, string $tmpDir): string $pubKeyContent = FS::getFileContents($pubKey); } - $phar = PharFactory::create($tmpFile); + $phar = PharFactory::create($tmpFile, $file); $pharMeta = PharMeta::fromPhar($phar, $pubKeyContent); $phar->extractTo($tmpDir); diff --git a/src/Phar/InvalidPhar.php b/src/Phar/InvalidPhar.php index 3a2bcbd44..f2eaf5d67 100644 --- a/src/Phar/InvalidPhar.php +++ b/src/Phar/InvalidPhar.php @@ -25,24 +25,40 @@ final class InvalidPhar extends PharError { - public static function fileNotLocal(string $file): self - { + public static function fileNotLocal( + string $file, + ?string $originalFile = null, + ): self { // Covers: // https://github.com/php/php-src/blob/930db2b2d315b2acc917706cf76bed8b09f94b79/ext/phar/phar.c#L1328 return new self( sprintf( - 'Could not create a Phar or PharData instance for the file path "%s". PHAR objects can only be created from local files.', + 'Could not create a Phar or PharData instance for the file path "%s"%s. PHAR objects can only be created from local files.', $file, + null === $originalFile + ? '' + : sprintf( + ' (of the original file "%s")', + $originalFile, + ), ), ); } - public static function fileNotFound(string $file): self - { + public static function fileNotFound( + string $file, + ?string $originalFile = null, + ): self { return new self( sprintf( - 'Could not find the file "%s".', + 'Could not find the file "%s"%s.', $file, + null === $originalFile + ? '' + : sprintf( + ' (of the original file "%s")', + $originalFile, + ), ), ); } @@ -57,32 +73,42 @@ public static function fileNotReadable(string $file): self ); } - public static function forPhar(string $file, ?Throwable $previous): self - { + public static function forPhar( + string $file, + ?string $originalFile, + ?Throwable $previous, + ): self { return new self( - self::mapThrowableToErrorMessage($file, $previous, false), + self::mapThrowableToErrorMessage($file, $originalFile, $previous, false), previous: $previous, ); } - public static function forPharData(string $file, ?Throwable $previous): self - { + public static function forPharData( + string $file, + ?string $originalFile, + ?Throwable $previous, + ): self { return new self( - self::mapThrowableToErrorMessage($file, $previous, true), + self::mapThrowableToErrorMessage($file, $originalFile, $previous, true), previous: $previous, ); } - public static function forPharAndPharData(string $file, ?Throwable $previous): self - { + public static function forPharAndPharData( + string $file, + ?string $originalFile, + ?Throwable $previous, + ): self { return new self( - self::mapThrowableToErrorMessage($file, $previous, null), + self::mapThrowableToErrorMessage($file, $originalFile, $previous, null), previous: $previous, ); } private static function mapThrowableToErrorMessage( string $file, + ?string $originalFile, ?Throwable $throwable, ?bool $isPharData, ): string { @@ -92,15 +118,24 @@ private static function mapThrowableToErrorMessage( $pharObject = $isPharData ? 'PharData' : 'Phar'; } - $message = $throwable->getMessage(); + $errorMessageStart = sprintf( + 'Could not create a %s instance for the file "%s"%s', + $pharObject, + $file, + null === $originalFile + ? '' + : sprintf( + ' (of the original file "%s")', + $originalFile, + ), + ); + $message = $throwable?->getMessage() ?? ''; if ($throwable instanceof UnexpectedValueException) { // https://github.com/php/php-src/blob/930db2b2d315b2acc917706cf76bed8b09f94b79/ext/phar/phar.c#L1330 if (str_ends_with($message, 'file extension (or combination) not recognised or the directory does not exist')) { return sprintf( - 'Could not create a %s instance for the file "%s". The file must have the extension "%s".', - $pharObject, - $file, + $errorMessageStart.'. The file must have the extension "%s".', $isPharData ? '.zip", ".tar", ".tar.bz2" or ".tar.gz' : '.phar', ); } @@ -111,9 +146,7 @@ private static function mapThrowableToErrorMessage( preg_match('/^internal corruption of phar \".+\" \((?.+)\)$/', $message, $matches); return sprintf( - 'Could not create a %s instance for the file "%s". The archive is corrupted: %s.', - $pharObject, - $file, + $errorMessageStart.'. The archive is corrupted: %s.', ucfirst($matches['reason']), ); } @@ -122,11 +155,7 @@ private static function mapThrowableToErrorMessage( // https://github.com/php/php-src/blob/930db2b2d315b2acc917706cf76bed8b09f94b79/ext/phar/phar.c#L892 // https://github.com/php/php-src/blob/930db2b2d315b2acc917706cf76bed8b09f94b79/ext/phar/phar.c#L903 if (str_contains($message, ' openssl signature ')) { - return sprintf( - 'Could not create a %s instance for the file "%s". The OpenSSL signature could not be read or verified.', - $pharObject, - $file, - ); + return $errorMessageStart.'. The OpenSSL signature could not be read or verified.'; } // https://github.com/php/php-src/blob/930db2b2d315b2acc917706cf76bed8b09f94b79/ext/phar/phar.c#L1002 @@ -137,18 +166,12 @@ private static function mapThrowableToErrorMessage( || str_contains($message, ' signature could not be verified') || str_contains($message, ' has a broken or unsupported signature') ) { - return sprintf( - 'Could not create a %s instance for the file "%s". The archive signature is broken.', - $pharObject, - $file, - ); + return $errorMessageStart.'. The archive signature is broken.'; } } return sprintf( - 'Could not create a %s instance for the file "%s": %s', - $pharObject, - $file, + $errorMessageStart.': %s', $message, ); } diff --git a/src/Phar/PharFactory.php b/src/Phar/PharFactory.php index 11d75d546..6cdc7df81 100644 --- a/src/Phar/PharFactory.php +++ b/src/Phar/PharFactory.php @@ -33,19 +33,21 @@ private function __construct() /** * @throws InvalidPhar */ - public static function create(string $file): Phar|PharData - { + public static function create( + string $file, + ?string $originalFile = null, + ): Phar|PharData { if (!Path::isLocal($file)) { // This is needed as otherwise Phar::__construct() does correctly bail out on a URL // path, but not on other non-local variants, e.g. FTPS, which case it may fail still // but after a timeout, which is too slow. - throw InvalidPhar::fileNotLocal($file); + throw InvalidPhar::fileNotLocal($file, $originalFile); } if (!file_exists($file)) { // We need to check this case since the goal of this factory is to instantiate an existing // PHAR, not create a new one. - throw InvalidPhar::fileNotFound($file); + throw InvalidPhar::fileNotFound($file, $originalFile); } try { @@ -57,45 +59,49 @@ public static function create(string $file): Phar|PharData try { return new PharData($file); } catch (Throwable) { - throw InvalidPhar::forPharAndPharData($file, $cannotCreatePhar); + throw InvalidPhar::forPharAndPharData($file, $originalFile, $cannotCreatePhar); } } /** * @throws InvalidPhar */ - public static function createPhar(string $file): Phar - { + public static function createPhar( + string $file, + ?string $originalFile = null, + ): Phar { if (!Path::isLocal($file)) { // This is needed as otherwise Phar::__construct() does correctly bail out on a URL // path, but not on other non-local variants, e.g. FTPS, which case it may fail still // but after a timeout, which is too slow. - throw InvalidPhar::fileNotLocal($file); + throw InvalidPhar::fileNotLocal($file, $originalFile); } if (!file_exists($file)) { // We need to check this case since the goal of this factory is to instantiate an existing // PHAR, not create a new one. - throw InvalidPhar::fileNotFound($file); + throw InvalidPhar::fileNotFound($file, $originalFile); } try { return new Phar($file); } catch (Throwable $throwable) { - throw InvalidPhar::forPhar($file, $throwable); + throw InvalidPhar::forPhar($file, $originalFile, $throwable); } } /** * @throws InvalidPhar */ - public static function createPharData(string $file): PharData - { + public static function createPharData( + string $file, + ?string $originalFile = null, + ): PharData { if (!Path::isLocal($file)) { // This is needed as otherwise Phar::__construct() does correctly bail out on a URL // path, but not on other non-local variants, e.g. FTPS, which case it may fail still // but after a timeout, which is too slow. - throw InvalidPhar::fileNotLocal($file); + throw InvalidPhar::fileNotLocal($file, $originalFile); } if (!file_exists($file)) { @@ -107,7 +113,7 @@ public static function createPharData(string $file): PharData try { return new PharData($file); } catch (Throwable $throwable) { - throw InvalidPhar::forPharData($file, $throwable); + throw InvalidPhar::forPharData($file, $originalFile, $throwable); } } } diff --git a/tests/Phar/PharFactoryTest.php b/tests/Phar/PharFactoryTest.php index cb81d4575..f5adf5608 100644 --- a/tests/Phar/PharFactoryTest.php +++ b/tests/Phar/PharFactoryTest.php @@ -86,6 +86,19 @@ public function test_it_fails_with_a_comprehensive_error_when_cannot_create_a_ph PharFactory::createPhar($file); } + public function test_it_fails_with_a_comprehensive_error_when_cannot_create_a_phar_which_is_a_copy(): void + { + $this->expectException(InvalidPhar::class); + $this->expectExceptionMessageMatches( + '/^Could not create a Phar instance for the file ".+"\ \(of the original file "original\.phar"\)\. The file must have the extension "\.phar"\.$/', + ); + + PharFactory::createPhar( + self::FIXTURES_DIR.'/empty-pdf.pdf', + 'original.phar', + ); + } + /** * @dataProvider invalidPharDataProvider */