Skip to content
This repository was archived by the owner on Aug 13, 2026. It is now read-only.
This repository was archived by the owner on Aug 13, 2026. It is now read-only.

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

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. Filing here as well because the affected code is in this repository. I'm aware this bundle no longer accepts pull requests, so this is for the record and for anyone hitting it on a 2.x installation — close it as a duplicate if you'd rather track it centrally.

Summary

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

Affected code

https://github.com/pimcore/admin-ui-classic-bundle/blob/2.x/src/Controller/Admin/Asset/AssetController.php#L1979

$zip->addFile($a->getLocalFile(), preg_replace('@^' . preg_quote($asset->getRealPath(), '@') . '@i', '', $a->getRealFullPath()));

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. Select the containing folder in the asset tree and use Download as ZIP.
  3. Every affected entry is 0 bytes; the job chain 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.

Note for anyone replacing this flow

We hit this while replacing the download-as-zip-jobsdownload-as-zip-add-filesdownload-as-zip chain with a single streamed response, for unrelated reasons (the chain's shared PIMCORE_SYSTEM_TEMP_DIRECTORY archive is not safe when var/tmp is on a shared filesystem and requests are load-balanced across nodes).

Streaming makes this bug materially worse, which is worth recording: the response is committed before a later asset turns out to be missing, so there is no status code left to change and the only channel back to the user is inside the archive itself. The job-chain version at least assembles the archive server-side before any download starts, so it could abort and report — it just doesn't. Anything replacing this flow inherits the blind spot and has fewer places to put the error.

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 before adding the entry.

An interim check at the call site (empty file ⇒ one Storage::get('asset')->fileExists() call ⇒ report) would close the user-visible hole without any core change, if this bundle is still taking fixes of that size.

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/admin-ui-classic-bundle 2.x (verified on the current 2.x and 2.3 branches)
  • pimcore/pimcore ^12.3.2; the core code paths above are unchanged on 11.x, 12.x and 2026.x
  • 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

    Labels

    No labels
    No labels

    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