Repository: encode/starlette
Pull request: Ignore malformed cookie
Fixes #866 

Problem statement:
[bug] Invalid cookie name leads to exception
When handling a request with an invalid cookie name (does not conform to RFC2109) starlette raises an exception. i.e iam/cookiename
This is because Starlette uses Python's stdlib cookie library, which is very strict.
I do understand the strictness, but in real life scenarios you receive such malformed cookies and I want to handle those requests.
My suggestion for a solution would be to catch those exceptions and ignore the invalid cookie. 
** EDIT **
I just realized stdlib is used for the whole cookie header, hence can't ignore only one cookie.
I'll create a PR for ignoring the whole cookie on such case, but maybe we should create our own Cookie/Morsel class and override the methods to ignore such error in the inbound case?

Hints from issue discussion:
I’m always a bit wary of encouraging broken behaviors... Eg even Django just ignores invalid cookies, preventing developers from processing them: https://github.com/django/django/commit/30bdabb2b0

Can you solve your use case with a custom middleware that converts the broken cookie into a valid one?
> I’m always a bit wary of encouraging broken behaviors... Eg even Django just ignores invalid cookies, preventing developers from processing them: [django/django@30bdabb](https://github.com/django/django/commit/30bdabb2b0)
> 
> Can you solve your use case with a custom middleware that converts the broken cookie into a valid one?

Thanks for your feedback. I assume a middleware is OK if I want to process invalid cookie.
Do you have any feedback on the PR itself maybe? Right now in case of invalid cookie it ignores it.

Commit message: ignore malformed cookie instead of raising an exception

Patch to review:
diff --git a/docs/requests.md b/docs/requests.md
index f0ffc66bf..a72cb75dc 100644
--- a/docs/requests.md
+++ b/docs/requests.md
@@ -73,6 +73,8 @@ Cookies are exposed as a regular dictionary interface.
 
 For example: `request.cookies.get('mycookie')`
 
+Cookies are ignored in case of an invalid cookie. (RFC2109)
+
 #### Body
 
 There are a few different interfaces for returning the body of the request:
diff --git a/starlette/requests.py b/starlette/requests.py
index 1f7b09e34..0ebeb3f4f 100644
--- a/starlette/requests.py
+++ b/starlette/requests.py
@@ -91,7 +91,10 @@ def cookies(self) -> typing.Dict[str, str]:
             cookie_header = self.headers.get("cookie")
             if cookie_header:
                 cookie = http.cookies.SimpleCookie()  # type: http.cookies.BaseCookie
-                cookie.load(cookie_header)
+                try:
+                    cookie.load(cookie_header)
+                except http.cookies.CookieError:
+                    pass
                 for key, morsel in cookie.items():
                     cookies[key] = morsel.value
             self._cookies = cookies
diff --git a/tests/test_requests.py b/tests/test_requests.py
index defdf3a45..caa2bf576 100644
--- a/tests/test_requests.py
+++ b/tests/test_requests.py
@@ -285,6 +285,23 @@ async def app(scope, receive, send):
     assert response.text == "Hello, cookies!"
 
 
+def test_invalid_cookie():
+    async def app(scope, receive, send):
+        request = Request(scope, receive)
+        if not request.cookies:
+            response = Response("ok", media_type="text/plain")
+        else:
+            response = Response("not", media_type="text/plain")
+        await response(scope, receive, send)
+
+    client = TestClient(app)
+    response = client.get("/", cookies={"invalid/cookie": "test", "valid": "test2"})
+    assert response.text == "ok"
+
+    response = client.get("/", cookies={"valid": "test2"})
+    assert response.text == "not"
+
+
 def test_chunked_encoding():
     async def app(scope, receive, send):
         request = Request(scope, receive)


Code review comments:
diff_hunk: @@ -285,6 +285,23 @@ def test_request_cookies():
     assert response.text == "Hello, cookies!"
 
 
+def test_invalid_cookie():
+    async def app(scope, receive, send):
+        request = Request(scope, receive)
+        if not request.cookies:
+            response = Response("ok", media_type="text/plain")
+        else:
+            response = Response("not", media_type="text/plain")
+        await response(scope, receive, send)
+
+    client = TestClient(app)
+    response = client.get("/", cookies={"invalid/cookie": "test", "valid": "test2"})
+    assert response.text == "ok"
+
+    response = client.get("/", cookies={"valid": "test2"})
+    assert response.text == "not"
path: tests/test_requests.py
text: @user1:
Nits:

- The fact that "ok" corresponds to "cookies are invalid" is a bit counter-intuitive to me.
- We already test the valid case, so we can probably focus this test on "what happens if we pass invalid cookies?".

For example…

```suggestion
def test_invalid_cookie():
    async def app(scope, receive, send):
        request = Request(scope, receive)
        response = JSONResponse({"cookies": request.cookies})
        await response(scope, receive, send)

    client = TestClient(app)
    response = client.get("/", cookies={"invalid/cookie": "test", "valid": "test2"})
    assert response.json() == {"cookies": {}}
```