From 35e9bab505987db7f852fc78d8e8f139d9f38ad5 Mon Sep 17 00:00:00 2001
From: Anton Pirker <anton.pirker@sentry.io>
Date: Thu, 23 May 2024 14:34:50 +0200
Subject: [PATCH] Handle also byte arras as strings (#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.
---
 sentry_sdk/integrations/redis/utils.py        | 16 +++++++++-----
 .../redis/test_redis_cache_module.py          | 22 +++++++++++++++++++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/sentry_sdk/integrations/redis/utils.py b/sentry_sdk/integrations/redis/utils.py
index 9bfa656158..207468ac77 100644
--- a/sentry_sdk/integrations/redis/utils.py
+++ b/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)
 
diff --git a/tests/integrations/redis/test_redis_cache_module.py b/tests/integrations/redis/test_redis_cache_module.py
index 2459958f13..d96d074343 100644
--- a/tests/integrations/redis/test_redis_cache_module.py
+++ b/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