Skip to content

Commit

Permalink
Clean up extension handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@whikloj
whikloj committed Apr 12, 2024
1 parent 2d04d1a commit e6a0b8f
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 41 deletions.
3 changes: 2 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@
],
"test": [
"@check",
"@phpunit"
"@phpunit",
"@phpstan"
]
},
"config": {
Expand Down
59 changes: 27 additions & 32 deletions src/Bag.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,17 @@ class Bag
* Extensions which map to a tar file.
*/
private const TAR_EXTENSIONS = [
'tar',
'tgz',
'tar.gz',
'tar.bz2',
'.tar',
'.tgz',
'.tar.gz',
'.tar.bz2',
];

/**
* Extensions which map to a zip file.
*/
private const ZIP_EXTENSIONS = [
'zip',
'.zip',
];

/**
Expand Down Expand Up @@ -324,7 +324,7 @@ public static function load(string $rootPath): Bag
$serialized_extension = null;
if (is_file($rootPath)) {
$extension = self::getExtension($rootPath);
if (self::isCompressed($extension)) {
if (self::hasExtension($extension, array_merge(self::ZIP_EXTENSIONS, self::TAR_EXTENSIONS))) {
$serialized_extension = $extension;
$rootPath = self::uncompressBag($rootPath, $serialized_extension);
}
Expand Down Expand Up @@ -416,7 +416,7 @@ public function finalize(): void
*/
public function package(string $filepath): void
{
if (!self::hasExtension($filepath, $this->packageExtensions)) {
if (!self::hasExtension(self::getExtension($filepath), $this->packageExtensions)) {
throw new BagItException(
"Unknown archive type ($filepath), the file extension must be one of (" .
implode(", ", $this->packageExtensions) . ")"
Expand Down Expand Up @@ -1978,9 +1978,10 @@ private function loadFetch(): void
*/
private function makePackage(string $filename): void
{
if (self::hasExtension($filename, self::ZIP_EXTENSIONS)) {
$extension = self::getExtension($filename);
if (self::hasExtension($extension, self::ZIP_EXTENSIONS)) {
$this->makeZip($filename);
} elseif (self::hasExtension($filename, self::TAR_EXTENSIONS)) {
} elseif (self::hasExtension($extension, self::TAR_EXTENSIONS)) {
$this->makeTar($filename);
} else {
throw new BagItException("Unable to determine archive format.");
Expand Down Expand Up @@ -2060,9 +2061,9 @@ private static function uncompressBag(string $filepath, string $extension): stri
if (!file_exists($filepath)) {
throw new BagItException("File $filepath does not exist.");
}
if (in_array($extension, self::ZIP_EXTENSIONS)) {
if (self::hasExtension($extension, self::ZIP_EXTENSIONS)) {
$directory = self::unzipBag($filepath);
} elseif (in_array($extension, self::TAR_EXTENSIONS)) {
} elseif (self::hasExtension($extension, self::TAR_EXTENSIONS)) {
$directory = self::untarBag($filepath);
} else {
throw new BagItException("Unable to determine archive format.");
Expand Down Expand Up @@ -2147,31 +2148,27 @@ private static function extractDir(): string
}

/**
* Test a filepath to see if we think it is compressed.
* Determine whether the given file extensions ends with an accepted extensions
*
* @param string $extension
* The file extension
* @return bool
* True if compressed file (we support).
*/
private static function isCompressed(string $extension): bool
{
return in_array($extension, array_merge(self::ZIP_EXTENSIONS, self::TAR_EXTENSIONS));
}

/**
* Retrieve whether the given filepath has one of the extensions
*
* @param string $filepath
* The full file path.
* @param string|null $file_extension
* The file extensions.
* @param array $extensions
* The list of extensions to check.
* @return bool
* The list of extensions or an empty array.
*/
private static function hasExtension(string $filepath, array $extensions): bool
private static function hasExtension(?string $file_extension, array $extensions): bool
{
return in_array(self::getExtension($filepath), $extensions);
if (is_null($file_extension)) {
return false;
}
foreach ($extensions as $extension) {
// Need to loop and check each to avoid failing on foo.tmp.tar.gz or bar.old.zip
if (str_ends_with($file_extension, $extension)) {
return true;
}
}
return false;
}

/**
Expand All @@ -2186,17 +2183,15 @@ private static function getExtension(string $filepath): ?string
{
$filename = strtolower(basename($filepath));
$pathinfo = pathinfo($filename);
var_dump($pathinfo);
$extensions = [];
$extensions[] = $pathinfo['extension'] ?? null;
while (strpos($pathinfo['filename'], ".") > -1) {
$pathinfo = pathinfo($pathinfo['filename']);
$extensions[] = $pathinfo['extension'] ?? null;
}
var_dump($extensions);
$extensions = array_filter($extensions);
if (count($extensions) > 0) {
return implode(".", array_reverse($extensions));
return "." . ltrim(implode(".", array_reverse($extensions)), ".\ \n\r\t\v\0");
}
return null;
}
Expand Down
38 changes: 30 additions & 8 deletions tests/BagTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ public function testWarningOnMd5(): void
* @group Bag
* @covers ::load
* @covers ::hasExtension
* @covers ::isCompressed
* @covers ::getExtension
*/
public function testNonExistantCompressed(): void
{
Expand All @@ -644,7 +644,7 @@ public function testNonExistantCompressed(): void
* Test opening a tar gzip
* @group Bag
* @covers ::load
* @covers ::isCompressed
* @covers ::getExtension
* @covers ::uncompressBag
* @covers ::hasExtension
* @covers ::untarBag
Expand All @@ -668,7 +668,7 @@ public function testUncompressTarGz(): void
* Test opening a tar bzip2.
* @group Bag
* @covers ::load
* @covers ::isCompressed
* @covers ::getExtension
* @covers ::uncompressBag
* @covers ::hasExtension
* @covers ::untarBag
Expand All @@ -691,7 +691,7 @@ public function testUncompressTarBzip(): void
/**
* Test opening a zip file.
* @group Bag
* @covers ::isCompressed
* @covers ::getExtension
* @covers ::uncompressBag
* @covers ::hasExtension
* @covers ::unzipBag
Expand All @@ -711,6 +711,30 @@ public function testUncompressZip(): void
);
}

/**
* Test a valid archive with an invalid extension included
*
* @group Bag
* @covers ::getExtension
* @covers ::hasExtension
*/
public function testInvalidExtension(): void
{
$this->tmpdir = $this->prepareBasicTestBag();
$bag = Bag::load($this->tmpdir);
$archivefile = $this->getTempName();
$archivefile .= ".tmp.zip";
$this->assertFileDoesNotExist($archivefile);
$bag->package($archivefile);
$this->assertFileExists($archivefile);
$newbag = Bag::load($archivefile);
$this->assertTrue($newbag->isValid());
$this->assertEquals(
$bag->getPayloadManifests()['sha256']->getHashes(),
$newbag->getPayloadManifests()['sha256']->getHashes()
);
}

/**
* Test generating a zip.
*
Expand All @@ -729,10 +753,8 @@ public function testZipBag(): void
$this->assertFileDoesNotExist($archivefile);
$bag->package($archivefile);
$this->assertFileExists($archivefile);

$newbag = Bag::load($archivefile);
$this->assertTrue($newbag->isValid());

$this->assertEquals(
$bag->getPayloadManifests()['sha256']->getHashes(),
$newbag->getPayloadManifests()['sha256']->getHashes()
Expand Down Expand Up @@ -838,8 +860,8 @@ public function testInvalidPackage(): void
$this->assertFileDoesNotExist($archivefile);

$this->expectException(BagItException::class);
$this->expectExceptionMessage("Unknown archive type ($archivefile), the file extension must be one of (tar, " .
"tgz, tar.gz, tar.bz2, zip)");
$this->expectExceptionMessage("Unknown archive type ($archivefile), the file extension must be one of (.tar, " .
".tgz, .tar.gz, .tar.bz2, .zip)");

$bag->package($archivefile);
}
Expand Down

0 comments on commit e6a0b8f

Please sign in to comment.