[6.x] Hybrid Glide image caching - #14570
duncanmcclean wants to merge 18 commits into
Conversation
|
🔥 |
jasonvarga
left a comment
There was a problem hiding this comment.
Hybrid approach looks great overall — one bug to address before merging (inline). A few smaller notes I'll leave as a follow-up comment so they don't block.
When `itemType()` returns 'id', the item is already a valid asset ID
like `container::folder/file.jpg`. The str_replace was incorrectly
converting `/` to `::`, producing invalid IDs like
`container::folder::file.jpg`.
This caused `Assets::find()` to return null, leading to fatal errors
when building hybrid URLs via `Image::manipulate('container::path')`.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Did a bit of a more thorough review and this introduces a security issue in its current state. I've marked it as draft so we don't merge prematurely. |
Changes were made but there's more to do.
|
Ooo really looking forward to this one! 🔥 |
…hing # Conflicts: # src/Imaging/ImageGenerator.php
jasonvarga
left a comment
There was a problem hiding this comment.
The hybrid design itself is solid — the clean URL is only generatable from a mapping the server wrote during rendering, so the mass-resize vector is closed for hybrid's own URLs. I checked this out and exercised it; the inline comments are the things that need fixing before this can merge. One is a page-breaking crash, the rest are correctness/security gaps in the surrounding routes.
Non-blocking, but worth doing while you're in here:
generateOnDemand()passes the raw request path toGlide::cacheDisk()->exists(). Paths like/img/../../.envthrowLeague\Flysystem\PathTraversalDetected(correctly blocked, no disclosure) but surface as a 500 rather than a 404. Catch it and 404.- Direct serving only works when
routematches the public-relativecache_path(img/public_path('img')), and nothing checks that. If someone changes one, every image request silently hits PHP forever. Worth a docs note or a boot-time warning. - A published hybrid URL is only resolvable while its mapping survives in the Glide cache store. On a statically cached site, losing
storage/framework/cache/glide(e.g. a container redeploy) while the HTML is retained means those images 404 permanently. Docs note at minimum. resolveForUrl()and the'url'mapping branch have no tests.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U772LC9aHMR8DvJ76tBDYt
This pull request implements a new "hybrid" caching strategy for Glide image manipulation.
The Problem
Statamic currently offers two Glide caching modes:
cache: false) — Fast template rendering, but every image request hits PHP, even after the image has already been generated.cache: true) — Images are served directly by the web server, but they're all eagerly generated during template rendering. A page with 10 images × 11 variants = 110 images blocking the page from even loading.Hybrid Caching
Setting
cacheto'hybrid'enables a new mode that combines the best of both:The Glide tag outputs a URL pointing to where the cached image will live, but doesn't generate the image during template rendering. When the browser requests the image:
cache_path, and serves it.After the first request, the web server serves the static file directly on all subsequent requests. The default Nginx/Apache/IIS configurations handle this automatically — no additional rewrite rules are needed.
How It Works
GlideCachePathResolverpredicts the deterministic cache path for an image without generating it.HybridUrlBuilderbuilds clean URLs using the cache path, and stores a mapping in the Glide cache store so the controller knows how to generate the image later.GlideControllerhandles the PHP fallback: it checks if the file already exists (logging a debug message if server rewrite rules aren't configured), looks up the mapping, and generates the image on demand.php please glide:clearwork automatically — the cache disk is the public path, so existing cleanup logic handles it.Closes statamic/ideas#1420
Docs PR: statamic/docs#1899