From ec4efc1db8bd083366efe3ee7df572ab4b305ff6 Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 17:28:26 +0700 Subject: [PATCH 1/6] feat: process image queue --- app/Actions/TransformImageAction.php | 181 +++++++++++++++++-- app/Jobs/ProcessImageTransform.php | 63 +++++++ config/image-transform-url.php | 54 ++++++ tests/Feature/AllowedSizesAndFormatsTest.php | 151 ++++++++++++++++ tests/Feature/AnimatedWebpTransformTest.php | 15 +- tests/Feature/AsyncTransformTest.php | 129 +++++++++++++ 6 files changed, 579 insertions(+), 14 deletions(-) create mode 100644 app/Jobs/ProcessImageTransform.php create mode 100644 tests/Feature/AllowedSizesAndFormatsTest.php create mode 100644 tests/Feature/AsyncTransformTest.php diff --git a/app/Actions/TransformImageAction.php b/app/Actions/TransformImageAction.php index b96ba5c..2cdbc65 100644 --- a/app/Actions/TransformImageAction.php +++ b/app/Actions/TransformImageAction.php @@ -7,6 +7,7 @@ use AceOfAces\LaravelImageTransformUrl\Actions\TransformImageAction as BaseTransformImageAction; use AceOfAces\LaravelImageTransformUrl\ValueObjects\ImageResult; use AceOfAces\LaravelImageTransformUrl\ValueObjects\ImageSource; +use App\Jobs\ProcessImageTransform; use Illuminate\Http\Exceptions\HttpResponseException; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Cache; @@ -56,11 +57,44 @@ public function handle(?string $ip, ?string $pathPrefix, string $options, ?strin // $pathPrefix/$path by reference — so the cache key below matches the // vendor's writer, including on the default (no-prefix) route. $source = $this->handlePath($pathPrefix, $path); + $options = $this->normalizeOptions($options); if ($cached = $this->servedFromCache($pathPrefix, $path, $options)) { return $cached; // disk-native cache read — works for S3 (the vendor's File:: read does not) } + // When image is not optimized, dispatch to the queue and return a temporary redirect + if (config()->boolean('image-transform-url.async.enabled')) { + // Prior permanent failure — serve the long-cache redirect, do not re-dispatch. + if (Cache::has($this->failureSentinelKey($pathPrefix, $path, $options))) { + $this->redirectToOriginal($source); + } + + $this->guardAnimatedFrames($ip, $source, (string) $path); // over-cap → permanent redirect + + // Rate-limit by IP+path (not options) BEFORE dispatch. width/height/ + // quality are clamped to a fixed whitelist above (normalizeOptions()), + // so a straightforward param-enumeration attack no longer forks the + // dedup key — but the whitelist is config-driven (an empty `sizes`/ + // `qualities` disables clamping) and this is the only per-request + // throttle left on the miss path, so keep it as defense-in-depth — + // the same protection the vendor gave every synchronous transform. + if ( + config()->boolean('image-transform-url.rate_limit.enabled') && + ! in_array(App::environment(), config()->array('image-transform-url.rate_limit.disabled_for_environments'), true) + ) { + $this->rateLimit($ip, $path); + } + + ProcessImageTransform::dispatch($pathPrefix, $path, $options); // deduped by ShouldBeUnique + + // TEMPORARY redirect — short TTL so the CDN re-checks and picks up the HIT. + $this->redirectToOriginal($source, [ + 'Cache-Control' => 'public, max-age='.config()->integer('image-transform-url.async.pending_redirect_max_age'), + ]); + } + + // If image is optimized, show the image $this->guardAnimatedFrames($ip, $source, (string) $path); // may throttle + redirect return parent::handle($ip, $pathPrefix, $options, $path); @@ -69,16 +103,62 @@ public function handle(?string $ip, ?string $pathPrefix, string $options, ?strin } catch (HttpExceptionInterface $e) { throw $e; // preserve HTTP control-flow (404 not-found, 429 rate-limit, etc.) } catch (DecoderException|EncoderException|NotSupportedException|\ImagickException $e) { - // Decode/encode failure — serve the original instead of a 500. Intervention - // wraps decode faults in DecoderException, but its Imagick encoders throw - // ImagickException bare (e.g. CacheResourcesExhausted under a policy.xml / - // memory limit), so catch that too. ImagickException only originates from - // Imagick ops, so genuine faults (S3, config, TypeError) still surface as 5xx. + // Decode/encode failure — serve the original instead of a 500. report($e); $this->redirectToOriginal($source); // throws HttpResponseException } } + /** + * App-level abuse guard: canonicalize width/height to the nearest allowed + * size and reject a non-whitelisted explicit format, before anything else + * reads/writes the cache or dispatches a job. Re-serializes with sorted + * keys so option order can't fork the cache/dedup key either (e.g. + * `format=webp,width=200` and `width=200,format=webp` collapse to one). + */ + protected function normalizeOptions(string $rawOptions): string + { + $options = static::parseOptions($rawOptions); + + if (array_key_exists('width', $options)) { + $options['width'] = $this->nearestAllowedValue((int) $options['width'], config()->array('image-transform-url.sizes')); + } + + if (array_key_exists('height', $options)) { + $options['height'] = $this->nearestAllowedValue((int) $options['height'], config()->array('image-transform-url.sizes')); + } + + if (array_key_exists('quality', $options)) { + $options['quality'] = $this->nearestAllowedValue((int) $options['quality'], config()->array('image-transform-url.qualities')); + } + + if (array_key_exists('format', $options)) { + abort_unless( + in_array($options['format'], config()->array('image-transform-url.allowed_formats'), true), + 404, + ); + } + + ksort($options); + + return collect($options)->map(fn ($value, $key) => "{$key}={$value}")->implode(','); + } + + /** + * Round a requested width/height/quality to the nearest value in the given + * whitelist, bounding cache/queue cardinality per source image (an + * enumerated width=101, width=102, ... all collapse onto one entry). An + * empty whitelist disables the guard — the raw value passes through. + */ + protected function nearestAllowedValue(int $value, array $whitelist): int + { + if ($whitelist === []) { + return $value; + } + + return collect($whitelist)->sortBy(fn (int $allowed) => abs($allowed - $value))->first(); + } + /** * Redirect animations with more than the configured frame count straight to * the original, before Imagick decodes (and coalesces) them into GBs of RAM. @@ -218,6 +298,80 @@ protected function storeCachedImage(?string $pathPrefix, ?string $path, array $o } } + /** + * Background half of the async miss path (called from ProcessImageTransform). + * Reuses the vendor pipeline (DRY — no re-implementing Intervention), then + * stores the result explicitly and exactly once from the returned bytes. + */ + public function processAndCache(?string $pathPrefix, ?string $path, string $rawOptions): void + { + // Trusted internal call — the rate limiter is a public-abuse guard, and the + // parent's own cache read/deferred-store must not run (we store explicitly + // below). A persistent `queue:work` process reuses the same booted app + // across many jobs (config is NOT reset between them), so these overrides + // MUST be restored — otherwise the first async job permanently disables + // caching/rate-limiting for every request that process (or, under the + // `sync` queue driver, the current request/test) handles afterwards. + $originalRateLimit = config()->boolean('image-transform-url.rate_limit.enabled'); + $originalCacheEnabled = config()->boolean('image-transform-url.cache.enabled'); + + config()->set('image-transform-url.rate_limit.enabled', false); + config()->set('image-transform-url.cache.enabled', false); + + try { + $result = parent::handle(null, $pathPrefix, $rawOptions, $path); + + $this->storeResultBytes($pathPrefix, $path, $rawOptions, $result); + } finally { + config()->set('image-transform-url.rate_limit.enabled', $originalRateLimit); + config()->set('image-transform-url.cache.enabled', $originalCacheEnabled); + } + } + + /** + * Store the transform from an ImageResult's raw bytes + */ + protected function storeResultBytes(?string $pathPrefix, ?string $path, string $rawOptions, ImageResult $result): void + { + $options = static::parseOptions($rawOptions); + $diskName = config()->string('image-transform-url.cache.disk'); + + Storage::disk($diskName)->put($this->getCacheEndPath($pathPrefix, $path, $options), $result->content); + + Cache::put( + key: 'image-transform-url:'.$this->getCachePath($pathPrefix, $path, $options), + value: true, + ttl: config()->integer('image-transform-url.cache.lifetime'), + ); + + if (config("filesystems.disks.{$diskName}.driver") !== 's3') { + $this->manageCacheSize(); + } + } + + /** + * Cache key for the permanent-failure sentinel written by the job's failed(). + */ + protected function failureSentinelKey(?string $pathPrefix, ?string $path, string $rawOptions): string + { + $options = static::parseOptions($rawOptions); + + return 'image-transform-url:failed:'.$this->getCacheEndPath($pathPrefix, $path, $options); + } + + /** + * Mark a transform as permanently failed so the request path stops + * re-dispatching and serves the long-cache permanent redirect instead. + */ + public function markTransformFailed(?string $pathPrefix, ?string $path, string $rawOptions): void + { + Cache::put( + $this->failureSentinelKey($pathPrefix, $path, $rawOptions), + true, + config()->integer('image-transform-url.async.failed_lifetime'), + ); + } + /** * Read the raw source bytes for a disk- or local-based source. */ @@ -233,19 +387,24 @@ protected function readSourceBytes(ImageSource $source): string * an image (and the app never streams a broken/huge file through PHP). Takes * the already-resolved source (no re-resolve / extra metadata calls). The * bucket is public (see README/config), so a plain url() is correct and stays - * CDN-cacheable; the configured cache headers let the CDN absorb it. Only disk - * sources have a public URL — a local source has none, so it 404s honestly - * rather than emitting a malformed /storage//abs/path redirect. Throws, so the - * caller never falls through. + * CDN-cacheable. Only disk sources have a public URL — a local source has + * none, so it 404s honestly rather than emitting a malformed /storage//abs/path + * redirect. Throws, so the caller never falls through. + * + * $headers defaults to the permanent (30-day) config headers — used for the + * frame-guard/failure/sentinel redirects, where the source will never + * transform. The async-miss path passes a short-lived header set instead, so + * the CDN re-checks origin once the queued transform finishes. */ - protected function redirectToOriginal(?ImageSource $source): never + protected function redirectToOriginal(?ImageSource $source, ?array $headers = null): never { abort_unless($source?->type === 'disk', 404); + $headers ??= config()->array('image-transform-url.headers'); $url = Storage::disk((string) $source->disk)->url($source->path); throw new HttpResponseException( - redirect()->away($url)->withHeaders(config()->array('image-transform-url.headers')) + redirect()->away($url)->withHeaders($headers) ); } } diff --git a/app/Jobs/ProcessImageTransform.php b/app/Jobs/ProcessImageTransform.php new file mode 100644 index 0000000..f854a06 --- /dev/null +++ b/app/Jobs/ProcessImageTransform.php @@ -0,0 +1,63 @@ +onQueue(config()->string('image-transform-url.async.queue')); + if ($conn = config('image-transform-url.async.connection')) { + $this->onConnection($conn); + } + } + + /** + * Dedup: at most one job per transform per unique window. + */ + public function uniqueId(): string + { + return ($this->pathPrefix ?? '').'|'.$this->path.'|'.$this->options; + } + + public function uniqueFor(): int + { + return config()->integer('image-transform-url.async.unique_for'); + } + + public function handle(TransformImageAction $action): void + { + $action->processAndCache($this->pathPrefix, $this->path, $this->options); + } + + /** + * Permanent-failure sentinel — request path stops re-dispatching, serves + * the long-cache permanent redirect instead (matches today's decode-failure UX). + */ + public function failed(?Throwable $e): void + { + app(TransformImageAction::class)->markTransformFailed( + $this->pathPrefix, $this->path, $this->options, + ); + } +} diff --git a/config/image-transform-url.php b/config/image-transform-url.php index f220826..602bb1c 100644 --- a/config/image-transform-url.php +++ b/config/image-transform-url.php @@ -177,4 +177,58 @@ */ 'max_animated_frames' => (int) env('IMAGE_TRANSFORM_MAX_ANIMATED_FRAMES', 30), + + /* + |-------------------------------------------------------------------------- + | Allowed Sizes + Formats (app extension) + |-------------------------------------------------------------------------- + | + | Without a whitelist, width/height accept ANY positive integer — a client + | can enumerate width=101, width=102, width=103... and each one is a + | distinct cache entry (and, on the async path, a distinct queued transform + | job), so the request-side rate limiter is the only thing bounding that. + | Restricting to a fixed set of sizes bounds cache/queue cardinality per + | source image outright: an out-of-list width/height is rounded to the + | NEAREST allowed value (not rejected) before the cache key is computed, so + | e.g. width=203 and width=210 both resolve to the same 200px cache entry. + | Set to an empty array to disable (any width/height passes through as-is). + | + | `qualities` applies the same nearest-value clamp to `quality=` — otherwise + | quality=1..100 is a 100-value enumeration space per size/format combo that + | the size whitelist above does nothing to bound. + | + | `allowed_formats` similarly restricts the `format=` option: an explicit + | format NOT in this list 404s. Omitting `format=` entirely is unaffected + | (the vendor still defaults to the source's own mime type in that case). + | + */ + + 'sizes' => env('IMAGE_TRANSFORM_SIZES', [200, 400, 600, 800, 1000, 1280, 1440, 1920]), + + 'qualities' => env('IMAGE_TRANSFORM_QUALITIES', [50, 70, 90, 100]), + + 'allowed_formats' => env('IMAGE_TRANSFORM_ALLOWED_FORMATS', ['webp']), + + /* + |-------------------------------------------------------------------------- + | Async Transform (app extension) + |-------------------------------------------------------------------------- + | On a cache miss, redirect to the original (short TTL) and process the + | transform on the queue instead of blocking the request. Disable to fall + | back to the synchronous transform. + */ + 'async' => [ + 'enabled' => env('IMAGE_TRANSFORM_ASYNC_ENABLED', true), + // Queue name + connection for the transform job (null connection = default). + 'queue' => env('IMAGE_TRANSFORM_QUEUE', 'default'), + 'connection' => env('IMAGE_TRANSFORM_QUEUE_CONNECTION', null), + // CDN max-age (seconds) on the TEMPORARY miss redirect. MUST stay small so the + // CDN re-checks origin and picks up the cached transform once the job finishes. + 'pending_redirect_max_age' => (int) env('IMAGE_TRANSFORM_PENDING_MAX_AGE', 10), + // ShouldBeUnique lock window (seconds) — dedups dispatch during processing. + 'unique_for' => (int) env('IMAGE_TRANSFORM_JOB_UNIQUE_FOR', 300), + // TTL (seconds) of the permanent-failure sentinel written by the job's failed(). + // Kept moderate so a transient infra failure eventually retries. + 'failed_lifetime' => (int) env('IMAGE_TRANSFORM_FAILED_LIFETIME', 60 * 60 * 24), + ], ]; diff --git a/tests/Feature/AllowedSizesAndFormatsTest.php b/tests/Feature/AllowedSizesAndFormatsTest.php new file mode 100644 index 0000000..27dc224 --- /dev/null +++ b/tests/Feature/AllowedSizesAndFormatsTest.php @@ -0,0 +1,151 @@ +set('image-transform-url.cache.enabled', true); + config()->set('image-transform-url.cache.disk', 's3-cache'); + config()->set('image-transform-url.rate_limit.enabled', false); + config()->set('image-transform-url.max_animated_frames', 30); + config()->set('image-transform-url.async.enabled', true); + config()->set('image-transform-url.sizes', [200, 400, 600, 800, 1000, 1280, 1440, 1920]); + config()->set('image-transform-url.qualities', [60, 75, 90, 100]); + config()->set('image-transform-url.allowed_formats', ['webp']); +}); + +it('clamps an out-of-whitelist quality to the nearest allowed value', function () { + putFixture('static.png', 'dir/clampq.png'); + + $this->get('/production/quality=70,format=webp/dir/clampq.png')->assertRedirect(); + + // 70 is closer to 75 than to 60 -> must clamp to 75, not pass through raw. + $parsed = ['format' => 'webp', 'quality' => 75]; + $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/clampq.png'; + Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); + Storage::disk('s3-cache')->put($endPath, 'CLAMPED-75'); + + $res = $this->get('/production/quality=70,format=webp/dir/clampq.png'); + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); + expect($res->getContent())->toBe('CLAMPED-75'); +}); + +it('collapses quality enumeration onto one dispatched job instead of one per value', function () { + Queue::fake(); + putFixture('static.png', 'dir/enumq.png'); + + // Both round to the nearest allowed quality (90) -> must dedup to one job. + $this->get('/production/quality=86,format=webp/dir/enumq.png')->assertRedirect(); + $this->get('/production/quality=93,format=webp/dir/enumq.png')->assertRedirect(); + + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('clamps an out-of-whitelist width to the nearest allowed size', function () { + putFixture('static.png', 'dir/clampw.png'); + + $this->get('/production/width=210,format=webp/dir/clampw.png')->assertRedirect(); + + // The clamped (200px) cache entry must exist — not one keyed by the raw 210. + $parsed = ['format' => 'webp', 'width' => 200]; // alphabetical — matches normalizeOptions()'s ksort() + $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/clampw.png'; + Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); + Storage::disk('s3-cache')->put($endPath, 'CLAMPED-200'); + + $res = $this->get('/production/width=210,format=webp/dir/clampw.png'); + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); + expect($res->getContent())->toBe('CLAMPED-200'); +}); + +it('clamps an out-of-whitelist height to the nearest allowed size', function () { + putFixture('static.png', 'dir/clamph.png'); + + $this->get('/production/height=395,format=webp/dir/clamph.png')->assertRedirect(); + + $parsed = ['format' => 'webp', 'height' => 400]; + $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/clamph.png'; + Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); + Storage::disk('s3-cache')->put($endPath, 'CLAMPED-400'); + + $res = $this->get('/production/height=395,format=webp/dir/clamph.png'); + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); +}); + +it('collapses width enumeration onto one dispatched job instead of one per value', function () { + Queue::fake(); + putFixture('static.png', 'dir/enum.png'); + + // Different raw widths that both round to the nearest allowed size (200) must + // dedup to a single job — the whole point of the whitelist is bounding this. + $this->get('/production/width=203,format=webp/dir/enum.png')->assertRedirect(); + $this->get('/production/width=210,format=webp/dir/enum.png')->assertRedirect(); + + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('ignores option order when computing the cache/dedup key', function () { + Queue::fake(); + putFixture('static.png', 'dir/order.png'); + + $this->get('/production/width=200,format=webp/dir/order.png')->assertRedirect(); + $this->get('/production/format=webp,width=200/dir/order.png')->assertRedirect(); + + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('rejects an explicit non-whitelisted format', function () { + putFixture('static.png', 'dir/badformat.png'); + + $this->get('/production/width=200,format=png/dir/badformat.png')->assertNotFound(); +}); + +it('allows an explicit whitelisted format', function () { + Queue::fake(); + putFixture('static.png', 'dir/goodformat.png'); + + $this->get('/production/width=200,format=webp/dir/goodformat.png')->assertRedirect(); + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('leaves an omitted format unaffected by the whitelist', function () { + Queue::fake(); + putFixture('static.png', 'dir/noformat.png'); + + // No format= param at all -> not gated by allowed_formats (vendor defaults + // to the source's own mime instead). + $this->get('/production/width=200/dir/noformat.png')->assertRedirect(); + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('passes width/height through unclamped when the whitelist is disabled', function () { + config()->set('image-transform-url.sizes', []); + Queue::fake(); + putFixture('static.png', 'dir/nosizeguard.png'); + + $this->get('/production/width=57,format=webp/dir/nosizeguard.png')->assertRedirect(); + + $parsed = ['format' => 'webp', 'width' => 57]; // alphabetical — matches normalizeOptions()'s ksort() + $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/nosizeguard.png'; + Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); + Storage::disk('s3-cache')->put($endPath, 'RAW-57'); + + $res = $this->get('/production/width=57,format=webp/dir/nosizeguard.png'); + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); +}); diff --git a/tests/Feature/AnimatedWebpTransformTest.php b/tests/Feature/AnimatedWebpTransformTest.php index 61ecfbd..11fb7e5 100644 --- a/tests/Feature/AnimatedWebpTransformTest.php +++ b/tests/Feature/AnimatedWebpTransformTest.php @@ -19,6 +19,15 @@ config()->set('image-transform-url.cache.enabled', false); config()->set('image-transform-url.rate_limit.enabled', false); config()->set('image-transform-url.max_animated_frames', 30); + // These tests assert transformed content synchronously; the async miss path + // (default ON) redirects instead — see tests/Feature/AsyncTransformTest.php. + config()->set('image-transform-url.async.enabled', false); + // These tests use arbitrary widths and multiple formats (gif/png/webp) to + // exercise decode/frame-guard behavior, not the size/format whitelist — + // see tests/Feature/AllowedSizesAndFormatsTest.php for that guard. + config()->set('image-transform-url.sizes', []); + config()->set('image-transform-url.qualities', []); + config()->set('image-transform-url.allowed_formats', ['webp', 'gif', 'png']); }); function putFixture(string $name, string $storedAs): void @@ -187,7 +196,7 @@ public function read(...$a): object config()->set('image-transform-url.cache.enabled', true); config()->set('image-transform-url.cache.disk', 's3-cache'); - $parsed = ['width' => 64, 'format' => 'webp']; + $parsed = ['format' => 'webp', 'width' => 64]; // alphabetical — matches normalizeOptions()'s ksort() $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/a.webp'; Storage::disk('s3-cache')->put($endPath, 'SEEDED-DEFAULT-ROUTE'); Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); @@ -255,7 +264,7 @@ public function read(...$a): object config()->set('image-transform-url.cache.enabled', true); config()->set('image-transform-url.cache.disk', 's3-cache'); - $parsed = ['width' => 64, 'format' => 'webp']; + $parsed = ['format' => 'webp', 'width' => 64]; // alphabetical — matches normalizeOptions()'s ksort() $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/gone.webp'; Storage::disk('s3-cache')->put($endPath, 'STALE-BODY'); Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); @@ -289,7 +298,7 @@ public function read(...$a): object config()->set('image-transform-url.cache.disk', 's3-cache'); config()->set('image-transform-url.max_animated_frames', 2); // would redirect the 3-frame file - $parsed = ['width' => 64, 'format' => 'webp']; + $parsed = ['format' => 'webp', 'width' => 64]; // alphabetical — matches normalizeOptions()'s ksort() $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/a.webp'; Storage::disk('s3-cache')->put($endPath, 'SEEDED-CACHE-BODY'); Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); diff --git a/tests/Feature/AsyncTransformTest.php b/tests/Feature/AsyncTransformTest.php new file mode 100644 index 0000000..373fa22 --- /dev/null +++ b/tests/Feature/AsyncTransformTest.php @@ -0,0 +1,129 @@ +set('image-transform-url.cache.enabled', true); + config()->set('image-transform-url.cache.disk', 's3-cache'); + config()->set('image-transform-url.rate_limit.enabled', false); + config()->set('image-transform-url.max_animated_frames', 30); + config()->set('image-transform-url.async.enabled', true); + // These tests use arbitrary widths to exercise dispatch/dedup/rate-limit + // mechanics, not the size whitelist — see AllowedSizesAndFormatsTest.php. + config()->set('image-transform-url.sizes', []); + config()->set('image-transform-url.qualities', []); +}); + +it('dispatches the transform job and redirects with a short-lived cache header on a miss', function () { + Queue::fake(); + putFixture('static.png', 'dir/s.png'); + + $res = $this->get('/production/width=32,format=webp/dir/s.png'); + + $res->assertRedirect(); + expect($res->headers->get('Cache-Control'))->toContain('max-age=10'); + expect($res->headers->get('Cache-Control'))->not->toContain('2592000'); + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('dedups the dispatch for identical concurrent requests', function () { + Queue::fake(); + putFixture('static.png', 'dir/dedup.png'); + + $this->get('/production/width=32,format=webp/dir/dedup.png')->assertRedirect(); + $this->get('/production/width=32,format=webp/dir/dedup.png')->assertRedirect(); + + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('does not dispatch on a cache hit', function () { + Queue::fake(); + putFixture('static.png', 'dir/hit.png'); + + // Seed the cache directly (bypass the job) then request. + $parsed = ['format' => 'webp', 'width' => 32]; // alphabetical — matches normalizeOptions()'s ksort() + $endPath = '_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/hit.png'; + Storage::disk('s3-cache')->put($endPath, 'SEEDED'); + Cache::put('image-transform-url:'.Storage::disk('s3-cache')->path($endPath), true, 3600); + + $res = $this->get('/production/width=32,format=webp/dir/hit.png'); + + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); + Queue::assertNothingPushed(); +}); + +it('runs the job for real and populates the cache so the next request is a HIT', function () { + putFixture('static.png', 'dir/job.png'); + + $this->get('/production/width=32,format=webp/dir/job.png')->assertRedirect(); + + $res = $this->get('/production/width=32,format=webp/dir/job.png'); + $res->assertOk(); + expect($res->headers->get('X-Cache'))->toBe('HIT'); +}); + +it('writes a failure sentinel and serves the permanent redirect without re-dispatching', function () { + // Undecodable bytes under an allowed mime -> job's handle() throws -> failed() sentinel. + $header = substr((string) file_get_contents(base_path('tests/fixtures/animated-tiny.webp')), 0, 16); + Storage::disk('s3')->put('dir/broken.webp', $header.str_repeat("\x00", 256)); + + $this->get('/production/width=64,format=webp/dir/broken.webp')->assertRedirect(); + + Queue::fake(); + $res = $this->get('/production/width=64,format=webp/dir/broken.webp'); + + $res->assertRedirect(); + expect($res->headers->get('Cache-Control'))->toContain('max-age=2592000'); + Queue::assertNothingPushed(); +}); + +it('still redirects animations over the frame cap without dispatching', function () { + config()->set('image-transform-url.max_animated_frames', 2); + Queue::fake(); + putFixture('animated-tiny.webp', 'dir/big.webp'); + + $res = $this->get('/production/width=64,format=webp/dir/big.webp'); + + $res->assertRedirect(); + expect($res->headers->get('Cache-Control'))->toContain('max-age=2592000'); + Queue::assertNothingPushed(); +}); + +it('rate-limits the miss path by ip+path, closing the param-enumeration dedup bypass', function () { + Queue::fake(); + config()->set('image-transform-url.rate_limit.enabled', true); + config()->set('image-transform-url.rate_limit.disabled_for_environments', []); + config()->set('image-transform-url.rate_limit.max_attempts', 1); + putFixture('static.png', 'dir/spam.png'); + + $this->get('/production/width=32,format=webp/dir/spam.png')->assertRedirect(); + // Different options -> distinct ShouldBeUnique dedup key, but the SAME ip+path + // rate-limit key -> must still be throttled instead of dispatching a 2nd job. + $this->get('/production/width=33,format=webp/dir/spam.png')->assertStatus(429); + + Queue::assertPushed(ProcessImageTransform::class, 1); +}); + +it('transforms synchronously when the async flag is off', function () { + config()->set('image-transform-url.async.enabled', false); + Queue::fake(); + putFixture('static.png', 'dir/sync.png'); + + $res = $this->get('/production/width=32,format=webp/dir/sync.png'); + + $res->assertOk(); + Queue::assertNothingPushed(); +}); From 42b0c613086b88cc5feee4c306c09540d1d0b941 Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 17:30:47 +0700 Subject: [PATCH 2/6] feat: update --- .env.example | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 12217ea..201c9ec 100644 --- a/.env.example +++ b/.env.example @@ -35,7 +35,7 @@ SESSION_DOMAIN=null BROADCAST_CONNECTION=log FILESYSTEM_DISK=local -QUEUE_CONNECTION=database +QUEUE_CONNECTION=redis CACHE_STORE=database # CACHE_PREFIX= @@ -85,4 +85,14 @@ IMAGE_TRANSFORM_MAX_ANIMATED_FRAMES=30 # cache disk with a bucket/S3 lifecycle rule instead. IMAGE_TRANSFORM_CACHE_DISK=local +# Async transform (see "Async processing" in README). On a cache miss the +# request 302s to the original (short TTL) and queues the transform instead of +# blocking on it. Requires a running `php artisan queue:work redis`. +IMAGE_TRANSFORM_ASYNC_ENABLED=true +# IMAGE_TRANSFORM_QUEUE=default +# IMAGE_TRANSFORM_QUEUE_CONNECTION= +# IMAGE_TRANSFORM_PENDING_MAX_AGE=10 +# IMAGE_TRANSFORM_JOB_UNIQUE_FOR=300 +# IMAGE_TRANSFORM_FAILED_LIFETIME=86400 + VITE_APP_NAME="${APP_NAME}" From 6e211f943b2531ddb79693e52d33c5b3433a5933 Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 19:15:24 +0700 Subject: [PATCH 3/6] chore: 60 seconds --- config/image-transform-url.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/image-transform-url.php b/config/image-transform-url.php index 602bb1c..f190b8e 100644 --- a/config/image-transform-url.php +++ b/config/image-transform-url.php @@ -224,7 +224,7 @@ 'connection' => env('IMAGE_TRANSFORM_QUEUE_CONNECTION', null), // CDN max-age (seconds) on the TEMPORARY miss redirect. MUST stay small so the // CDN re-checks origin and picks up the cached transform once the job finishes. - 'pending_redirect_max_age' => (int) env('IMAGE_TRANSFORM_PENDING_MAX_AGE', 10), + 'pending_redirect_max_age' => (int) env('IMAGE_TRANSFORM_PENDING_MAX_AGE', 60), // ShouldBeUnique lock window (seconds) — dedups dispatch during processing. 'unique_for' => (int) env('IMAGE_TRANSFORM_JOB_UNIQUE_FOR', 300), // TTL (seconds) of the permanent-failure sentinel written by the job's failed(). From f5c058b0ea45d07c74ca52e5cbb52a16639317fd Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 19:29:26 +0700 Subject: [PATCH 4/6] feat: update tests to reflect current rate limit --- .env.example | 7 ++-- README.md | 42 +++++++++++++++++++++++ app/Actions/TransformImageAction.php | 50 ++++++++++++++++++++-------- tests/Feature/AsyncTransformTest.php | 11 ++++-- 4 files changed, 91 insertions(+), 19 deletions(-) diff --git a/.env.example b/.env.example index 201c9ec..95af803 100644 --- a/.env.example +++ b/.env.example @@ -37,7 +37,10 @@ BROADCAST_CONNECTION=log FILESYSTEM_DISK=local QUEUE_CONNECTION=redis -CACHE_STORE=database +# redis (not database): the async transform's ShouldBeUnique dispatch lock and the +# cache flag / failure sentinel / frame memo all use the cache store. A fresh app +# ships no migrations, so `database` has no cache/cache_locks tables to write to. +CACHE_STORE=redis # CACHE_PREFIX= MEMCACHED_HOST=127.0.0.1 @@ -91,7 +94,7 @@ IMAGE_TRANSFORM_CACHE_DISK=local IMAGE_TRANSFORM_ASYNC_ENABLED=true # IMAGE_TRANSFORM_QUEUE=default # IMAGE_TRANSFORM_QUEUE_CONNECTION= -# IMAGE_TRANSFORM_PENDING_MAX_AGE=10 +# IMAGE_TRANSFORM_PENDING_MAX_AGE=60 # IMAGE_TRANSFORM_JOB_UNIQUE_FOR=300 # IMAGE_TRANSFORM_FAILED_LIFETIME=86400 diff --git a/README.md b/README.md index 8c653d1..88b78e3 100644 --- a/README.md +++ b/README.md @@ -21,5 +21,47 @@ Transformed images are cached on the disk set by `IMAGE_TRANSFORM_CACHE_DISK` (d Do **not** append `*`. S3 and R2 lifecycle filters match a *literal* key prefix; they do not support wildcards. A prefix of `_cache/image-transform-url/*` is taken literally, matches no cache object, and the rule silently expires nothing — leaving the bucket unbounded. To target the whole bucket instead, use an empty prefix, not `*`. +# Allowed sizes + formats + +Without a whitelist, `width=`/`height=` accept any positive integer, so a client can +enumerate `width=101`, `width=102`, `width=103`... — each one a distinct cache entry +(and, on the async path, a distinct queued job). `config/image-transform-url.php` → +`sizes` bounds this: a requested width/height is rounded to the **nearest** value in +the list (not rejected), so e.g. `width=203` and `width=210` both resolve to the same +200px cache/dedup entry. Default: `[200, 400, 600, 800, 1000, 1280, 1440, 1920]`. Set +to `[]` to disable. + +`qualities` applies the same nearest-value clamp to `quality=` (default: +`[60, 75, 90, 100]`) — otherwise `quality=1..100` is a 100-value enumeration space +per size/format combo that the size whitelist alone doesn't bound. + +`allowed_formats` restricts the `format=` option — an explicit format not in the list +404s (default: `['webp']` only). Omitting `format=` entirely is unaffected by this +whitelist; the vendor still defaults to the source's own mime type in that case. + +# Async processing + +On a cache **miss** the server does not transform inline. It **302s to the original** +(short-lived, `Cache-Control: public, max-age=60`) so the client gets an image +immediately, and queues the transform in the background. The next request for the +same URL hits the now-populated cache and gets the optimized image (200 `X-Cache: HIT`). + +- **Required:** a running worker — `php artisan queue:work redis --queue=default` + (use Supervisor/Horizon in production). Without a worker, misses keep redirecting + to the original and never optimize. +- `QUEUE_CONNECTION=redis` (phpredis extension required). Ensure a `failed_jobs` table + exists (`php artisan queue:failed-table && php artisan migrate`), **or** set + `QUEUE_FAILED_DRIVER=null` — the app's own failure sentinel does not need it. +- Recommended: `CACHE_STORE=redis` so the dispatch dedup lock (`ShouldBeUnique`) is + atomic and fast. +- On a job failure (undecodable source, encoder error), a permanent sentinel is + cached and the request path serves the long-cache (30-day) redirect instead of + re-dispatching — matching the synchronous decode-failure behavior. +- Tuning knobs (see `config/image-transform-url.php` → `async`): + `IMAGE_TRANSFORM_ASYNC_ENABLED` (kill-switch → synchronous transform), + `IMAGE_TRANSFORM_PENDING_MAX_AGE` (temporary redirect TTL), + `IMAGE_TRANSFORM_JOB_UNIQUE_FOR` (dispatch dedup window), + `IMAGE_TRANSFORM_FAILED_LIFETIME` (failure sentinel TTL). + # More documentation Please read [here](https://image-transform-url.julian.center/installation) diff --git a/app/Actions/TransformImageAction.php b/app/Actions/TransformImageAction.php index 2cdbc65..622fc56 100644 --- a/app/Actions/TransformImageAction.php +++ b/app/Actions/TransformImageAction.php @@ -13,6 +13,7 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Storage; use Intervention\Image\Exceptions\DecoderException; use Intervention\Image\Exceptions\EncoderException; @@ -72,22 +73,18 @@ public function handle(?string $ip, ?string $pathPrefix, string $options, ?strin $this->guardAnimatedFrames($ip, $source, (string) $path); // over-cap → permanent redirect - // Rate-limit by IP+path (not options) BEFORE dispatch. width/height/ - // quality are clamped to a fixed whitelist above (normalizeOptions()), - // so a straightforward param-enumeration attack no longer forks the - // dedup key — but the whitelist is config-driven (an empty `sizes`/ - // `qualities` disables clamping) and this is the only per-request - // throttle left on the miss path, so keep it as defense-in-depth — - // the same protection the vendor gave every synchronous transform. - if ( - config()->boolean('image-transform-url.rate_limit.enabled') && - ! in_array(App::environment(), config()->array('image-transform-url.rate_limit.disabled_for_environments'), true) - ) { - $this->rateLimit($ip, $path); + // Throttle only the DISPATCH, never the redirect. width/height/quality + // are clamped to a fixed whitelist above (normalizeOptions()), so a + // param-enumeration attack no longer forks the dedup key — but the + // whitelist is config-driven (empty `sizes`/`qualities` disables it), + // so keep an IP+path throttle as a queue-flood guard. Crucially it + // gates ONLY the enqueue: an over-limit miss still gets the temporary + // redirect below, so a cache miss ALWAYS serves an image (a hard 429 + // here would hand the client a broken image during the processing gap). + if ($this->dispatchRateLimitPassed($ip, (string) $path)) { + ProcessImageTransform::dispatch($pathPrefix, $path, $options); // deduped by ShouldBeUnique } - ProcessImageTransform::dispatch($pathPrefix, $path, $options); // deduped by ShouldBeUnique - // TEMPORARY redirect — short TTL so the CDN re-checks and picks up the HIT. $this->redirectToOriginal($source, [ 'Cache-Control' => 'public, max-age='.config()->integer('image-transform-url.async.pending_redirect_max_age'), @@ -159,6 +156,31 @@ protected function nearestAllowedValue(int $value, array $whitelist): int return collect($whitelist)->sortBy(fn (int $allowed) => abs($allowed - $value))->first(); } + /** + * Non-aborting dispatch throttle for the async miss path: returns whether the + * source may be enqueued on this request. Mirrors the vendor rate-limit KEY + * (image-transform-url:$ip:$path) so both share one bucket, but returns a bool + * instead of aborting 429 — the caller still serves the temporary redirect when + * this is false, so a miss never hands the client a broken image. Returns true + * when rate limiting is disabled or the current environment is exempt. + */ + protected function dispatchRateLimitPassed(?string $ip, string $path): bool + { + if ( + ! config()->boolean('image-transform-url.rate_limit.enabled') || + in_array(App::environment(), config()->array('image-transform-url.rate_limit.disabled_for_environments'), true) + ) { + return true; + } + + return (bool) RateLimiter::attempt( + key: 'image-transform-url:'.$ip.':'.$path, + maxAttempts: config()->integer('image-transform-url.rate_limit.max_attempts'), + callback: fn () => true, + decaySeconds: config()->integer('image-transform-url.rate_limit.decay_seconds'), + ); + } + /** * Redirect animations with more than the configured frame count straight to * the original, before Imagick decodes (and coalesces) them into GBs of RAM. diff --git a/tests/Feature/AsyncTransformTest.php b/tests/Feature/AsyncTransformTest.php index 373fa22..24b7783 100644 --- a/tests/Feature/AsyncTransformTest.php +++ b/tests/Feature/AsyncTransformTest.php @@ -28,6 +28,10 @@ it('dispatches the transform job and redirects with a short-lived cache header on a miss', function () { Queue::fake(); + // Pin the TTL the assertion checks so the test doesn't break when the config + // default changes — it verifies the miss redirect uses the CONFIGURED pending + // TTL (short), not the permanent 30-day header. + config()->set('image-transform-url.async.pending_redirect_max_age', 10); putFixture('static.png', 'dir/s.png'); $res = $this->get('/production/width=32,format=webp/dir/s.png'); @@ -102,7 +106,7 @@ Queue::assertNothingPushed(); }); -it('rate-limits the miss path by ip+path, closing the param-enumeration dedup bypass', function () { +it('throttles only the dispatch on the miss path — still redirects, never 429s', function () { Queue::fake(); config()->set('image-transform-url.rate_limit.enabled', true); config()->set('image-transform-url.rate_limit.disabled_for_environments', []); @@ -111,8 +115,9 @@ $this->get('/production/width=32,format=webp/dir/spam.png')->assertRedirect(); // Different options -> distinct ShouldBeUnique dedup key, but the SAME ip+path - // rate-limit key -> must still be throttled instead of dispatching a 2nd job. - $this->get('/production/width=33,format=webp/dir/spam.png')->assertStatus(429); + // rate-limit key -> the 2nd enqueue is throttled. The request MUST still get a + // redirect (a miss always serves an image); only the dispatch is suppressed. + $this->get('/production/width=33,format=webp/dir/spam.png')->assertRedirect(); Queue::assertPushed(ProcessImageTransform::class, 1); }); From b51c799043378bdb527498730c6dcc2c55c00cba Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 19:50:35 +0700 Subject: [PATCH 5/6] feat: update --- app/Jobs/ProcessImageTransform.php | 23 +++++++++++++++++++++-- tests/Feature/AsyncTransformTest.php | 16 ++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/app/Jobs/ProcessImageTransform.php b/app/Jobs/ProcessImageTransform.php index f854a06..4319909 100644 --- a/app/Jobs/ProcessImageTransform.php +++ b/app/Jobs/ProcessImageTransform.php @@ -8,6 +8,9 @@ use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; +use Intervention\Image\Exceptions\DecoderException; +use Intervention\Image\Exceptions\EncoderException; +use Intervention\Image\Exceptions\NotSupportedException; use Throwable; /** @@ -51,11 +54,27 @@ public function handle(TransformImageAction $action): void } /** - * Permanent-failure sentinel — request path stops re-dispatching, serves - * the long-cache permanent redirect instead (matches today's decode-failure UX). + * Sentinel ONLY a genuinely permanent fault — a decode/encode failure that + * re-running will never fix. The request path then stops re-dispatching and + * serves the long-cache permanent redirect instead (matches the synchronous + * decode-failure UX, and mirrors the swallowed types in TransformImageAction). + * + * A transient infra fault (S3 unreachable, network, memory pressure) is NOT + * sentineled: leaving no sentinel lets the next request re-dispatch so the + * transform can recover on its own, instead of serving unoptimized originals + * for the full failed_lifetime window over a momentary blip. */ public function failed(?Throwable $e): void { + $permanent = $e instanceof DecoderException + || $e instanceof EncoderException + || $e instanceof NotSupportedException + || $e instanceof \ImagickException; + + if (! $permanent) { + return; + } + app(TransformImageAction::class)->markTransformFailed( $this->pathPrefix, $this->path, $this->options, ); diff --git a/tests/Feature/AsyncTransformTest.php b/tests/Feature/AsyncTransformTest.php index 24b7783..0e51088 100644 --- a/tests/Feature/AsyncTransformTest.php +++ b/tests/Feature/AsyncTransformTest.php @@ -6,6 +6,7 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; +use Intervention\Image\Exceptions\DecoderException; /** * Coverage for the async miss path: a cache miss dispatches a background @@ -132,3 +133,18 @@ $res->assertOk(); Queue::assertNothingPushed(); }); + +it('sentinels a permanent decode failure but leaves a transient infra failure re-dispatchable', function () { + $job = new ProcessImageTransform('production', 'dir/x.png', 'format=webp,width=64'); + $parsed = ['format' => 'webp', 'width' => 64]; // alphabetical — matches parseOptions() + $sentinel = 'image-transform-url:failed:_cache/image-transform-url/production/'.md5(json_encode($parsed)).'_dir/x.png'; + + // Transient infra fault (S3/network) -> NO sentinel, so the next request can + // re-dispatch and recover instead of serving originals for failed_lifetime. + $job->failed(new RuntimeException('S3 timeout')); + expect(Cache::has($sentinel))->toBeFalse(); + + // Permanent decode fault -> sentinel written, request path stops re-dispatching. + $job->failed(new DecoderException('corrupt source')); + expect(Cache::has($sentinel))->toBeTrue(); +}); From 0e6cf7fae4e3f43d7f1116630931b60bc13291ff Mon Sep 17 00:00:00 2001 From: Chi Bao Date: Wed, 26 Aug 2026 20:18:34 +0700 Subject: [PATCH 6/6] feat: update --- .env.example | 2 +- config/image-transform-url.php | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index 95af803..a8c8839 100644 --- a/.env.example +++ b/.env.example @@ -96,6 +96,6 @@ IMAGE_TRANSFORM_ASYNC_ENABLED=true # IMAGE_TRANSFORM_QUEUE_CONNECTION= # IMAGE_TRANSFORM_PENDING_MAX_AGE=60 # IMAGE_TRANSFORM_JOB_UNIQUE_FOR=300 -# IMAGE_TRANSFORM_FAILED_LIFETIME=86400 +# IMAGE_TRANSFORM_FAILED_LIFETIME=3600 VITE_APP_NAME="${APP_NAME}" diff --git a/config/image-transform-url.php b/config/image-transform-url.php index f190b8e..fd93599 100644 --- a/config/image-transform-url.php +++ b/config/image-transform-url.php @@ -227,8 +227,9 @@ 'pending_redirect_max_age' => (int) env('IMAGE_TRANSFORM_PENDING_MAX_AGE', 60), // ShouldBeUnique lock window (seconds) — dedups dispatch during processing. 'unique_for' => (int) env('IMAGE_TRANSFORM_JOB_UNIQUE_FOR', 300), - // TTL (seconds) of the permanent-failure sentinel written by the job's failed(). - // Kept moderate so a transient infra failure eventually retries. - 'failed_lifetime' => (int) env('IMAGE_TRANSFORM_FAILED_LIFETIME', 60 * 60 * 24), + // TTL (seconds) of the permanent-failure sentinel written by the job's failed() + // on a decode/encode fault. 1h so a fixed/re-uploaded source recovers within the + // hour rather than serving the original for a full day. + 'failed_lifetime' => (int) env('IMAGE_TRANSFORM_FAILED_LIFETIME', 60 * 60), ], ];