diff --git a/CHANGELOG.md b/CHANGELOG.md index a640c37..ae2230d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Latest ### Fixes * [#15](https://github.com/cleverage/archive-process-bundle/issues/15) Fix ZipTask and UnzipTask: resolve the options on every execution, so that each input is used (the options of the first input were reused for the following ones). Update documentation, add tests. +* [#16](https://github.com/cleverage/archive-process-bundle/issues/16) Fix ZipTask: only remove the leading `files_base_path` from the file paths (every occurrence was removed), and read the files at the given path when `files_base_path` is empty (relative paths were resolved from the filesystem root). Update documentation, add tests. v2.1 ------ diff --git a/docs/reference/tasks/zip_task.md b/docs/reference/tasks/zip_task.md index 6f76b48..6171a05 100644 --- a/docs/reference/tasks/zip_task.md +++ b/docs/reference/tasks/zip_task.md @@ -33,7 +33,7 @@ Options | Code | Type | Required | Default | Description | |-------------------|-------------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| | `filename` | `string` | **X** | | Path of the zip archive to create. An existing archive is overwritten | -| `files` | `string\|array` | **X** | | Path of the file, or list of paths of the files, to add to the archive. Paths are relative to `files_base_path`, or absolute and starting with `files_base_path` | +| `files` | `string\|array` | **X** | | Path of the file, or list of paths of the files, to add to the archive. Paths are relative to `files_base_path`, or absolute and starting with `files_base_path` (any path when `files_base_path` is empty) | | `files_base_path` | `string` | | `''` | Base directory of the files to add. It is removed from the file paths to build the names of the archive entries | Options can be provided either in the task configuration or in the input. @@ -87,12 +87,11 @@ Notes * Underlying class is [ZipArchive](https://www.php.net/manual/en/class.ziparchive.php), the archive is opened with the `ZipArchive::CREATE | ZipArchive::OVERWRITE` flags. A `\RuntimeException` is thrown if it cannot be opened. -* For each file, the entry name is the file path with every occurrence of `files_base_path` removed and leading - directory separators trimmed; the file actually read is `files_base_path` + directory separator + entry name. - Hence: - * all files must be located under `files_base_path`; - * with the default empty `files_base_path`, paths must be absolute (a relative path would be resolved from the - filesystem root), and the entries keep their full path (without the leading `/`) inside the archive. +* For each file, the entry name is the file path with the leading `files_base_path` removed (only when the path starts + with it, followed by a directory separator) and leading directory separators trimmed; the file actually read is + `files_base_path` + directory separator + entry name. Hence all files must be located under `files_base_path`. +* With the default empty `files_base_path`, the file is read at the given path (absolute, or relative to the current + directory), and the entries keep this path (without the leading `/`) inside the archive. * Each file must exist and be readable, otherwise an `\UnexpectedValueException` is thrown. Only files can be added: directories are not browsed. * Options are resolved on each execution of the task: when the task receives several inputs (e.g. after an iterable diff --git a/src/Task/ZipTask.php b/src/Task/ZipTask.php index 962ad7d..1971498 100644 --- a/src/Task/ZipTask.php +++ b/src/Task/ZipTask.php @@ -42,9 +42,20 @@ public function execute(ProcessState $state): void if (\is_string($files)) { $files = [$files]; } + $basePath = rtrim($options['files_base_path'], \DIRECTORY_SEPARATOR); foreach ($files as $file) { - $currentFilename = ltrim(str_replace($options['files_base_path'], '', $file), \DIRECTORY_SEPARATOR); - $currentFilepath = $options['files_base_path'].\DIRECTORY_SEPARATOR.$currentFilename; + if ('' === $options['files_base_path']) { + // No base path: the file is read at the given path (absolute or relative to the current directory) + $currentFilename = ltrim($file, \DIRECTORY_SEPARATOR); + $currentFilepath = $file; + } else { + // Only the leading base path is removed, other paths are relative to the base path + if (str_starts_with($file, $basePath.\DIRECTORY_SEPARATOR)) { + $file = substr($file, \strlen($basePath) + 1); + } + $currentFilename = ltrim($file, \DIRECTORY_SEPARATOR); + $currentFilepath = $basePath.\DIRECTORY_SEPARATOR.$currentFilename; + } if (!file_exists($currentFilepath)) { throw new \UnexpectedValueException("File does not exists: '{$currentFilepath}'"); } diff --git a/tests/Task/ZipTaskTest.php b/tests/Task/ZipTaskTest.php index 1f0af0e..519d84c 100644 --- a/tests/Task/ZipTaskTest.php +++ b/tests/Task/ZipTaskTest.php @@ -66,6 +66,39 @@ public function testEachInputIsUsed(): void self::assertSame(['file2.txt'], $this->getEntries($this->dir.'/archive2.zip')); } + public function testOnlyTheLeadingBasePathIsRemoved(): void + { + mkdir($this->dir.'/sub'.$this->dir, 0o777, true); + file_put_contents($this->dir.'/sub'.$this->dir.'/file3.txt', 'file 3'); + [$task, $state] = $this->createTask([ + 'filename' => $this->dir.'/archive.zip', + 'files' => $this->dir.'/sub'.$this->dir.'/file3.txt', + 'files_base_path' => $this->dir.'/', + ]); + + $this->execute($task, $state, null); + + self::assertSame(['sub'.$this->dir.'/file3.txt'], $this->getEntries($this->dir.'/archive.zip')); + } + + public function testRelativePathWithoutBasePath(): void + { + $cwd = (string) getcwd(); + chdir($this->dir); + try { + [$task, $state] = $this->createTask([ + 'filename' => $this->dir.'/archive.zip', + 'files' => ['file1.txt', $this->dir.'/file2.txt'], + ]); + + $this->execute($task, $state, null); + } finally { + chdir($cwd); + } + + self::assertSame(['file1.txt', ltrim($this->dir, '/').'/file2.txt'], $this->getEntries($this->dir.'/archive.zip')); + } + /** * @param array $options *