Parse set header cookies properly (#5084)

#### Description

Currently when an empty cookie attribute (`Secure`, `HttpOnly` ...) is
encountered while parsing a `Set-Cookie` header it will create a
`CookieAttrs` object containing a (key, value) pair with an empty string
for the attribute value ie:

```python
CookieAttrs[('Secure', ''), ('HttpOnly', ''), ('Path', '/')]
``` 
Resulting in an updated `Set-Cookie` header for the `Response` object
with invalid values for those empty attributes ie:
```python
(b'SetCookie', b'value=XYZ; Secure=; HttpOnly=; Path=/')
``` 
My browser (Firefox 95.0.1) does not pickup these attributes so the
cookie looses them.

______

This fix replaces the empty string attribute for empty cookie attributes
by the value `None` ie:

```python
CookieAttrs[('Secure', None), ('HttpOnly', None), ('Path', '/')]
``` 

So that they can be told apart from attributes with intentional empty
string values when setting the updated header, which results in a
properly formatted header:

```python
(b'SetCookie', b'value=XYZ; Secure; HttpOnly; Path=/')
``` 

#### Checklist

 - [x] I have updated tests where applicable.
 - [x] I have added an entry to the CHANGELOG.

Co-authored-by: Lucas FICHEUX <lficheux@corp.free.fr>
This commit is contained in:
Lucas Ficheux 2023-12-02 05:21:38 +01:00 committed by GitHub
parent 72679e5cf7
commit 43bbcefd1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 22 deletions

View File

@ -7,6 +7,8 @@
## Unreleased: mitmproxy next
* Fix empty cookie attributes being set to `Key=` instead of `Key`
([#5084](https://github.com/mitmproxy/mitmproxy/pull/5084), @Speedlulu)
## 14 November 2023: mitmproxy 10.1.5

View File

@ -48,8 +48,8 @@ class CookieAttrs(multidict.MultiDict):
return values[-1]
TSetCookie = tuple[str, str, CookieAttrs]
TPairs = list[list[str]] # TODO: Should be List[Tuple[str,str]]?
TSetCookie = tuple[str, str | None, CookieAttrs]
TPairs = list[tuple[str, str | None]]
def _read_until(s, start, term):
@ -169,10 +169,10 @@ def _read_set_cookie_pairs(s: str, off=0) -> tuple[list[TPairs], int]:
rhs = rhs + "," + trail
# as long as there's a "=", we consider it a pair
pairs.append([lhs, rhs])
pairs.append((lhs, rhs))
elif lhs:
pairs.append([lhs, rhs])
pairs.append((lhs, None))
# comma marks the beginning of a new cookie
if off < len(s) and s[off] == ",":
@ -206,10 +206,15 @@ def _format_pairs(pairs, specials=(), sep="; "):
"""
vals = []
for k, v in pairs:
if k.lower() not in specials and _has_special(v):
if v is None:
val = k
elif k.lower() not in specials and _has_special(v):
v = ESCAPE.sub(r"\\\1", v)
v = '"%s"' % v
vals.append(f"{k}={v}")
val = f"{k}={v}"
else:
val = f"{k}={v}"
vals.append(val)
return sep.join(vals)

View File

@ -93,23 +93,23 @@ def test_cookie_roundtrips():
def test_parse_set_cookie_pairs():
pairs = [
["=", [[["", ""]]]],
["=;foo=bar", [[["", ""], ["foo", "bar"]]]],
["=;=;foo=bar", [[["", ""], ["", ""], ["foo", "bar"]]]],
["=uno", [[["", "uno"]]]],
["one=uno", [[["one", "uno"]]]],
["one=un\x20", [[["one", "un\x20"]]]],
["one=uno; foo", [[["one", "uno"], ["foo", ""]]]],
["=", [[("", "")]]],
["=;foo=bar", [[("", ""), ("foo", "bar")]]],
["=;=;foo=bar", [[("", ""), ("", ""), ("foo", "bar")]]],
["=uno", [[("", "uno")]]],
["one=uno", [[("one", "uno")]]],
["one=un\x20", [[("one", "un\x20")]]],
["one=uno; foo", [[("one", "uno"), ("foo", None)]]],
[
"mun=1.390.f60; "
"expires=sun, 11-oct-2015 12:38:31 gmt; path=/; "
"domain=b.aol.com",
[
[
["mun", "1.390.f60"],
["expires", "sun, 11-oct-2015 12:38:31 gmt"],
["path", "/"],
["domain", "b.aol.com"],
("mun", "1.390.f60"),
("expires", "sun, 11-oct-2015 12:38:31 gmt"),
("path", "/"),
("domain", "b.aol.com"),
]
],
],
@ -120,10 +120,10 @@ def test_parse_set_cookie_pairs():
"path=/",
[
[
["rpb", r"190%3d1%2616726%3d1%2634832%3d1%2634874%3d1"],
["domain", ".rubiconproject.com"],
["expires", "mon, 11-may-2015 21:54:57 gmt"],
["path", "/"],
("rpb", r"190%3d1%2616726%3d1%2634832%3d1%2634874%3d1"),
("domain", ".rubiconproject.com"),
("expires", "mon, 11-may-2015 21:54:57 gmt"),
("path", "/"),
]
],
],

View File

@ -569,7 +569,7 @@ class TestResponseUtils:
assert attrs["domain"] == "example.com"
assert attrs["expires"] == "Wed Oct 21 16:29:41 2015"
assert attrs["path"] == "/"
assert attrs["httponly"] == ""
assert attrs["httponly"] is None
def test_get_cookies_no_value(self):
resp = tresp()