Handle also byte arras as strings (getsentry#3101)

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.

Author: antonpirker

sentry_sdk/integrations/redis/utils.py

@@ -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)
 

tests/integrations/redis/test_redis_cache_module.py

@@ -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
 
@@ -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