From a20f8e9620c0cfcb40500113cbeb813a05a195bb Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Wed, 13 Jul 2016 18:45:50 +1200 Subject: [PATCH] More powerful Options scheme This prepares us for the addon configuration mechanism and gives us a more flexible way to handle options changes. This changeset should spell the end of the current anti-pattern in our codebase where we duplicate data out of options onto the master when mutability is needed. From now on, Options can be the one source of thruth. - Change notifications - Rollback on error --- mitmproxy/console/master.py | 10 +---- mitmproxy/dump.py | 10 +---- mitmproxy/exceptions.py | 4 ++ mitmproxy/options.py | 66 +++++++++++++++++++++++++++++++++ mitmproxy/web/master.py | 11 +----- test/mitmproxy/test_options.py | 67 ++++++++++++++++++++++++++++++++++ 6 files changed, 143 insertions(+), 25 deletions(-) create mode 100644 mitmproxy/options.py create mode 100644 test/mitmproxy/test_options.py diff --git a/mitmproxy/console/master.py b/mitmproxy/console/master.py index 93b5766d8..99af07225 100644 --- a/mitmproxy/console/master.py +++ b/mitmproxy/console/master.py @@ -20,6 +20,7 @@ from mitmproxy import controller from mitmproxy import exceptions from mitmproxy import flow from mitmproxy import script +import mitmproxy.options from mitmproxy.console import flowlist from mitmproxy.console import flowview from mitmproxy.console import grideditor @@ -175,7 +176,7 @@ class ConsoleState(flow.State): self.add_flow_setting(flow, "marked", marked) -class Options(object): +class Options(mitmproxy.options.Options): attributes = [ "app", "app_domain", @@ -210,13 +211,6 @@ class Options(object): "outfile", ] - def __init__(self, **kwargs): - for k, v in kwargs.items(): - setattr(self, k, v) - for i in self.attributes: - if not hasattr(self, i): - setattr(self, i, None) - class ConsoleMaster(flow.FlowMaster): palette = [] diff --git a/mitmproxy/dump.py b/mitmproxy/dump.py index 10465b636..bfefb319d 100644 --- a/mitmproxy/dump.py +++ b/mitmproxy/dump.py @@ -11,6 +11,7 @@ from mitmproxy import controller from mitmproxy import exceptions from mitmproxy import filt from mitmproxy import flow +from mitmproxy import options from netlib import human from netlib import tcp from netlib import strutils @@ -20,7 +21,7 @@ class DumpError(Exception): pass -class Options(object): +class Options(options.Options): attributes = [ "app", "app_host", @@ -53,13 +54,6 @@ class Options(object): "replay_ignore_host" ] - def __init__(self, **kwargs): - for k, v in kwargs.items(): - setattr(self, k, v) - for i in self.attributes: - if not hasattr(self, i): - setattr(self, i, None) - class DumpMaster(flow.FlowMaster): diff --git a/mitmproxy/exceptions.py b/mitmproxy/exceptions.py index 63bd8d3d0..282784b6d 100644 --- a/mitmproxy/exceptions.py +++ b/mitmproxy/exceptions.py @@ -95,3 +95,7 @@ class FlowReadException(ProxyException): class ControlException(ProxyException): pass + + +class OptionsError(Exception): + pass diff --git a/mitmproxy/options.py b/mitmproxy/options.py new file mode 100644 index 000000000..7389df1f5 --- /dev/null +++ b/mitmproxy/options.py @@ -0,0 +1,66 @@ +from __future__ import absolute_import, print_function, division + +import contextlib +import blinker +import pprint + +from mitmproxy import exceptions + + +class Options(object): + """ + .changed is a blinker Signal that triggers whenever options are + updated. If any handler in the chain raises an exceptions.OptionsError + exception, all changes are rolled back, the exception is suppressed, + and the .errored signal is notified. + """ + attributes = [] + + def __init__(self, **kwargs): + self.__dict__["changed"] = blinker.Signal() + self.__dict__["errored"] = blinker.Signal() + self.__dict__["_opts"] = dict([(i, None) for i in self.attributes]) + for k, v in kwargs.items(): + self._opts[k] = v + + @contextlib.contextmanager + def rollback(self): + old = self._opts.copy() + try: + yield + except exceptions.OptionsError as e: + # Notify error handlers + self.errored.send(self, exc=e) + # Rollback + self.__dict__["_opts"] = old + self.changed.send(self) + + def __eq__(self, other): + return self._opts == other._opts + + def __copy__(self): + return self.__class__(**self._opts) + + def __getattr__(self, attr): + return self._opts[attr] + + def __setattr__(self, attr, value): + if attr not in self._opts: + raise KeyError("No such option: %s" % attr) + with self.rollback(): + self._opts[attr] = value + self.changed.send(self) + + def get(self, k, d=None): + return self._opts.get(k, d) + + def update(self, **kwargs): + for k in kwargs: + if k not in self._opts: + raise KeyError("No such option: %s" % k) + with self.rollback(): + self._opts.update(kwargs) + self.changed.send(self) + + def __repr__(self): + return pprint.pformat(self._opts) diff --git a/mitmproxy/web/master.py b/mitmproxy/web/master.py index 737bb95f4..2b55e74e4 100644 --- a/mitmproxy/web/master.py +++ b/mitmproxy/web/master.py @@ -9,6 +9,7 @@ import tornado.ioloop from mitmproxy import controller from mitmproxy import exceptions from mitmproxy import flow +from mitmproxy import options from mitmproxy.web import app from netlib.http import authentication @@ -88,7 +89,7 @@ class WebState(flow.State): ) -class Options(object): +class Options(options.Options): attributes = [ "app", "app_domain", @@ -124,14 +125,6 @@ class Options(object): "wsingleuser", "whtpasswd", ] - - def __init__(self, **kwargs): - for k, v in kwargs.items(): - setattr(self, k, v) - for i in self.attributes: - if not hasattr(self, i): - setattr(self, i, None) - def process_web_options(self, parser): if self.wsingleuser or self.whtpasswd: if self.wsingleuser: diff --git a/test/mitmproxy/test_options.py b/test/mitmproxy/test_options.py new file mode 100644 index 000000000..5fdb7abe9 --- /dev/null +++ b/test/mitmproxy/test_options.py @@ -0,0 +1,67 @@ +from __future__ import absolute_import, print_function, division +import copy + +from mitmproxy import options +from mitmproxy import exceptions +from netlib.tutils import raises + + +class TO(options.Options): + attributes = [ + "one", + "two" + ] + + +def test_options(): + o = TO(two="three") + assert o.one is None + assert o.two == "three" + o.one = "one" + assert o.one == "one" + raises("no such option", setattr, o, "nonexistent", "value") + raises("no such option", o.update, nonexistent = "value") + + rec = [] + + def sub(opts): + rec.append(copy.copy(opts)) + + o.changed.connect(sub) + + o.one = "ninety" + assert len(rec) == 1 + assert rec[-1].one == "ninety" + + o.update(one="oink") + assert len(rec) == 2 + assert rec[-1].one == "oink" + + +def test_rollback(): + o = TO(one="two") + + rec = [] + + def sub(opts): + rec.append(copy.copy(opts)) + + recerr = [] + + def errsub(opts, **kwargs): + recerr.append(kwargs) + + def err(opts): + if opts.one == "ten": + raise exceptions.OptionsError + + o.changed.connect(sub) + o.changed.connect(err) + o.errored.connect(errsub) + + o.one = "ten" + assert isinstance(recerr[0]["exc"], exceptions.OptionsError) + assert o.one == "two" + assert len(rec) == 2 + assert rec[0].one == "ten" + assert rec[1].one == "two"