Merge pull request #1121 from Kriechi/fix-cookies

improve cookie parsing

mitmproxy/console/flowview.py

@@ -364,12 +364,11 @@ class FlowView(tabs.Tabs):
             self.edit_form(conn)
 
     def set_cookies(self, lst, conn):
-        od = odict.ODict(lst)
+        conn.cookies = odict.ODict(lst)
-        conn.set_cookies(od)
         signals.flow_change.send(self, flow = self.flow)
 
     def set_setcookies(self, data, conn):
-        conn.set_cookies(data)
+        conn.cookies = data
         signals.flow_change.send(self, flow = self.flow)
 
     def edit(self, part):
@@ -389,7 +388,7 @@ class FlowView(tabs.Tabs):
             self.master.view_grideditor(
                 grideditor.CookieEditor(
                     self.master,
-                    message.get_cookies().lst,
+                    message.cookies.lst,
                     self.set_cookies,
                     message
                 )
@@ -398,7 +397,7 @@ class FlowView(tabs.Tabs):
             self.master.view_grideditor(
                 grideditor.SetCookieEditor(
                     self.master,
-                    message.get_cookies(),
+                    message.cookies,
                     self.set_setcookies,
                     message
                 )

mitmproxy/flow.py

@@ -13,9 +13,9 @@ from six.moves import http_cookies, http_cookiejar, urllib
 import os
 import re
 
-from netlib import wsgi
+from netlib import wsgi, odict
 from netlib.exceptions import HttpException
-from netlib.http import Headers, http1
+from netlib.http import Headers, http1, cookies
 from . import controller, tnetstring, filt, script, version, flow_format_compat
 from .onboarding import app
 from .proxy.config import HostMatcher
@@ -313,15 +313,17 @@ class StickyCookieState:
         self.jar = defaultdict(dict)
         self.flt = flt
 
-    def ckey(self, m, f):
+    def ckey(self, attrs, f):
         """
             Returns a (domain, port, path) tuple.
         """
-        return (
+        domain = f.request.host
-            m["domain"] or f.request.host,
+        path = "/"
-            f.request.port,
+        if attrs["domain"]:
-            m["path"] or "/"
+            domain = attrs["domain"][-1]
-        )
+        if attrs["path"]:
+            path = attrs["path"][-1]
+        return (domain, f.request.port, path)
 
     def domain_match(self, a, b):
         if http_cookiejar.domain_match(a, b):
@@ -334,11 +336,12 @@ class StickyCookieState:
         for i in f.response.headers.get_all("set-cookie"):
             # FIXME: We now know that Cookie.py screws up some cookies with
             # valid RFC 822/1123 datetime specifications for expiry. Sigh.
-            c = http_cookies.SimpleCookie(str(i))
+            name, value, attrs = cookies.parse_set_cookie_header(str(i))
-            for m in c.values():
+            a = self.ckey(attrs, f)
-                k = self.ckey(m, f)
+            if self.domain_match(f.request.host, a[0]):
-                if self.domain_match(f.request.host, k[0]):
+                b = attrs.lst
-                    self.jar[k][m.key] = m
+                b.insert(0, [name, value])
+                self.jar[a][name] = odict.ODictCaseless(b)
 
     def handle_request(self, f):
         l = []
@@ -350,7 +353,8 @@ class StickyCookieState:
                     f.request.path.startswith(i[2])
                 ]
                 if all(match):
-                    l.extend([m.output(header="").strip() for m in self.jar[i].values()])
+                    c = self.jar[i]
+                    l.extend([cookies.format_cookie_header(c[name]) for name in c.keys()])
         if l:
             f.request.stickycookie = True
             f.request.headers["cookie"] = "; ".join(l)

netlib/http/cookies.py

@@ -1,5 +1,6 @@
 from six.moves import http_cookies as Cookie
 import re
+import string
 from email.utils import parsedate_tz, formatdate, mktime_tz
 
 from .. import odict
@@ -27,7 +28,6 @@ variants. Serialization follows RFC6265.
 
 # TODO: Disallow LHS-only Cookie values
 
-
 def _read_until(s, start, term):
     """
         Read until one of the characters in term is reached.
@@ -203,16 +203,16 @@ def refresh_set_cookie_header(c, delta):
     Returns:
         A refreshed Set-Cookie string
     """
-    try:
+
-        c = Cookie.SimpleCookie(str(c))
+    name, value, attrs = parse_set_cookie_header(c)
-    except Cookie.CookieError:
+    if not name or not value:
         raise ValueError("Invalid Cookie")
-    for i in c.values():
+
-        if "expires" in i:
+    if "expires" in attrs:
-            d = parsedate_tz(i["expires"])
+        e = parsedate_tz(attrs["expires"][-1])
-            if d:
+        if e:
-                d = mktime_tz(d) + delta
+            f = mktime_tz(e) + delta
-                i["expires"] = formatdate(d)
+            attrs["expires"] = [formatdate(f)]
         else:
             # This can happen when the expires tag is invalid.
             # reddit.com sends a an expires tag like this: "Thu, 31 Dec
@@ -220,8 +220,9 @@ def refresh_set_cookie_header(c, delta):
             # strictly correct according to the cookie spec. Browsers
             # appear to parse this tolerantly - maybe we should too.
             # For now, we just ignore this.
-                del i["expires"]
+            del attrs["expires"]
-    ret = c.output(header="").strip()
+
+    ret = format_set_cookie_header(name, value, attrs)
     if not ret:
         raise ValueError("Invalid Cookie")
     return ret

netlib/http/request.py

@@ -343,14 +343,6 @@ class Request(Message):
 
     # Legacy
 
-    def get_cookies(self):  # pragma: no cover
-        warnings.warn(".get_cookies is deprecated, use .cookies instead.", DeprecationWarning)
-        return self.cookies
-
-    def set_cookies(self, odict):  # pragma: no cover
-        warnings.warn(".set_cookies is deprecated, use .cookies instead.", DeprecationWarning)
-        self.cookies = odict
-
     def get_query(self):  # pragma: no cover
         warnings.warn(".get_query is deprecated, use .query instead.", DeprecationWarning)
         return self.query or ODict([])

netlib/http/response.py

@@ -127,13 +127,3 @@ class Response(Message):
             c.append(refreshed)
         if c:
             self.headers.set_all("set-cookie", c)
-
-    # Legacy
-
-    def get_cookies(self):  # pragma: no cover
-        warnings.warn(".get_cookies is deprecated, use .cookies instead.", DeprecationWarning)
-        return self.cookies
-
-    def set_cookies(self, odict):  # pragma: no cover
-        warnings.warn(".set_cookies is deprecated, use .cookies instead.", DeprecationWarning)
-        self.cookies = odict

test/mitmproxy/test_flow.py

@@ -76,6 +76,21 @@ class TestStickyCookieState:
         googlekey = s.jar.keys()[0]
         assert len(s.jar[googlekey].keys()) == 2
 
+        # Test setting of weird cookie keys
+        s = flow.StickyCookieState(filt.parse(".*"))
+        f = tutils.tflow(req=netlib.tutils.treq(host="www.google.com", port=80), resp=True)
+        cs = [
+            "foo/bar=hello",
+            "foo:bar=world",
+            "foo@bar=fizz",
+            "foo,bar=buzz",
+        ]
+        for c in cs:
+            f.response.headers["Set-Cookie"] = c
+            s.handle_response(f)
+        googlekey = s.jar.keys()[0]
+        assert len(s.jar[googlekey].keys()) == len(cs)
+
         # Test overwriting of a cookie value
         c1 = "somecookie=helloworld; Path=/"
         c2 = "somecookie=newvalue; Path=/"
@@ -84,7 +99,7 @@ class TestStickyCookieState:
         s.handle_response(f)
         googlekey = s.jar.keys()[0]
         assert len(s.jar[googlekey].keys()) == 1
-        assert s.jar[googlekey]["somecookie"].value == "newvalue"
+        assert s.jar[googlekey]["somecookie"].items()[0][1] == "newvalue"
 
     def test_handle_request(self):
         s, f = self._response("SSID=mooo", "www.google.com")

test/netlib/http/test_cookies.py

@@ -228,7 +228,16 @@ def test_refresh_cookie():
     c = "MOO=BAR; Expires=Tue, 08-Mar-2011 00:20:38 GMT; Path=foo.com; Secure"
     assert "00:21:38" in cookies.refresh_set_cookie_header(c, 60)
 
-    # https://github.com/mitmproxy/mitmproxy/issues/773
+    c = "foo,bar"
-    c = ">=A"
     with raises(ValueError):
         cookies.refresh_set_cookie_header(c, 60)
+
+    # https://github.com/mitmproxy/mitmproxy/issues/773
+    c = ">=A"
+    assert cookies.refresh_set_cookie_header(c, 60)
+
+    # https://github.com/mitmproxy/mitmproxy/issues/1118
+    c = "foo:bar=bla"
+    assert cookies.refresh_set_cookie_header(c, 0)
+    c = "foo/bar=bla"
+    assert cookies.refresh_set_cookie_header(c, 0)

test/netlib/http/test_request.py

@@ -172,7 +172,7 @@ class TestRequestUtils(object):
     def test_get_cookies_none(self):
         request = treq()
         request.headers = Headers()
-        assert len(request.cookies) == 0
+        assert not request.cookies
 
     def test_get_cookies_single(self):
         request = treq()

test/netlib/http/test_response.py

@@ -98,7 +98,7 @@ class TestResponseUtils(object):
         resp = tresp()
         v = resp.cookies
         v.add("foo", ["bar", ODictCaseless()])
-        resp.set_cookies(v)
+        resp.cookies = v
 
         v = resp.cookies
         assert len(v) == 1