Skip to content

[Bug]: ZipService::addFile() silently adds 0-byte entries for assets whose binary is missing from storage #1987

Description

@cancan101

On routing: this repo's ISSUE_TEMPLATE/config.yml points reproducible bugs at pimcore/platform-version, and the primary report is filed there as pimcore/platform-version#279. This ticket exists because the affected code is in this repository and the fix here is possible independently of the core change. Happy for it to be closed as a duplicate if you'd rather track it centrally.

Summary

ZipService::addFile() adds an entry for every asset it is given, including assets whose binary is missing from the configured asset storage. Those arrive in the archive as 0-byte files, with no error, no job-run message, and nothing in the log. The download reports success.

Affected code

https://github.com/pimcore/studio-backend-bundle/blob/2026.x/src/Asset/Service/ExecutionEngine/ZipService.php#L70-L82

public function addFile(ZipArchive $archive, Asset $asset, string $basePath = '/'): void
{
    $archive->addFile(
        $asset->getLocalFile(),
        ...
    );
}

Called from ZipDownloadHandler once per asset.

Why the entry is empty rather than absent or failed

Asset::getLocalFile() is getLocalFileFromStream($this->getStream()), and Asset::getStream() in core substitutes a fresh tmpfile() whenever the storage read throws, discarding the exception entirely:

https://github.com/pimcore/pimcore/blob/2026.x/models/Asset.php#L1136-L1142

try {
    $this->stream = Storage::get('asset')->readStream($this->getRealFullPath());
} catch (Exception $e) {
    $this->stream = tmpfile();     // valid handle, zero bytes, $e discarded
}

TemporaryFileHelperTrait::getLocalFileFromStream() then resolves that handle to stream_get_meta_data($stream)['uri'] — a real path to a real, readable, empty temp file. ZipArchive::addFile() sees an ordinary file and adds it. Nothing in the chain can distinguish it from an asset the user legitimately uploaded empty.

Asset::getFileSize() is not usable as a detector either — it has the same swallow-and-substitute shape and returns 0 for a missing binary and for a genuinely empty file alike (models/Asset.php#L1565).

Steps to reproduce

  1. Have assets whose rows exist but whose binaries are absent from asset storage — routine after a database restore without the matching bucket, an interrupted storage migration, or a bucket lifecycle rule expiring objects.
  2. Download them as a ZIP through Studio.
  3. Every affected entry is 0 bytes; the job run reports success.

On one of our QA environments only 10 of the 300 newest asset rows had binaries present. The resulting archive was valid, had a complete and plausible file listing, and 290 empty files — indistinguishable from a correct archive. That is worse than a hard failure, because the bad output is silently trusted.

Why this repo can fix it without waiting for core

Unlike a streamed response, ZipDownloadHandler still has somewhere to put the information: it already calls abort()/getAbortData() for unsupported elements and updateProgress() per asset, and the job run is a natural channel for a per-asset warning. So a missing binary could be surfaced to the user — as a job-run message, a skipped-files list, or an entry in the archive — rather than silently written as an empty file.

Detection today requires a heuristic, because core exposes no signal. The cheapest correct one is a fileExists() round trip on the asset storage, and it should only be paid when the file is actually empty, so that legitimately-empty uploads are not mislabelled.

Suggested fix

Ideally, core grows a way to distinguish "binary missing from storage" from "legitimately empty" — two backward-compatible options (a placeholder flag on the fallback, or an opt-in accessor that throws) are laid out in pimcore/platform-version#279. This bundle would then consult that signal in addFile() and report through the job run.

Until then, an interim check in addFile() (empty file ⇒ one Storage::get('asset')->fileExists() call ⇒ report) would close the user-visible hole on its own.

Note that returning null from getStream() is deliberately not what's being proposed upstream — the tmpfile() fallback exists precisely to avoid that, cf. pimcore/pimcore#13260.

Related

Environment

  • pimcore/studio-backend-bundle 2026.x (verified on the current default branch, and against the copy installed from pimcore/pimcore ^12.3.2)
  • Storage: Flysystem, non-local adapter

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Fields

    Affected capability

    None yet

    Platform Version

    None yet

    Galaxy

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions