Skip to content

Commit

Permalink
feat: Add an internal option to the extract command (#975)
Browse files Browse the repository at this point in the history 
Introduce the option `--internal` to the extract command. There is a
number of changes with the introduction of this option:

- Instead of basing the default value to the question of whether the
  target dir should be deleted or not, it is now done based in this flag
- When this flag is passed, the error thrown when creating the PHAR is
  swallowed and its message written to the STDERR. This will allow to
retrieve it in Pharaoh when executing it without having the whole stack
trace.
  • Loading branch information
@theofidry
theofidry authored Apr 1, 2023
1 parent 3d0b441 commit 99dabd9
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 23 deletions.
26 changes: 24 additions & 2 deletions src/Console/Command/Extract.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
use Fidry\Console\ExitCode;
use Fidry\Console\Input\IO;
use KevinGH\Box\Phar\PharFactory;
use KevinGH\Box\Pharaoh\InvalidPhar;
use KevinGH\Box\Pharaoh\Pharaoh;
use ParagonIE\ConstantTime\Hex;
use Phar;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Question\ConfirmationQuestion;
use Throwable;
use function file_exists;
Expand All @@ -43,6 +45,7 @@ final class Extract implements Command
{
private const PHAR_ARG = 'phar';
private const OUTPUT_ARG = 'output';
private const INTERNAL_OPT = 'internal';

public function getConfiguration(): Configuration
{
Expand All @@ -62,6 +65,14 @@ public function getConfiguration(): Configuration
'The output directory',
),
],
[
new InputOption(
self::INTERNAL_OPT,
null,
InputOption::VALUE_NONE,
'Internal option; Should not be used.',
),
],
);
}

Expand All @@ -71,6 +82,7 @@ public function execute(IO $io): int

$pharPath = self::getPharFilePath($io);
$outputDir = $io->getArgument(self::OUTPUT_ARG)->asNonEmptyString();
$internal = $io->getOption(self::INTERNAL_OPT)->asBoolean();

if (null === $pharPath) {
return ExitCode::FAILURE;
Expand All @@ -82,7 +94,7 @@ public function execute(IO $io): int
'The output directory already exists. Do you want to delete its current content?',
// If is interactive, we want the prompt to default to false since it can be an error made by the user.
// Otherwise, this is likely launched by a script or Pharaoh in which case we do not care.
!$io->isInteractive(),
$internal,
),
);

Expand All @@ -97,7 +109,17 @@ public function execute(IO $io): int

mkdir($outputDir);

self::dumpPhar($pharPath, $outputDir);
try {
self::dumpPhar($pharPath, $outputDir);
} catch (InvalidPhar $invalidPhar) {
if (!$internal) {
throw $invalidPhar;
}

$io->getErrorIO()->write($invalidPhar->getMessage());

return ExitCode::FAILURE;
}

return ExitCode::SUCCESS;
}
Expand Down
88 changes: 67 additions & 21 deletions tests/Console/Command/ExtractTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public function test_it_can_extract_a_phar(
'command' => 'extract',
'phar' => $pharPath,
'output' => $this->tmp,
'--internal' => null,
],
['interactive' => false],
);
Expand Down Expand Up @@ -133,21 +134,31 @@ private static function pharProvider(): iterable
*/
public function test_it_asks_confirmation_before_deleting_the_output_dir(
bool $outputDirExists,
bool $interactive,
bool $input,
bool $internal,
?bool $input,
bool $expectedToSucceed,
): void {
$outputDir = $outputDirExists ? $this->tmp : $this->tmp.'/subdir';

$this->commandTester->setInputs([$input ? 'yes' : 'no']);
$this->commandTester->execute(
[
'command' => 'extract',
'phar' => self::FIXTURES.'/simple-phar.phar',
'output' => $outputDir,
],
['interactive' => $interactive],
);
if (null !== $input) {
$commandOptions = ['interactive' => true];
$this->commandTester->setInputs([$input ? 'yes' : 'no']);
} else {
$commandOptions = ['interactive' => false];
}

$commandInput = [
'command' => 'extract',
'phar' => self::FIXTURES.'/simple-phar.phar',
'output' => $outputDir,
'--internal' => null,
];

if (!$internal) {
unset($commandInput['--internal']);
}

$this->commandTester->execute($commandInput, $commandOptions);

$actualFiles = $this->collectExtractedFiles();

Expand All @@ -162,46 +173,53 @@ public function test_it_asks_confirmation_before_deleting_the_output_dir(

public static function confirmationQuestionProvider(): iterable
{
yield 'exists; accept' => [
yield 'exists; internal; interactive & accept => delete' => [
true,
true,
true,
true,
];

yield 'exists; refuse' => [
yield 'exists; internal; interactive & refuse => do not delete' => [
true,
true,
false,
false,
];

yield 'does not exist: the question is not asked' => [
false,
yield 'exists; internal; not interactive => delete' => [
true,
false,
true,
null,
true,
];

yield 'exists; not interactive; accept' => [
yield 'exists; not internal; interactive & accept => delete' => [
true,
false,
true,
true,
];

yield 'exists; not interactive; refuse' => [
yield 'exists; not internal; interactive & refuse => do not delete' => [
true,
false,
false,
true,
false,
];

yield 'not interactive; does not exist: the question is not asked' => [
yield 'exists; not internal; not interactive => do not delete' => [
true,
false,
null,
false,
];

yield 'does not exist: the question is not asked' => [
false,
true,
null,
true,
];
}

Expand All @@ -216,11 +234,12 @@ public function test_it_cannot_extract_an_invalid_phar(string $pharPath): void
'command' => 'extract',
'phar' => $pharPath,
'output' => $this->tmp,
'--internal' => false,
],
['interactive' => false],
);

self::fail('Expected exception to be thrown.');
self::assertSame(ExitCode::FAILURE, $this->commandTester->getStatusCode());
} catch (InvalidPhar $exception) {
// Continue
}
Expand All @@ -230,11 +249,38 @@ public function test_it_cannot_extract_an_invalid_phar(string $pharPath): void

public static function invalidPharPath(): iterable
{
yield 'non-existent file' => [
'/unknown',
];

yield 'not a valid PHAR' => [
self::FIXTURES.'/../phar/empty-pdf.pdf',
];
}

public function test_it_writes_the_invalid_phar_error_to_the_stderr_when_cannot_extract_a_phar_in_internal_mode(): void
{
$this->commandTester->execute(
[
'command' => 'extract',
'phar' => self::FIXTURES.'/../phar/empty-pdf.pdf',
'output' => $this->tmp,
'--internal' => true,
],
[
'interactive' => false,
'capture_stderr_separately' => true,
],
);

self::assertSame(ExitCode::FAILURE, $this->commandTester->getStatusCode());
self::assertSame('', $this->commandTester->getDisplay(true));
self::assertMatchesRegularExpression(
'/^Could not create a Phar or PharData instance for the file/',
$this->commandTester->getErrorOutput(true),
);
}

/**
* @return array<string,string>
*/
Expand Down

0 comments on commit 99dabd9

Please sign in to comment.