Opened 113 minutes ago
Last modified 97 minutes ago
#37439 assigned Bug
DatabaseCache.touch() can revive expired cache entries
| 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 (last modified by )
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 existing cache test suite passes successfully on the current codebase.
python tests/runtests.py cache
Result:
- 696 tests passed.
- 211 tests were skipped, primarily due to unavailable optional dependencies.
- No tests failed.
This confirms that the existing cache test suite is passing before applying the fix.
The existing BaseCacheTests.test_touch does not currently cover touching an already-expired key, so this behavior is not detected by the existing test suite.
Change History (2)
comment:1 by , 110 minutes ago
| Description: | modified (diff) |
|---|
Added a regression test for this behavior in
BaseCacheTests.test_touch_expired.The test verifies that an expired cache entry cannot be touched:
cache.touch("expire_key") -> False cache.get("expire_key") -> NoneThe test is intentionally failing on the current implementation, before applying the proposed fix.
Running:
currently produces:
The current implementation returns
TruefromDatabaseCache.touch()for the expired entry, while the regression test expectsFalse.The test also reproduces the same failure for
DBCacheWithTimeZoneTests.Only
tests/cache/tests.pywas modified; no production code has been changed.