Fix NaClCookieStorage for corrupt cookie generating error 500 (#317)

* NaClCookieStorage: manage corrupt cookies

* fix doc for NaClCookieStorage

Author: tardyp

aiohttp_session/nacl_storage.py

@@ -1,10 +1,13 @@
+import binascii
 import json
 
 import nacl.secret
 import nacl.utils
+import nacl.exceptions
 from nacl.encoding import Base64Encoder
 
 from . import AbstractStorage, Session
+from .log import log
 
 
 class NaClCookieStorage(AbstractStorage):
@@ -22,16 +25,26 @@ def __init__(self, secret_key, *, cookie_name="AIOHTTP_SESSION",
 
         self._secretbox = nacl.secret.SecretBox(secret_key)
 
+    def empty_session(self):
+        return Session(None, data=None, new=True, max_age=self.max_age)
+
     async def load_session(self, request):
         cookie = self.load_cookie(request)
         if cookie is None:
-            return Session(None, data=None, new=True, max_age=self.max_age)
+            return self.empty_session()
         else:
-            data = self._decoder(
-                self._secretbox.decrypt(cookie.encode('utf-8'),
-                                        encoder=Base64Encoder).decode('utf-8')
-            )
-            return Session(None, data=data, new=False, max_age=self.max_age)
+            try:
+                data = self._decoder(
+                    self._secretbox.decrypt(
+                        cookie.encode('utf-8'),
+                        encoder=Base64Encoder).decode('utf-8')
+                )
+                return Session(None, data=data, new=False,
+                               max_age=self.max_age)
+            except (binascii.Error, nacl.exceptions.CryptoError):
+                log.warning("Cannot decrypt cookie value, "
+                            "create a new fresh session")
+                return self.empty_session()
 
     async def save_session(self, request, response, session):
         if session.empty:

docs/reference.rst

@@ -348,7 +348,7 @@ To use the storage you should push it into
 :func:`~aiohttp_session.session_middleware`::
 
    app = aiohttp.web.Application(middlewares=[
-       aiohttp_session.cookie_storage.NaClCookieStorage(
+       aiohttp_session.nacl_storage.NaClCookieStorage(
            b'Thirty  two  length  bytes  key.'])
 
 .. class:: NaClCookieStorage(secret_key, *, \

tests/test_nacl_storage.py

@@ -179,3 +179,36 @@ async def handler(request):
 
     resp = await client.get('/?exp=yes')
     assert resp.status == 200
+
+
+async def test_load_corrupted_session(aiohttp_client, key):
+
+    async def handler(request):
+        session = await get_session(request)
+        assert isinstance(session, Session)
+        assert session.new
+        assert {} == session
+        return web.Response(body=b'OK')
+
+    client = await aiohttp_client(create_app(handler, key))
+    client.session.cookie_jar.update_cookies({'AIOHTTP_SESSION': 'bad key'})
+    resp = await client.get('/')
+    assert resp.status == 200
+
+
+async def test_load_session_different_key(aiohttp_client, key):
+
+    async def handler(request):
+        session = await get_session(request)
+        assert isinstance(session, Session)
+        assert session.new
+        assert {} == session
+        return web.Response(body=b'OK')
+
+    client = await aiohttp_client(create_app(handler, key))
+    # create another box with another key
+    key = nacl.utils.random(nacl.secret.SecretBox.KEY_SIZE)
+    secretbox = nacl.secret.SecretBox(key)
+    make_cookie(client, secretbox, {'a': 1, 'b': 12})
+    resp = await client.get('/')
+    assert resp.status == 200