[SF-11323] disallow sending command getkeys (#7)

* [SF-11323] disallow sending command getkeys

Author: bellatoris

redis/connection.py

@@ -878,6 +878,14 @@ def pack_command(self, *args):
         elif b" " in args[0]:
             args = tuple(args[0].split()) + args[1:]
 
+        # `COMMAND GETKEYS` can crash redis server entirely under certain conditions.
+        # So we have decided to make sure that `COMMAND GETKEYS` is never sent to the
+        # server. If you need to send `COMMAND GETKEYS` to the server, please reach out
+        # to Doogie and Zach to discuss the use case.
+        # ref: https://github.com/redis/redis/pull/12380
+        if len(args) > 1 and args[0].lower() == b'command' and args[1].lower().startswith(b'getkeys'):
+            raise Exception(f'Redis command "{args[0].decode()} {args[1].decode()}" is not supported')
+
         buff = SYM_EMPTY.join((SYM_STAR, str(len(args)).encode(), SYM_CRLF))
 
         buffer_cutoff = self._buffer_cutoff

tests/test_cluster.py

@@ -1420,8 +1420,9 @@ def test_time(self, r):
 
     @skip_if_server_version_lt("4.0.0")
     def test_memory_usage(self, r):
-        r.set("foo", "bar")
-        assert isinstance(r.memory_usage("foo"), int)
+        with pytest.raises(Exception):
+            r.set("foo", "bar")
+            assert isinstance(r.memory_usage("foo"), int)
 
     @skip_if_server_version_lt("4.0.0")
     @skip_if_redis_enterprise()
@@ -1732,80 +1733,87 @@ def test_cluster_sunionstore(self, r):
 
     @skip_if_server_version_lt("6.2.0")
     def test_cluster_zdiff(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
-        r.zadd("{foo}b", {"a1": 1, "a2": 2})
-        assert r.zdiff(["{foo}a", "{foo}b"]) == [b"a3"]
-        assert r.zdiff(["{foo}a", "{foo}b"], withscores=True) == [b"a3", b"3"]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
+            r.zadd("{foo}b", {"a1": 1, "a2": 2})
+            assert r.zdiff(["{foo}a", "{foo}b"]) == [b"a3"]
+            assert r.zdiff(["{foo}a", "{foo}b"], withscores=True) == [b"a3", b"3"]
 
     @skip_if_server_version_lt("6.2.0")
     def test_cluster_zdiffstore(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
-        r.zadd("{foo}b", {"a1": 1, "a2": 2})
-        assert r.zdiffstore("{foo}out", ["{foo}a", "{foo}b"])
-        assert r.zrange("{foo}out", 0, -1) == [b"a3"]
-        assert r.zrange("{foo}out", 0, -1, withscores=True) == [(b"a3", 3.0)]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
+            r.zadd("{foo}b", {"a1": 1, "a2": 2})
+            assert r.zdiffstore("{foo}out", ["{foo}a", "{foo}b"])
+            assert r.zrange("{foo}out", 0, -1) == [b"a3"]
+            assert r.zrange("{foo}out", 0, -1, withscores=True) == [(b"a3", 3.0)]
 
     @skip_if_server_version_lt("6.2.0")
     def test_cluster_zinter(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 1})
-        r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        assert r.zinter(["{foo}a", "{foo}b", "{foo}c"]) == [b"a3", b"a1"]
-        # invalid aggregation
-        with pytest.raises(DataError):
-            r.zinter(["{foo}a", "{foo}b", "{foo}c"], aggregate="foo", withscores=True)
-        # aggregate with SUM
-        assert r.zinter(["{foo}a", "{foo}b", "{foo}c"], withscores=True) == [
-            (b"a3", 8),
-            (b"a1", 9),
-        ]
-        # aggregate with MAX
-        assert r.zinter(
-            ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX", withscores=True
-        ) == [(b"a3", 5), (b"a1", 6)]
-        # aggregate with MIN
-        assert r.zinter(
-            ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN", withscores=True
-        ) == [(b"a1", 1), (b"a3", 1)]
-        # with weights
-        assert r.zinter({"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}, withscores=True) == [
-            (b"a3", 20),
-            (b"a1", 23),
-        ]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 1})
+            r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            assert r.zinter(["{foo}a", "{foo}b", "{foo}c"]) == [b"a3", b"a1"]
+            # invalid aggregation
+            with pytest.raises(DataError):
+                r.zinter(["{foo}a", "{foo}b", "{foo}c"], aggregate="foo", withscores=True)
+            # aggregate with SUM
+            assert r.zinter(["{foo}a", "{foo}b", "{foo}c"], withscores=True) == [
+                (b"a3", 8),
+                (b"a1", 9),
+            ]
+            # aggregate with MAX
+            assert r.zinter(
+                ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX", withscores=True
+            ) == [(b"a3", 5), (b"a1", 6)]
+            # aggregate with MIN
+            assert r.zinter(
+                ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN", withscores=True
+            ) == [(b"a1", 1), (b"a3", 1)]
+            # with weights
+            assert r.zinter({"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}, withscores=True) == [
+                (b"a3", 20),
+                (b"a1", 23),
+            ]
 
     def test_cluster_zinterstore_sum(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
-        r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        assert r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"]) == 2
-        assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 8), (b"a1", 9)]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
+            r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            assert r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"]) == 2
+            assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 8), (b"a1", 9)]
 
     def test_cluster_zinterstore_max(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
-        r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        assert (
-            r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX")
-            == 2
-        )
-        assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 5), (b"a1", 6)]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
+            r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            assert (
+                r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX")
+                == 2
+            )
+            assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 5), (b"a1", 6)]
 
     def test_cluster_zinterstore_min(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
-        r.zadd("{foo}b", {"a1": 2, "a2": 3, "a3": 5})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        assert (
-            r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN")
-            == 2
-        )
-        assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a1", 1), (b"a3", 3)]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 2, "a3": 3})
+            r.zadd("{foo}b", {"a1": 2, "a2": 3, "a3": 5})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            assert (
+                r.zinterstore("{foo}d", ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN")
+                == 2
+            )
+            assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a1", 1), (b"a3", 3)]
 
     def test_cluster_zinterstore_with_weight(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
-        r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        assert r.zinterstore("{foo}d", {"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}) == 2
-        assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 20), (b"a1", 23)]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
+            r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            assert r.zinterstore("{foo}d", {"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}) == 2
+            assert r.zrange("{foo}d", 0, -1, withscores=True) == [(b"a3", 20), (b"a1", 23)]
 
     @skip_if_server_version_lt("4.9.0")
     def test_cluster_bzpopmax(self, r):
@@ -1855,32 +1863,33 @@ def test_cluster_zrangestore(self, r):
 
     @skip_if_server_version_lt("6.2.0")
     def test_cluster_zunion(self, r):
-        r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
-        r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
-        r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
-        # sum
-        assert r.zunion(["{foo}a", "{foo}b", "{foo}c"]) == [b"a2", b"a4", b"a3", b"a1"]
-        assert r.zunion(["{foo}a", "{foo}b", "{foo}c"], withscores=True) == [
-            (b"a2", 3),
-            (b"a4", 4),
-            (b"a3", 8),
-            (b"a1", 9),
-        ]
-        # max
-        assert r.zunion(
-            ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX", withscores=True
-        ) == [(b"a2", 2), (b"a4", 4), (b"a3", 5), (b"a1", 6)]
-        # min
-        assert r.zunion(
-            ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN", withscores=True
-        ) == [(b"a1", 1), (b"a2", 1), (b"a3", 1), (b"a4", 4)]
-        # with weight
-        assert r.zunion({"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}, withscores=True) == [
-            (b"a2", 5),
-            (b"a4", 12),
-            (b"a3", 20),
-            (b"a1", 23),
-        ]
+        with pytest.raises(Exception):
+            r.zadd("{foo}a", {"a1": 1, "a2": 1, "a3": 1})
+            r.zadd("{foo}b", {"a1": 2, "a2": 2, "a3": 2})
+            r.zadd("{foo}c", {"a1": 6, "a3": 5, "a4": 4})
+            # sum
+            assert r.zunion(["{foo}a", "{foo}b", "{foo}c"]) == [b"a2", b"a4", b"a3", b"a1"]
+            assert r.zunion(["{foo}a", "{foo}b", "{foo}c"], withscores=True) == [
+                (b"a2", 3),
+                (b"a4", 4),
+                (b"a3", 8),
+                (b"a1", 9),
+            ]
+            # max
+            assert r.zunion(
+                ["{foo}a", "{foo}b", "{foo}c"], aggregate="MAX", withscores=True
+            ) == [(b"a2", 2), (b"a4", 4), (b"a3", 5), (b"a1", 6)]
+            # min
+            assert r.zunion(
+                ["{foo}a", "{foo}b", "{foo}c"], aggregate="MIN", withscores=True
+            ) == [(b"a1", 1), (b"a2", 1), (b"a3", 1), (b"a4", 4)]
+            # with weight
+            assert r.zunion({"{foo}a": 1, "{foo}b": 2, "{foo}c": 3}, withscores=True) == [
+                (b"a2", 5),
+                (b"a4", 12),
+                (b"a3", 20),
+                (b"a1", 23),
+            ]
 
     def test_cluster_zunionstore_sum(self, r):
         assert r.zunionstore("{foo}d", ["{foo}" + str(i) for i in range(0, 256)]) == 0
@@ -1980,9 +1989,10 @@ def test_cluster_pfmerge(self, r):
         assert r.pfcount("{foo}d") == 7
 
     def test_cluster_sort_store(self, r):
-        r.rpush("{foo}a", "2", "3", "1")
-        assert r.sort("{foo}a", store="{foo}sorted_values") == 3
-        assert r.lrange("{foo}sorted_values", 0, -1) == [b"1", b"2", b"3"]
+        with pytest.raises(Exception):
+            r.rpush("{foo}a", "2", "3", "1")
+            assert r.sort("{foo}a", store="{foo}sorted_values") == 3
+            assert r.lrange("{foo}sorted_values", 0, -1) == [b"1", b"2", b"3"]
 
     # GEO COMMANDS
     @skip_if_server_version_lt("6.2.0")
@@ -2026,33 +2036,35 @@ def test_geosearchstore_dist(self, r):
 
     @skip_if_server_version_lt("3.2.0")
     def test_cluster_georadius_store(self, r):
-        values = (2.1909389952632, 41.433791470673, "place1") + (
-            2.1873744593677,
-            41.406342043777,
-            "place2",
-        )
+        with pytest.raises(Exception):
+            values = (2.1909389952632, 41.433791470673, "place1") + (
+                2.1873744593677,
+                41.406342043777,
+                "place2",
+            )
 
-        r.geoadd("{foo}barcelona", values)
-        r.georadius(
-            "{foo}barcelona", 2.191, 41.433, 1000, store="{foo}places_barcelona"
-        )
-        assert r.zrange("{foo}places_barcelona", 0, -1) == [b"place1"]
+            r.geoadd("{foo}barcelona", values)
+            r.georadius(
+                "{foo}barcelona", 2.191, 41.433, 1000, store="{foo}places_barcelona"
+            )
+            assert r.zrange("{foo}places_barcelona", 0, -1) == [b"place1"]
 
     @skip_unless_arch_bits(64)
     @skip_if_server_version_lt("3.2.0")
     def test_cluster_georadius_store_dist(self, r):
-        values = (2.1909389952632, 41.433791470673, "place1") + (
-            2.1873744593677,
-            41.406342043777,
-            "place2",
-        )
+        with pytest.raises(Exception):
+            values = (2.1909389952632, 41.433791470673, "place1") + (
+                2.1873744593677,
+                41.406342043777,
+                "place2",
+            )
 
-        r.geoadd("{foo}barcelona", values)
-        r.georadius(
-            "{foo}barcelona", 2.191, 41.433, 1000, store_dist="{foo}places_barcelona"
-        )
-        # instead of save the geo score, the distance is saved.
-        assert r.zscore("{foo}places_barcelona", "place1") == 88.05060698409301
+            r.geoadd("{foo}barcelona", values)
+            r.georadius(
+                "{foo}barcelona", 2.191, 41.433, 1000, store_dist="{foo}places_barcelona"
+            )
+            # instead of save the geo score, the distance is saved.
+            assert r.zscore("{foo}places_barcelona", "place1") == 88.05060698409301
 
     def test_cluster_dbsize(self, r):
         d = {"a": b"1", "b": b"2", "c": b"3", "d": b"4"}

tests/test_command_parser.py

@@ -23,41 +23,42 @@ def test_get_keys_predetermined_key_location(self, r):
     @pytest.mark.filterwarnings("ignore:ResponseError")
     @skip_if_redis_enterprise()
     def test_get_moveable_keys(self, r):
-        commands_parser = CommandsParser(r)
-        args1 = [
-            "EVAL",
-            "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}",
-            2,
-            "key1",
-            "key2",
-            "first",
-            "second",
-        ]
-        args2 = ["XREAD", "COUNT", 2, b"STREAMS", "mystream", "writers", 0, 0]
-        args3 = ["ZUNIONSTORE", "out", 2, "zset1", "zset2", "WEIGHTS", 2, 3]
-        args4 = ["GEORADIUS", "Sicily", 15, 37, 200, "km", "WITHCOORD", b"STORE", "out"]
-        args5 = ["MEMORY USAGE", "foo"]
-        args6 = [
-            "MIGRATE",
-            "192.168.1.34",
-            6379,
-            "",
-            0,
-            5000,
-            b"KEYS",
-            "key1",
-            "key2",
-            "key3",
-        ]
-        args7 = ["MIGRATE", "192.168.1.34", 6379, "key1", 0, 5000]
+        with pytest.raises(Exception):
+            commands_parser = CommandsParser(r)
+            args1 = [
+                "EVAL",
+                "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}",
+                2,
+                "key1",
+                "key2",
+                "first",
+                "second",
+            ]
+            args2 = ["XREAD", "COUNT", 2, b"STREAMS", "mystream", "writers", 0, 0]
+            args3 = ["ZUNIONSTORE", "out", 2, "zset1", "zset2", "WEIGHTS", 2, 3]
+            args4 = ["GEORADIUS", "Sicily", 15, 37, 200, "km", "WITHCOORD", b"STORE", "out"]
+            args5 = ["MEMORY USAGE", "foo"]
+            args6 = [
+                "MIGRATE",
+                "192.168.1.34",
+                6379,
+                "",
+                0,
+                5000,
+                b"KEYS",
+                "key1",
+                "key2",
+                "key3",
+            ]
+            args7 = ["MIGRATE", "192.168.1.34", 6379, "key1", 0, 5000]
 
-        assert sorted(commands_parser.get_keys(r, *args1)) == ["key1", "key2"]
-        assert sorted(commands_parser.get_keys(r, *args2)) == ["mystream", "writers"]
-        assert sorted(commands_parser.get_keys(r, *args3)) == ["out", "zset1", "zset2"]
-        assert sorted(commands_parser.get_keys(r, *args4)) == ["Sicily", "out"]
-        assert sorted(commands_parser.get_keys(r, *args5)) == ["foo"]
-        assert sorted(commands_parser.get_keys(r, *args6)) == ["key1", "key2", "key3"]
-        assert sorted(commands_parser.get_keys(r, *args7)) == ["key1"]
+            assert sorted(commands_parser.get_keys(r, *args1)) == ["key1", "key2"]
+            assert sorted(commands_parser.get_keys(r, *args2)) == ["mystream", "writers"]
+            assert sorted(commands_parser.get_keys(r, *args3)) == ["out", "zset1", "zset2"]
+            assert sorted(commands_parser.get_keys(r, *args4)) == ["Sicily", "out"]
+            assert sorted(commands_parser.get_keys(r, *args5)) == ["foo"]
+            assert sorted(commands_parser.get_keys(r, *args6)) == ["key1", "key2", "key3"]
+            assert sorted(commands_parser.get_keys(r, *args7)) == ["key1"]
 
     # A bug in redis<7.0 causes this to fail: https://github.com/redis/redis/issues/9493
     @skip_if_server_version_lt("7.0.0")