Skip to content

Commit

Permalink
Handle also byte arras as strings (#3101)
Browse files Browse the repository at this point in the history
In some cases it can happen that the array of redis keys to get can be byte arrays and not string. Make sure we can deal with all kinds of keys, no matter if byte array or string.
  • Loading branch information
antonpirker committed May 23, 2024
1 parent 45bf880 commit 35e9bab
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
16 changes: 11 additions & 5 deletions sentry_sdk/integrations/redis/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,27 @@ def _get_safe_key(method_name, args, kwargs):
key = ""
if args is not None and method_name.lower() in _MULTI_KEY_COMMANDS:
# for example redis "mget"
key = ", ".join(args)
key = ", ".join(x.decode() if isinstance(x, bytes) else x for x in args)

elif args is not None and len(args) >= 1:
# for example django "set_many/get_many" or redis "get"
key = args[0]
key = args[0].decode() if isinstance(args[0], bytes) else args[0]

elif kwargs is not None and "key" in kwargs:
# this is a legacy case for older versions of django (I guess)
key = kwargs["key"]
key = (
kwargs["key"].decode()
if isinstance(kwargs["key"], bytes)
else kwargs["key"]
)

if isinstance(key, dict):
# Django caching set_many() has a dictionary {"key": "data", "key2": "data2"}
# as argument. In this case only return the keys of the dictionary (to not leak data)
key = ", ".join(key.keys())
key = ", ".join(x.decode() if isinstance(x, bytes) else x for x in key.keys())

if isinstance(key, list):
key = ", ".join(key)
key = ", ".join(x.decode() if isinstance(x, bytes) else x for x in key)

return str(key)

Expand Down
22 changes: 22 additions & 0 deletions tests/integrations/redis/test_redis_cache_module.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import pytest

import fakeredis
from fakeredis import FakeStrictRedis

from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.integrations.redis.utils import _get_safe_key
from sentry_sdk.utils import parse_version
import sentry_sdk

Expand Down Expand Up @@ -185,3 +188,22 @@ def test_cache_data(sentry_init, capture_events):
assert spans[4]["data"]["network.peer.address"] == "mycacheserver.io"

assert spans[5]["op"] == "db.redis" # we ignore db spans in this test.


@pytest.mark.parametrize(
"method_name,args,kwargs,expected_key",
[
(None, None, None, ""),
("", None, None, ""),
("set", ["bla", "valuebla"], None, "bla"),
("setex", ["bla", 10, "valuebla"], None, "bla"),
("get", ["bla"], None, "bla"),
("mget", ["bla", "blub", "foo"], None, "bla, blub, foo"),
("set", [b"bla", "valuebla"], None, "bla"),
("setex", [b"bla", 10, "valuebla"], None, "bla"),
("get", [b"bla"], None, "bla"),
("mget", [b"bla", "blub", "foo"], None, "bla, blub, foo"),
],
)
def test_get_safe_key(method_name, args, kwargs, expected_key):
assert _get_safe_key(method_name, args, kwargs) == expected_key

0 comments on commit 35e9bab

Please sign in to comment.