From a20f8e9620c0cfcb40500113cbeb813a05a195bb Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Wed, 13 Jul 2016 18:45:50 +1200 Subject: 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 93b5766d..99af0722 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 10465b63..bfefb319 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 63bd8d3d..282784b6 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 00000000..7389df1f --- /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 737bb95f..2b55e74e 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 00000000..5fdb7abe --- /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" -- cgit v1.2.3 From c9a0fe6a0e6d70fa2aea1b8dc337609a9439ade1 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Wed, 13 Jul 2016 19:05:32 +1200 Subject: Show how options integrates with console This is not functional at the moment, because all mutable options are still on master. --- mitmproxy/console/master.py | 8 ++++++++ mitmproxy/console/options.py | 1 + mitmproxy/console/statusbar.py | 1 + mitmproxy/web/master.py | 1 + 4 files changed, 11 insertions(+) diff --git a/mitmproxy/console/master.py b/mitmproxy/console/master.py index 99af0722..1daf1127 100644 --- a/mitmproxy/console/master.py +++ b/mitmproxy/console/master.py @@ -217,8 +217,10 @@ class ConsoleMaster(flow.FlowMaster): def __init__(self, server, options): flow.FlowMaster.__init__(self, server, ConsoleState()) + self.stream_path = None self.options = options + self.options.errored.connect(self.options_error) if options.replacements: for i in options.replacements: @@ -298,6 +300,12 @@ class ConsoleMaster(flow.FlowMaster): self.__dict__[name] = value signals.update_settings.send(self) + def options_error(self, opts, exc): + signals.status_message.send( + message=str(exc), + expire=1 + ) + def load_script(self, command, use_reloader=True): # We default to using the reloader in the console ui. return super(ConsoleMaster, self).load_script(command, use_reloader) diff --git a/mitmproxy/console/options.py b/mitmproxy/console/options.py index 5a01c9d5..6a4b8dd6 100644 --- a/mitmproxy/console/options.py +++ b/mitmproxy/console/options.py @@ -140,6 +140,7 @@ class Options(urwid.WidgetWrap): ) self.master.loop.widget.footer.update("") signals.update_settings.connect(self.sig_update_settings) + master.options.changed.connect(self.sig_update_settings) def sig_update_settings(self, sender): self.lb.walker._modified() diff --git a/mitmproxy/console/statusbar.py b/mitmproxy/console/statusbar.py index e576b565..d1ab5906 100644 --- a/mitmproxy/console/statusbar.py +++ b/mitmproxy/console/statusbar.py @@ -122,6 +122,7 @@ class StatusBar(urwid.WidgetWrap): self._w = urwid.Pile([self.ib, self.ab]) signals.update_settings.connect(self.sig_update_settings) signals.flowlist_change.connect(self.sig_update_settings) + master.options.changed.connect(self.sig_update_settings) self.redraw() def sig_update_settings(self, sender): diff --git a/mitmproxy/web/master.py b/mitmproxy/web/master.py index 2b55e74e..008b74f8 100644 --- a/mitmproxy/web/master.py +++ b/mitmproxy/web/master.py @@ -125,6 +125,7 @@ class Options(options.Options): "wsingleuser", "whtpasswd", ] + def process_web_options(self, parser): if self.wsingleuser or self.whtpasswd: if self.wsingleuser: -- cgit v1.2.3 From 077850bd107d7ffe1cf3d4a8667bb04ea47beb96 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Wed, 13 Jul 2016 20:05:17 +1200 Subject: Raise AttributeError from Options.__getattr__ --- mitmproxy/options.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mitmproxy/options.py b/mitmproxy/options.py index 7389df1f..0cc5fee1 100644 --- a/mitmproxy/options.py +++ b/mitmproxy/options.py @@ -42,7 +42,10 @@ class Options(object): return self.__class__(**self._opts) def __getattr__(self, attr): - return self._opts[attr] + if attr in self._opts: + return self._opts[attr] + else: + raise AttributeError() def __setattr__(self, attr, value): if attr not in self._opts: -- cgit v1.2.3