Skip to content

Fix .jpeg extension mismatch when mime option is jpeg #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wakqasahmed wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix .jpeg extension mismatch when mime option is jpeg #320

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a28096
Fix .jpeg extension mismatch when mime option is jpeg (#160)
aheinze Aug 16, 2026
afe0d59
Normalize mime/ext before hash construction, fix jpeg extension misma…
aheinze Aug 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions modules/Assets/Helper/Asset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ protected function imageByPath(array $options = [], bool $asPath = false, ?strin
if ($mime === 'jpg') $mime = 'jpeg';

if ($mime && \in_array($mime, ['avif', 'gif', 'jpeg', 'png', 'webp', 'bmp'])) {
$ext = $mime;
$mime = "image/{$ext}";
$ext = ($mime === 'jpeg') ? 'jpg' : $mime;

@wakqasahmed wakqasahmed Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead for the reported case. image() L91 already built $hash as md5(...)."_{$quality}_{$mode}.{$mime}" from the raw option and passed it down here, so L239 short-circuits ($hash ?? ...) and this $ext never reaches the output filename or cache key whenever mime is set — which is the only situation in which #160 occurs. With mime: 'jpeg' the thumbnail still lands at ..._thumbnail.jpeg. The normalization needs to happen before/at L91, or $hash construction needs to move after this block.

The $mime = "image/{$mime}" half is correct and preserves image/jpeg for Img::toString() — no objection there.

@wakqasahmed wakqasahmed Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed properly this time. Root cause was exactly as you described: image() built $hash from the raw $mime at what was line 91 before any normalization ran, so imageByPath()'s $ext fix at L227 never reached the hash/filename whenever a mime option was set — the only case #160 occurs in.

Fix: extracted the jpeg->jpg (and image/ prefix stripping) normalization into a shared normalizeImageMime() helper, and call it in image() before $hash is built, then reuse the same helper in imageByPath() so both paths agree. Verified end-to-end this time by bootstrapping the real Cockpit app (via bootstrap.php/Cockpit::instance()) in a docker php:8.3 container matching the repo's own Dockerfile deps (gd, zip, pdo_sqlite), calling the actual public $app->helper('asset')->image([...]) entry point against a real uploaded jpg, and asserting on the real returned storage path — not a reimplemented snippet. Confirmed the added assertion fails against the pre-fix code (produces ..._thumbnail.jpeg) and passes against the new fix (produces ..._thumbnail.jpg).

$mime = "image/{$mime}";
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
} else {
$mime = null;
}
Expand Down Expand Up @@ -297,7 +297,8 @@ protected function imageByPath(array $options = [], bool $asPath = false, ?strin
}

if ($base64) {
return "data:image/{$ext};base64,".\base64_encode($this->app->fileStorage->read($thumbpath));
$dataMime = $mime ?: "image/{$ext}";
return "data:{$dataMime};base64,".\base64_encode($this->app->fileStorage->read($thumbpath));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

return $asPath ? $thumbpath : $this->app->fileStorage->getURL($thumbpath);
Expand Down