Repository: aio-libs/aiomcache

Issue:
aiomcache not support chinese bytes as a key
I want use chinese bytes as a key:

```python
In [1]: import asyncio 
   ...: import aiomcache 
   ...:  
   ...: loop = asyncio.get_event_loop()                                                                                                                                                                                                                                             

In [2]: async def hello_aiomcache(): 
   ...:     mc = aiomcache.Client('127.0.0.1', 11211, loop=loop) 
   ...:     await mc.set(bytes('中文', 'utf-8'), b'123') 
   ...:                                                                                                            

In [3]: loop.run_until_complete(hello_aiomcache())                                                                 
---------------------------------------------------------------------------
ValidationException                       Traceback (most recent call last)
<ipython-input-5-adb51212a606> in <module>
----> 1 loop.run_until_complete(hello_aiomcache())

/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/asyncio/base_events.py in run_until_complete(self, future)
    465             raise RuntimeError('Event loop stopped before Future completed.')
    466 
--> 467         return future.result()
    468 
    469     def stop(self):

<ipython-input-4-d36e59b6000e> in hello_aiomcache()
      1 async def hello_aiomcache():
      2     mc = aiomcache.Client('127.0.0.1', 11211, loop=loop)
----> 3     await mc.set(bytes('中文', 'utf-8'), b'123')
      4 

~/.virtualenvs/lyanna-tp9VnnDu/lib/python3.6/site-packages/aiomcache/client.py in wrapper(self, *args, **kwargs)
     18         conn = yield from self._pool.acquire()
     19         try:
---> 20             return (yield from func(self, conn, *args, **kwargs))
     21         except Exception as exc:
     22             conn[0].set_exception(exc)

~/.virtualenvs/lyanna-tp9VnnDu/lib/python3.6/site-packages/aiomcache/client.py in set(self, conn, key, value, exptime)
    250         flags = 0  # TODO: fix when exception removed
    251         resp = yield from self._storage_command(
--> 252             conn, b'set', key, value, flags, exptime)
    253         return resp
    254 

~/.virtualenvs/lyanna-tp9VnnDu/lib/python3.6/site-packages/aiomcache/client.py in _storage_command(self, conn, command, key, value, flags, exptime, cas)
    218         # however custom-compiled memcached can have different limit
    219         # so, we'll let the server decide what's too much
--> 220         assert self._validate_key(key)
    221 
    222         if not isinstance(exptime, int):

~/.virtualenvs/lyanna-tp9VnnDu/lib/python3.6/site-packages/aiomcache/client.py in _validate_key(self, key)
     54                 raise ValidationException('trailing newline', key)
     55         else:
---> 56             raise ValidationException('invalid key', key)
     57 
     58         return key

ValidationException: invalid key: b'\xe4\xb8\xad\xe6\x96\x87'

In [4]: debug                                                                                                      
> /Users/dongweiming/.virtualenvs/lyanna-tp9VnnDu/lib/python3.6/site-packages/aiomcache/client.py(56)_validate_key()
     54                 raise ValidationException('trailing newline', key)
     55         else:
---> 56             raise ValidationException('invalid key', key)
     57 
     58         return key

ipdb> key                                                                                                          
b'\xe4\xb8\xad\xe6\x96\x87'
```

Discussion:
Dreamsorcerer: @webknjaz LGTM checks blocking merging is a little annoying. We have flake8 setup now, so it should rarely trigger any errors anyway.
webknjaz: Doesn't it make LGTM even more significant? I mean that it catches things flake8 missed? It's not like WPS linter set is integrated.
Dreamsorcerer: > Doesn't it make LGTM even more significant? I mean that it catches things flake8 missed? It's not like WPS linter set is integrated.

Problem is that it takes 15+ mins to complete (compared with <1 min for everything else), and will find an issue in probably about 1 in 500 PRs. I'd rather merge smaller PRs without waiting, and in the unlikely chance of it finding an issue, it can be fixed in a followup PR. I think this is how all the other repos are being used as well, including aiohttp.

Patch:
diff --git a/aiomcache/client.py b/aiomcache/client.py
index d11063c..5719878 100644
--- a/aiomcache/client.py
+++ b/aiomcache/client.py
@@ -37,21 +37,22 @@ class Client(object):
         self._pool = MemcachePool(
             host, port, minsize=pool_minsize, maxsize=pool_size)
 
-    # key supports ascii sans space and control chars
-    # \x21 is !, right after space, and \x7e is -, right before DEL
-    # also 1 <= len <= 250 as per the spec
-    _valid_key_re = re.compile(b'^[\x21-\x7e]{1,250}$')
+    # key may be anything except whitespace and control chars, upto 250 characters.
+    # Must be str for unicode-aware regex.
+    _valid_key_re = re.compile("^[^\\s\x00-\x1F\x7F-\x9F]{1,250}$")
 
     def _validate_key(self, key: bytes) -> bytes:
         if not isinstance(key, bytes):  # avoid bugs subtle and otherwise
             raise ValidationException('key must be bytes', key)
 
-        m = self._valid_key_re.match(key)
+        # Must decode to str for unicode-aware comparison.
+        key_str = key.decode()
+        m = self._valid_key_re.match(key_str)
         if m:
             # in python re, $ matches either end of line or right before
             # \n at end of line. We can't allow latter case, so
             # making sure length matches is simplest way to detect
-            if len(m.group(0)) != len(key):
+            if len(m.group(0)) != len(key_str):
                 raise ValidationException('trailing newline', key)
         else:
             raise ValidationException('invalid key', key)


Test patch:
diff --git a/tests/commands_test.py b/tests/commands_test.py
index 3a286e3..5cbab50 100644
--- a/tests/commands_test.py
+++ b/tests/commands_test.py
@@ -7,6 +7,42 @@ import pytest
 from aiomcache.exceptions import ClientException, ValidationException
 
 
+@pytest.mark.parametrize("key", (
+    b"key",
+    b"123",
+    bytes("!@#", "utf-8"),
+    bytes("中文", "utf-8"),
+    bytes("こんにちは", "utf-8"),
+    bytes("안녕하세요", "utf-8"),
+))
+@pytest.mark.asyncio
+async def test_valid_key(mcache, key):
+    assert mcache._validate_key(key) == key
+
+
+@pytest.mark.parametrize("key", (
+    # Whitespace
+    b"foo bar",
+    b"foo\t",
+    b"\nbar",
+    b"foo\x20\x0Dbar",
+    b"\x18\x0E",
+    b"\x20\x60",
+    b"\x30\x00",
+    b"\x20\x01",
+    # Control characters
+    b"foo\x00bar",
+    b"\x1F",
+    b"\x7F",
+    "\u0080".encode(),
+    "\u009F".encode(),
+))
+@pytest.mark.asyncio
+async def test_invalid_key(mcache, key):
+    with pytest.raises(ValidationException, match="invalid key"):
+        mcache._validate_key(key)
+
+
 @pytest.mark.asyncio
 async def test_version(mcache):
     version = await mcache.version()