Skip to content

Cache: rate limiting fixes, Redis/prefix safety, Cache::store(), Cache::tags() - #351

Merged
techmahedy merged 1 commit into
doppar:4.xfrom
techmahedy:cache
Oct 3, 2026
Merged

techmahedy merged 1 commit into
doppar:4.xfrom
techmahedy:cache

Conversation

@techmahedy

@techmahedy techmahedy commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Pull Request Checklist

Q A
Branch? 4.x
Bug fix? yes
New feature? yes
Deprecations? no
Issues -
License MIT

Summary

An audit of the cache code, with every suspected problem reproduced against the real classes (array, file and Redis) before changing anything. It found ten defects, mostly in the rate limiter and the Redis path, and two gaps: the config defines several stores but only the default was usable, and there were no tags. This PR fixes the defects and adds those features.

Bug fixes

Rate limiting (ThrottleRequests, RateLimiter)

  1. The handler ran for blocked requests. The middleware called $next($request) first and checked the limit afterwards, so a request over the limit still executed the controller (a throttled login was still evaluated). It also ran the handler a second time when the window had ended. With a limit of 2, the handler ran 5 times for 5 requests. The limit is now checked before $next, and only allowed requests reach it.
  2. All throttled routes shared one counter. The key was sha1(user id or ip), so traffic on a 100/min API route used up a 5/min login limit. The limit is now part of the key. Routes with the same throttle:N,M still share a counter per client.
  3. Retry-After / X-RateLimit-Reset were wrong. The reset timer was pushed back on every allowed hit while the counter kept its original expiry (it reported 3s with 1s left). The timer is now set when the window opens and not moved; X-RateLimit-Reset is derived from it.

CacheStore

  1. increment() / decrement() crashed on the array store. DateTime::createFromFormat('U', (string) $expiry) returns false for a float expiry, giving a TypeError in expiresAt(); rate limiting broke on that driver. Expiry is now restored without string parsing.
  2. stash() could return null for a value that had just expired, because it did has() then get(). It now reads once. See the stampede note below.
  3. forever() expired on Redis. The adapter applied its default lifetime (the shipped config has 'ttl' => 3600) to expiresAfter(null). Verified with TTL on the key. The default ttl is now applied by CacheStore, not the adapter.

CacheLauncher

  1. An empty CACHE_PREFIX made Cache::clear() flush the whole Redis database (Symfony clears everything when namespace and prefix are both empty). The prefix is now never empty; it falls back to doppar_cache_.
  2. A prefix with a space crashed the file store (APP_NAME="My App"): the filesystem adapter only accepts [-+_.A-Za-z0-9]. Other characters are replaced with _.
  3. A misspelled store name threw a TypeError (null passed to createCustomAdapter(array)); it now throws Cache store [x] is not defined., and an unsupported driver gets its own message.
  4. Redis options.parameters (password, database) were silently ignored; only the connection URL was read. They are now used, and the URL still takes precedence. A URL username is passed to AUTH for ACL users.

New

API Behaviour
Cache::store('name') Use any store from caching.stores. Stores are built once; every store can reach the others. store() without a name returns the current store.
Cache::tags(string|array) Returns a TaggedCache with set, forever, get, has, pull, delete/forget, stash, stashForever and flush(). Built on Symfony's TagAwareAdapter in its own sub-namespace. flush() removes items carrying any of the tags.
Cache::pull($key, $default), Cache::missing($key) Read-and-delete, and the inverse of has().
cache:clear --store=name Clear one configured store without deleting the framework cache files.
Store options ttl, prefix Per-store default ttl and key prefix in caching.stores.*.

Tag semantics worth reviewing:

  • Tagged items live apart from plain ones: read them through tags(), not get().
  • Tags group items for flushing and do not scope keys: the same key under two tags is one item (documented on tags()).
  • Cache::clear() also clears tagged items.
  • tags() throws This cache store does not support tags. if the adapter has no NamespacedPoolInterface (all built-in adapters do).

stash / stashForever / stashWhen now call the adapter's get($key, $callback) when it supports it, so the built-in adapters run the callback once for concurrent misses (a per-server lock; not distributed).

Behaviour changes to be aware of

  • Cache keys move once. An empty or odd prefix is rewritten, so existing entries under the old key are simply missed. Nothing is deleted.
  • Rate-limit counters reset once. The key now includes the limit.
  • Redis default ttl moved from the adapter to the store. set() / setMultiple() / add() / stash() still get the configured ttl; forever() and stashForever() no longer do (that was the bug). Custom adapters that carry their own default lifetime are unaffected.
  • Redis options.parameters.database is now honoured. The shipped config says env('REDIS_DB', 1), so an install that never set REDIS_DB moves its cache from database 0 to 1. An unset or empty value of the form REDIS_DB = "" stays on 0.
  • ThrottleRequests: buildResponse() (protected) was replaced by rejectIfStillLimited(), and resolveRequestSignature() takes the limit as two extra optional arguments. A subclass overriding either needs updating.
  • Two RateLimiterTest cases asserted the old timer reset on every hit; they now assert that an open window's timer is not touched.

Files

  • Cache/CacheStore.php, new Cache/TaggedCache.php, Cache/RateLimiter.php
  • Launchers/CacheLauncher.php
  • Middleware/ThrottleRequests.php
  • Console/Commands/ClearCacheCommand.php, Support/Facades/Cache.php (docblock)

Checklist

  • Tests have been added or updated
  • Documentation has been updated
  • Code follows the project coding standards
  • All tests pass locally

@techmahedy
techmahedy requested a review from rrr63 October 3, 2026 05:53
@techmahedy techmahedy added bug Something isn't working enhancement New feature or request feat new feature labels Oct 3, 2026
@techmahedy
techmahedy merged commit 7035047 into doppar:4.x Oct 3, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request feat new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant