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.

Change History (0)

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