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 Patel Namraa)

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 Patel Namraa, 110 minutes ago

Description: modified (diff)

comment:2 by Patel Namraa, 97 minutes ago

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")   -> None

The test is intentionally failing on the current implementation, before applying the proposed fix.

Running:

python tests/runtests.py cache.tests --parallel=1

currently produces:

FAIL: test_touch_expired (cache.tests.DBCacheTests.test_touch_expired)

AssertionError: True is not False

The current implementation returns True from DatabaseCache.touch() for the expired entry, while the regression test expects False.

The test also reproduces the same failure for DBCacheWithTimeZoneTests.

Only tests/cache/tests.py was modified; no production code has been changed.

Note: See TracTickets for help on using tickets.
Back to Top