From be782dae05cf711905018c25cabc1aeea42eef66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Afonso=20Janu=C3=A1rio?= Date: Fri, 4 Sep 2026 14:02:26 +0100 Subject: [PATCH] Let memoize's ignore set match a positional arg by name ignore={'session'} only worked when session was passed as a keyword. Call the same function with session passed positionally instead and diskcache tried to build a cache key that included the raw argument, which is exactly the footgun described in #240: a caller has to know whether they always call with keywords, or ignore has to spell out the positional index too. memoize() now grabs the wrapped function's parameter names once via inspect.signature and passes them through to args_to_key, which checks a positional argument's name against ignore in addition to its index. Keyword matching was already fine and is untouched. args_to_key's new arg_names parameter defaults to an empty tuple, so the two other callers (DjangoCache.memoize, the memoize_stampede recipe) keep their existing index-only behavior unless they're updated separately. Added a regression test using an unpicklable value for the ignored argument, so a caller passing it positionally raises immediately if it leaks into the key instead of silently caching under a wrong key. --- diskcache/core.py | 28 ++++++++++++++++++++++++---- tests/test_core.py | 14 ++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/diskcache/core.py b/diskcache/core.py index 7a3d23b..7fd7829 100644 --- a/diskcache/core.py +++ b/diskcache/core.py @@ -5,6 +5,7 @@ import contextlib as cl import errno import functools as ft +import inspect import io import json import os @@ -384,7 +385,7 @@ class EmptyDirWarning(UserWarning): """Warning used by Cache.check for empty directories.""" -def args_to_key(base, args, kwargs, typed, ignore): +def args_to_key(base, args, kwargs, typed, ignore, arg_names=()): """Create cache key out of function arguments. :param tuple base: base of key @@ -392,10 +393,18 @@ def args_to_key(base, args, kwargs, typed, ignore): :param dict kwargs: function keyword arguments :param bool typed: include types in cache key :param set ignore: positional or keyword args to ignore + :param tuple arg_names: names of the wrapped function's positional + parameters, in order, so a name in `ignore` matches a positional + argument the same way it matches a keyword argument (default ()) :return: cache key tuple """ - args = tuple(arg for index, arg in enumerate(args) if index not in ignore) + args = tuple( + arg + for index, arg in enumerate(args) + if index not in ignore + and not (index < len(arg_names) and arg_names[index] in ignore) + ) key = base + args + (None,) if kwargs: @@ -1853,7 +1862,10 @@ def memoize( :param float expire: seconds until arguments expire (default None, no expiry) :param str tag: text to associate with arguments (default None) - :param set ignore: positional or keyword args to ignore (default ()) + :param set ignore: positional or keyword args to ignore (default ()). + A parameter's name in this set is matched whether it was passed + positionally or by keyword; a positional index is also still + matched by position, same as before. :return: callable decorator """ @@ -1865,6 +1877,14 @@ def decorator(func): """Decorator created by memoize() for callable `func`.""" base = (full_name(func),) if name is None else (name,) + try: + arg_names = tuple(inspect.signature(func).parameters) + except (TypeError, ValueError): + # Some callables (e.g. certain builtins) don't expose a + # signature; fall back to matching `ignore` by position only, + # same as before this parameter-name lookup was added. + arg_names = () + @ft.wraps(func) def wrapper(*args, **kwargs): """Wrapper for callable to cache arguments and return values.""" @@ -1880,7 +1900,7 @@ def wrapper(*args, **kwargs): def __cache_key__(*args, **kwargs): """Make key for cache given function arguments.""" - return args_to_key(base, args, kwargs, typed, ignore) + return args_to_key(base, args, kwargs, typed, ignore, arg_names) wrapper.__cache_key__ = __cache_key__ return wrapper diff --git a/tests/test_core.py b/tests/test_core.py index 788afef..23a9a51 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1393,6 +1393,20 @@ def test(*args, **kwargs): assert cache.stats() == (2, 1) +def test_memoize_ignore_by_name_regardless_of_call_style(cache): + # Regression test for GH #240: a name in `ignore` should be honored + # whether the caller passes that argument positionally or by keyword. + # `session` is deliberately something that can't be pickled, so if it + # ever leaks into the cache key this raises instead of quietly caching + # under two different keys. + @cache.memoize(ignore={'session'}) + def get(entity_id, session): + return entity_id + + assert get('a', session=threading.Lock()) + assert get('b', threading.Lock()) + + def test_memoize_iter(cache): @cache.memoize() def test(*args, **kwargs):