From 18ee6255c03dafff715650b15ae3cd6ff6152bac Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Fri, 21 Oct 2016 11:40:05 +1300 Subject: [PATCH 1/2] multidict: ditch ImmutableMultiDict A contorted class we only use for cookie attributes. We don't need it. --- mitmproxy/addons/stickycookie.py | 3 +- mitmproxy/net/http/cookies.py | 6 ++-- mitmproxy/types/multidict.py | 47 +------------------------- test/mitmproxy/test_types_multidict.py | 34 ------------------- 4 files changed, 6 insertions(+), 84 deletions(-) diff --git a/mitmproxy/addons/stickycookie.py b/mitmproxy/addons/stickycookie.py index 27d78646b..293de5654 100644 --- a/mitmproxy/addons/stickycookie.py +++ b/mitmproxy/addons/stickycookie.py @@ -59,7 +59,8 @@ class StickyCookie: if not self.jar[dom_port_path]: self.jar.pop(dom_port_path, None) else: - b = attrs.with_insert(0, name, value) + b = attrs.copy() + b.insert(0, name, value) self.jar[dom_port_path][name] = b def request(self, flow): diff --git a/mitmproxy/net/http/cookies.py b/mitmproxy/net/http/cookies.py index 9f32fa5ee..2f99568b5 100644 --- a/mitmproxy/net/http/cookies.py +++ b/mitmproxy/net/http/cookies.py @@ -31,7 +31,7 @@ _cookie_params = set(( ESCAPE = re.compile(r"([\"\\])") -class CookieAttrs(multidict.ImmutableMultiDict): +class CookieAttrs(multidict.MultiDict): @staticmethod def _kconv(key): return key.lower() @@ -300,7 +300,7 @@ def refresh_set_cookie_header(c, delta): e = email.utils.parsedate_tz(attrs["expires"]) if e: f = email.utils.mktime_tz(e) + delta - attrs = attrs.with_set_all("expires", [email.utils.formatdate(f)]) + attrs.set_all("expires", [email.utils.formatdate(f)]) else: # This can happen when the expires tag is invalid. # reddit.com sends a an expires tag like this: "Thu, 31 Dec @@ -308,7 +308,7 @@ 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. - attrs = attrs.with_delitem("expires") + del attrs["expires"] rv = format_set_cookie_header([(name, value, attrs)]) if not rv: diff --git a/mitmproxy/types/multidict.py b/mitmproxy/types/multidict.py index d351e48b7..e02d28a9c 100644 --- a/mitmproxy/types/multidict.py +++ b/mitmproxy/types/multidict.py @@ -1,11 +1,6 @@ from abc import ABCMeta, abstractmethod - -try: - from collections.abc import MutableMapping -except ImportError: # pragma: no cover - from collections import MutableMapping # Workaround for Python < 3.3 - +from collections.abc import MutableMapping from mitmproxy.types import serializable @@ -227,46 +222,6 @@ class MultiDict(_MultiDict): return key -class ImmutableMultiDict(MultiDict, metaclass=ABCMeta): - def _immutable(self, *_): - raise TypeError('{} objects are immutable'.format(self.__class__.__name__)) - - __delitem__ = set_all = insert = _immutable - - def __hash__(self): - return hash(self.fields) - - def with_delitem(self, key): - """ - Returns: - An updated ImmutableMultiDict. The original object will not be modified. - """ - ret = self.copy() - # FIXME: This is filthy... - super(ImmutableMultiDict, ret).__delitem__(key) - return ret - - def with_set_all(self, key, values): - """ - Returns: - An updated ImmutableMultiDict. The original object will not be modified. - """ - ret = self.copy() - # FIXME: This is filthy... - super(ImmutableMultiDict, ret).set_all(key, values) - return ret - - def with_insert(self, index, key, value): - """ - Returns: - An updated ImmutableMultiDict. The original object will not be modified. - """ - ret = self.copy() - # FIXME: This is filthy... - super(ImmutableMultiDict, ret).insert(index, key, value) - return ret - - class MultiDictView(_MultiDict): """ The MultiDictView provides the MultiDict interface over calculated data. diff --git a/test/mitmproxy/test_types_multidict.py b/test/mitmproxy/test_types_multidict.py index d566905cb..cf80c05e9 100644 --- a/test/mitmproxy/test_types_multidict.py +++ b/test/mitmproxy/test_types_multidict.py @@ -12,10 +12,6 @@ class TMultiDict(_TMulti, multidict.MultiDict): pass -class TImmutableMultiDict(_TMulti, multidict.ImmutableMultiDict): - pass - - class TestMultiDict: @staticmethod def _multi(): @@ -194,36 +190,6 @@ class TestMultiDict: assert md == md2 -class TestImmutableMultiDict: - def test_modify(self): - md = TImmutableMultiDict() - with tutils.raises(TypeError): - md["foo"] = "bar" - - with tutils.raises(TypeError): - del md["foo"] - - with tutils.raises(TypeError): - md.add("foo", "bar") - - def test_hash(self): - assert hash(TImmutableMultiDict()) - - def test_with_delitem(self): - md = TImmutableMultiDict([("foo", "bar")]) - assert md.with_delitem("foo").fields == () - assert md.fields == (("foo", "bar"),) - - def test_with_set_all(self): - md = TImmutableMultiDict() - assert md.with_set_all("foo", ["bar"]).fields == (("foo", "bar"),) - assert md.fields == () - - def test_with_insert(self): - md = TImmutableMultiDict() - assert md.with_insert(0, "foo", "bar").fields == (("foo", "bar"),) - - class TParent: def __init__(self): self.vals = tuple() From cc8b422d9d4cdf5e6933567ec7be45f59ca95f3a Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Fri, 21 Oct 2016 11:41:54 +1300 Subject: [PATCH 2/2] multidict: remove to_dict We never use it, and it is dangerously ambiguous when a key is associated with a list. --- mitmproxy/types/multidict.py | 21 --------------------- test/mitmproxy/test_types_multidict.py | 7 ------- 2 files changed, 28 deletions(-) diff --git a/mitmproxy/types/multidict.py b/mitmproxy/types/multidict.py index e02d28a9c..31a1f22b7 100644 --- a/mitmproxy/types/multidict.py +++ b/mitmproxy/types/multidict.py @@ -174,27 +174,6 @@ class _MultiDict(MutableMapping, serializable.Serializable, metaclass=ABCMeta): coll.append([key, values]) return coll - def to_dict(self): - """ - Get the MultiDict as a plain Python dict. - Keys with multiple values are returned as lists. - - Example: - - .. code-block:: python - - # Simple dict with duplicate values. - >>> d = MultiDict([("name", "value"), ("a", False), ("a", 42)]) - >>> d.to_dict() - { - "name": "value", - "a": [False, 42] - } - """ - return { - k: v for k, v in self.collect() - } - def get_state(self): return self.fields diff --git a/test/mitmproxy/test_types_multidict.py b/test/mitmproxy/test_types_multidict.py index cf80c05e9..e0bbc9b14 100644 --- a/test/mitmproxy/test_types_multidict.py +++ b/test/mitmproxy/test_types_multidict.py @@ -172,13 +172,6 @@ class TestMultiDict: assert list(md.items()) == [("foo", "bar"), ("bar", "baz")] assert list(md.items(multi=True)) == [("foo", "bar"), ("bar", "baz"), ("Bar", "bam")] - def test_to_dict(self): - md = self._multi() - assert md.to_dict() == { - "foo": "bar", - "bar": ["baz", "bam"] - } - def test_state(self): md = self._multi() assert len(md.get_state()) == 3