Repository: django/channels_redis

Issue:
Channel names not unique 
It seems channel names are generated using a global instance of random, which can cause problems if the seed is reset.  For instance see:
https://groups.google.com/forum/#!msg/otree/0UEtVVA_98M/9nUo0nLPAgAJ
Presumably, there is also a small risk of non-uniqure names even if the seed is not reset.


Discussion:
tarikki: This looks pretty good and the following two tests fail with the old implementation but pass with this version:

```
@pytest.mark.asyncio
async def test_random_reset__channel_name(channel_layer):
    """
    Makes sure resetting random seed does not make us reuse channel names.
    """

    channel_layer = RedisChannelLayer()
    random.seed(1)
    channel_name_1 = await channel_layer.new_channel()
    random.seed(1)
    channel_name_2 = await channel_layer.new_channel()

    assert channel_name_1 != channel_name_2


@pytest.mark.asyncio
async def test_random_reset__client_prefix(channel_layer):
    """
    Makes sure resetting random seed does not make us reuse client_prefixes.
    """

    random.seed(1)
    channel_layer_1 = RedisChannelLayer()
    random.seed(1)
    channel_layer_2 = RedisChannelLayer()
    assert channel_layer_1.client_prefix != channel_layer_2.client_prefix

```


Also the length is probably not a problem because it's still under the maximum of 100 bytes mentioned in the Channel Layer Specification / Channel Semantics: https://channels.readthedocs.io/en/latest/channel_layer_spec.html
tarikki: @astutejoe: Wanna add those two test to the PR? :blush:  
astutejoe: > @astutejoe: Wanna add those two test to the PR? 😊

Will do! :D
astutejoe: @tarikki there you go!
adamchainz: Thanks for the tests @tarikki 
mpasternak: @astutejoe , @tarikki , thanks for your work! I came here to submit this exact fix, I'm glad to see all this work already done. @adamchainz , you're lucky guy having all this help from the community. If there's anything I could help you all with to get this issue merged, please let me know. 

Patch:
diff --git a/.gitignore b/.gitignore
index ccc6479..e1f94cd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@ build/
 /.tox
 .DS_Store
 .pytest_cache
+.vscode
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index e322f5c..0020a29 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -3,6 +3,8 @@ Unreleased
 
 * Updated msgpack requirement to `~=1.0`.
 
+* Ensured channel names are unique using UUIDs.
+
 
 2.4.2 (2020-02-19)
 ------------------
diff --git a/channels_redis/core.py b/channels_redis/core.py
index d7100a3..7045958 100644
--- a/channels_redis/core.py
+++ b/channels_redis/core.py
@@ -5,11 +5,10 @@ import collections
 import hashlib
 import itertools
 import logging
-import random
-import string
 import sys
 import time
 import types
+import uuid
 
 import aioredis
 import msgpack
@@ -216,10 +215,7 @@ class RedisChannelLayer(BaseChannelLayer):
         self._receive_index_generator = itertools.cycle(range(len(self.hosts)))
         self._send_index_generator = itertools.cycle(range(len(self.hosts)))
         # Decide on a unique client prefix to use in ! sections
-        # TODO: ensure uniqueness better, e.g. Redis keys with SETNX
-        self.client_prefix = "".join(
-            random.choice(string.ascii_letters) for i in range(8)
-        )
+        self.client_prefix = uuid.uuid4().hex
         # Set up any encryption objects
         self._setup_encryption(symmetric_encryption_keys)
         # Number of coroutines trying to receive right now
@@ -533,12 +529,7 @@ class RedisChannelLayer(BaseChannelLayer):
         Returns a new channel name that can be used by something in our
         process as a specific channel.
         """
-        # TODO: Guarantee uniqueness better?
-        return "%s.%s!%s" % (
-            prefix,
-            self.client_prefix,
-            "".join(random.choice(string.ascii_letters) for i in range(12)),
-        )
+        return "%s.%s!%s" % (prefix, self.client_prefix, uuid.uuid4().hex,)
 
     ### Flush extension ###
 


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index f14a9ef..8da3cae 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -1,4 +1,5 @@
 import asyncio
+import random
 
 import async_timeout
 import pytest
@@ -416,3 +417,31 @@ async def test_receive_cancel(channel_layer):
             await asyncio.wait_for(task, None)
         except asyncio.CancelledError:
             pass
+
+
+@pytest.mark.asyncio
+async def test_random_reset__channel_name(channel_layer):
+    """
+    Makes sure resetting random seed does not make us reuse channel names.
+    """
+
+    channel_layer = RedisChannelLayer()
+    random.seed(1)
+    channel_name_1 = await channel_layer.new_channel()
+    random.seed(1)
+    channel_name_2 = await channel_layer.new_channel()
+
+    assert channel_name_1 != channel_name_2
+
+
+@pytest.mark.asyncio
+async def test_random_reset__client_prefix(channel_layer):
+    """
+    Makes sure resetting random seed does not make us reuse client_prefixes.
+    """
+
+    random.seed(1)
+    channel_layer_1 = RedisChannelLayer()
+    random.seed(1)
+    channel_layer_2 = RedisChannelLayer()
+    assert channel_layer_1.client_prefix != channel_layer_2.client_prefix