From 43bbcefd1eebf24439dc70a24dbf32abbd7b0b90 Mon Sep 17 00:00:00 2001 From: Lucas Ficheux Date: Sat, 2 Dec 2023 05:21:38 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 2 ++ mitmproxy/net/http/cookies.py | 17 +++++++++----- test/mitmproxy/net/http/test_cookies.py | 30 ++++++++++++------------- test/mitmproxy/test_http.py | 2 +- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5201d2c0b..7d0e0a9d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/mitmproxy/net/http/cookies.py b/mitmproxy/net/http/cookies.py index be8c94f8e..0c615febc 100644 --- a/mitmproxy/net/http/cookies.py +++ b/mitmproxy/net/http/cookies.py @@ -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) diff --git a/test/mitmproxy/net/http/test_cookies.py b/test/mitmproxy/net/http/test_cookies.py index 03c4a4046..3e4e824e7 100644 --- a/test/mitmproxy/net/http/test_cookies.py +++ b/test/mitmproxy/net/http/test_cookies.py @@ -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", "/"), ] ], ], diff --git a/test/mitmproxy/test_http.py b/test/mitmproxy/test_http.py index c20eb3977..fecf09e7b 100644 --- a/test/mitmproxy/test_http.py +++ b/test/mitmproxy/test_http.py @@ -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()