Opened 4 hours ago
Last modified 4 hours ago
#37439 assigned Bug
DatabaseCache.touch() can revive expired cache entries — at Initial Version
| Reported by: | Patel Namraa | Owned by: | Patel Namraa |
|---|---|---|---|
| Component: | Core (Cache system) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Patel Namraa | Triage Stage: | Unreviewed |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
DatabaseCache.touch() can successfully extend the expiry of a cache entry that has already expired, as long as its database row has not yet been culled.
This is inconsistent with the behavior of the other built-in cache backends, where touch() returns False for an expired entry.
Steps to reproduce
Using DatabaseCache:
from django.core.cache import caches
import time
cache = caches["default"]
cache.set("expired-key", "stale-value", timeout=1)
time.sleep(2)
print(cache.touch("expired-key", timeout=60))
print(cache.get("expired-key"))
This currently produces:
True "stale-value"
For comparison, LocMemCache returns:
False None
The important part of the reproduction is that touch() must be called before get(). DatabaseCache.get() removes expired rows, so calling get() first would remove the row and hide the issue.
Actual behavior
If a cache entry has expired but its row is still present in the database, DatabaseCache.touch() updates the expiry time and returns True.
The previously expired value can then be retrieved again with get().
The database cache does not immediately remove every expired row. Expired rows can therefore remain in the database until they are encountered by an operation that removes them.
Expected behavior
DatabaseCache.touch() should return False when the cache entry has already expired and should not extend its expiry.
This is consistent with the behavior of the other built-in cache backends.
Cause
DatabaseCache._base_set() already checks the expiry time for the add operation, but the touch branch only checks whether the database row exists.
It does not check whether current_expires is already in the past.
As a result, an expired database row is treated as a live cache entry by touch().
Regression test
A regression assertion has been added to BaseCacheTests.test_touch to verify that an expired entry cannot be touched.
The test checks that:
cache.touch(expired_key) == False
and that the expired value is not subsequently available:
cache.get(expired_key) == None
The test calls touch() before get() because DatabaseCache.get() removes expired rows.
Proposed fix
The touch path in DatabaseCache._base_set() can reuse the existing expiry comparison used by the add path and return False when current_expires < now.
The change is limited to expired entries and does not change the behavior of touch() for non-expired entries.
Testing
The regression test passes for the database, locmem, and file-based cache backends with the fix applied.
The behavior was reproduced using SQLite.
The full cache test suite has not yet been run. Before submitting a patch, the following should be run:
./runtests.py cache
The database backend has currently only been tested with SQLite; PostgreSQL, MySQL, and Oracle have not been tested yet.
A patch containing the fix and regression test is attached to this ticket.