Skip to content

Commit

Permalink
remove pickle usage in redis-backed cache helper
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-codecov committed Nov 13, 2024
1 parent 9a2692a commit 6d26b60
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 18 deletions.
10 changes: 5 additions & 5 deletions shared/helpers/cache.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import asyncio
import base64
import hashlib
import json
import logging
import pickle
from functools import wraps
from typing import Any, Callable, Hashable, List, Optional

Expand Down Expand Up @@ -86,8 +86,6 @@ def set(self, key: str, ttl: int, value: Any):


class RedisBackend(BaseBackend):
current_protocol = pickle.DEFAULT_PROTOCOL

def __init__(self, redis_connection: Redis):
self.redis_connection = redis_connection

Expand All @@ -100,16 +98,18 @@ def get(self, key: str) -> Any:
if serialized_value is None:
return NO_VALUE
try:
return pickle.loads(serialized_value)
return json.loads(serialized_value)
except ValueError:
return NO_VALUE

def set(self, key: str, ttl: int, value: Any):
serialized_value = pickle.dumps(value, self.current_protocol)
try:
serialized_value = json.dumps(value)
self.redis_connection.setex(key, ttl, serialized_value)
except RedisError:
log.warning("Unable to set cache on redis", exc_info=True)
except TypeError:
log.exception("Attempted to cache a type that is not JSON-serializable")


class LogMapping(dict):
Expand Down
30 changes: 17 additions & 13 deletions tests/unit/helpers/test_cache.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import unittest

import pytest
from redis.exceptions import TimeoutError

Expand Down Expand Up @@ -67,28 +69,30 @@ def setex(self, key, expire, value):
raise TimeoutError()


class TestRedisBackend(object):
class TestRedisBackend(unittest.TestCase):
def test_simple_redis_call(self):
redis_backend = RedisBackend(FakeRedis())
assert redis_backend.get("normal_key") == NO_VALUE
redis_backend.set("normal_key", 120, {"value_1": set("ascdefgh"), 1: [1, 3]})
value_1 = list(set("ascdefgh"))
redis_backend.set("normal_key", 120, {"value_1": value_1, "1": [1, 3]})
assert redis_backend.get("normal_key") == {
"value_1": set("ascdefgh"),
1: [1, 3],
"value_1": value_1,
"1": [1, 3],
}

def test_simple_redis_call_invalid_pickle_version(self):
redis_instance = FakeRedis()
# PICKLE HERE WILL BE SET TO VERSION 9 (\x09 in the second byte of the value)
# IF THIS STOPS FAILING WITH ValueError, CHANGE THE SECOND BYTE TO SOMETHING HIGHER
redis_instance.setex("key", 120, b"\x80\x09X\x05\x00\x00\x00valueq\x00.")
redis_backend = RedisBackend(redis_instance)
assert redis_backend.get("key") == NO_VALUE

def test_simple_redis_call_exception(self):
redis_backend = RedisBackend(FakeRedisWithIssues())
assert redis_backend.get("normal_key") == NO_VALUE
redis_backend.set("normal_key", 120, {"value_1": set("ascdefgh"), 1: [1, 3]})
redis_backend.set(
"normal_key", 120, {"value_1": list(set("ascdefgh")), "1": [1, 3]}
)
assert redis_backend.get("normal_key") == NO_VALUE

def test_simple_redis_call_not_json_serializable(self):
redis_backend = RedisBackend(FakeRedis())

unserializable = set("abcdefg")
redis_backend.set("normal_key", 120, unserializable)
assert redis_backend.get("normal_key") == NO_VALUE


Expand Down

0 comments on commit 6d26b60

Please sign in to comment.