Merge pull request #2186 from cortesi/tighten

Tighten some options-related functionality

18 files changed, 103 insertions, 75 deletions

diff --git a/mitmproxy/addons/dumper.py b/mitmproxy/addons/dumper.py
index 222f1167..ca0d32d3 100644
--- a/mitmproxy/addons/dumper.py
+++ b/mitmproxy/addons/dumper.py
@@ -35,12 +35,12 @@ class Dumper:
         self.default_contentview = "auto"  # type: str
 
     def configure(self, options, updated):
-        if "filtstr" in updated:
-            if options.filtstr:
-                self.filter = flowfilter.parse(options.filtstr)
+        if "view_filter" in updated:
+            if options.view_filter:
+                self.filter = flowfilter.parse(options.view_filter)
                 if not self.filter:
                     raise exceptions.OptionsError(
-                        "Invalid filter expression: %s" % options.filtstr
+                        "Invalid filter expression: %s" % options.view_filter
                     )
             else:
                 self.filter = None
diff --git a/mitmproxy/addons/streamfile.py b/mitmproxy/addons/streamfile.py
index 624297f2..183d2036 100644
--- a/mitmproxy/addons/streamfile.py
+++ b/mitmproxy/addons/streamfile.py
@@ -22,12 +22,12 @@ class StreamFile:
 
     def configure(self, options, updated):
         # We're already streaming - stop the previous stream and restart
-        if "filtstr" in updated:
-            if options.filtstr:
-                self.filt = flowfilter.parse(options.filtstr)
+        if "streamfile_filter" in updated:
+            if options.streamfile_filter:
+                self.filt = flowfilter.parse(options.streamfile_filter)
                 if not self.filt:
                     raise exceptions.OptionsError(
-                        "Invalid filter specification: %s" % options.filtstr
+                        "Invalid filter specification: %s" % options.streamfile_filter
                     )
             else:
                 self.filt = None
diff --git a/mitmproxy/addons/view.py b/mitmproxy/addons/view.py
index 2218327c..1b8a30e4 100644
--- a/mitmproxy/addons/view.py
+++ b/mitmproxy/addons/view.py
@@ -109,7 +109,7 @@ class View(collections.Sequence):
 
         self.default_order = OrderRequestStart(self)
         self.orders = dict(
-            time = self.default_order,
+            time = OrderRequestStart(self),
             method = OrderRequestMethod(self),
             url = OrderRequestURL(self),
             size = OrderKeySize(self),
@@ -300,24 +300,21 @@ class View(collections.Sequence):
 
     # Event handlers
     def configure(self, opts, updated):
-        if "filter" in updated:
+        if "view_filter" in updated:
             filt = None
-            if opts.filter:
-                filt = flowfilter.parse(opts.filter)
+            if opts.view_filter:
+                filt = flowfilter.parse(opts.view_filter)
                 if not filt:
                     raise exceptions.OptionsError(
-                        "Invalid interception filter: %s" % opts.filter
+                        "Invalid interception filter: %s" % opts.view_filter
                     )
             self.set_filter(filt)
         if "console_order" in updated:
-            if opts.console_order is None:
-                self.set_order(self.default_order)
-            else:
-                if opts.console_order not in self.orders:
-                    raise exceptions.OptionsError(
-                        "Unknown flow order: %s" % opts.console_order
-                    )
-                self.set_order(self.orders[opts.console_order])
+            if opts.console_order not in self.orders:
+                raise exceptions.OptionsError(
+                    "Unknown flow order: %s" % opts.console_order
+                )
+            self.set_order(self.orders[opts.console_order])
         if "console_order_reversed" in updated:
             self.set_reversed(opts.console_order_reversed)
         if "console_focus_follow" in updated:
diff --git a/mitmproxy/options.py b/mitmproxy/options.py
index 9232378f..8f8c1484 100644
--- a/mitmproxy/options.py
+++ b/mitmproxy/options.py
@@ -1,6 +1,7 @@
 from typing import Optional, Sequence
 
 from mitmproxy import optmanager
+from mitmproxy import contentviews
 from mitmproxy.net import tcp
 
 # We redefine these here for now to avoid importing Urwid-related guff on
@@ -154,13 +155,18 @@ class Options(optmanager.OptManager):
         )
         self.add_option(
             "default_contentview", str, "auto",
-            "The default content view mode."
+            "The default content view mode.",
+            choices = [i.name.lower() for i in contentviews.views]
         )
         self.add_option(
             "streamfile", Optional[str], None,
             "Write flows to file. Prefix path with + to append."
         )
         self.add_option(
+            "streamfile_filter", Optional[str], None,
+            "Filter which flows are written to file."
+        )
+        self.add_option(
             "server_replay_ignore_content", bool, False,
             "Ignore request's content while searching for a saved flow to replay."
         )
@@ -386,7 +392,7 @@ class Options(optmanager.OptManager):
             "Console mouse interaction."
         )
         self.add_option(
-            "console_order", Optional[str], None,
+            "console_order", str, "time",
             "Flow sort order.",
             choices=view_orders,
         )
@@ -396,8 +402,8 @@ class Options(optmanager.OptManager):
         )
 
         self.add_option(
-            "filter", Optional[str], None,
-            "Filter view expression."
+            "view_filter", Optional[str], None,
+            "Limit which flows are displayed."
         )
 
         # Web options
@@ -420,10 +426,6 @@ class Options(optmanager.OptManager):
 
         # Dump options
         self.add_option(
-            "filtstr", Optional[str], None,
-            "The filter string for mitmdump."
-        )
-        self.add_option(
             "flow_detail", int, 1,
             "Flow detail display level."
         )
diff --git a/mitmproxy/optmanager.py b/mitmproxy/optmanager.py
index f1d6461d..c58bc7d0 100644
--- a/mitmproxy/optmanager.py
+++ b/mitmproxy/optmanager.py
@@ -196,7 +196,7 @@ class OptManager:
                 unknown[k] = v
         updated = set(known.keys())
         if updated:
-            with self.rollback(updated):
+            with self.rollback(updated, reraise=True):
                 for k, v in known.items():
                     self._options[k].set(v)
                 self.changed.send(self, updated=updated)
diff --git a/mitmproxy/tools/cmdline.py b/mitmproxy/tools/cmdline.py
index da091c12..fbdbce52 100644
--- a/mitmproxy/tools/cmdline.py
+++ b/mitmproxy/tools/cmdline.py
@@ -109,7 +109,7 @@ def mitmproxy(opts):
         "See help in mitmproxy for filter expression syntax."
     )
     opts.make_parser(group, "intercept", metavar="FILTER")
-    opts.make_parser(group, "filter", metavar="FILTER")
+    opts.make_parser(group, "view_filter", metavar="FILTER")
     return parser
 
 
@@ -122,8 +122,8 @@ def mitmdump(opts):
         'filter_args',
         nargs="...",
         help="""
-            Filter view expression, used to only show flows that match a certain
-            filter. See help in mitmproxy for filter expression syntax.
+            Filter expression, equivalent to setting both the view_filter
+            and streamfile_filter options.
         """
     )
     return parser
diff --git a/mitmproxy/tools/console/flowlist.py b/mitmproxy/tools/console/flowlist.py
index 5fe86975..04052ec8 100644
--- a/mitmproxy/tools/console/flowlist.py
+++ b/mitmproxy/tools/console/flowlist.py
@@ -366,8 +366,8 @@ class FlowListBox(urwid.ListBox):
         elif key == "f":
             signals.status_prompt.send(
                 prompt = "Filter View",
-                text = self.master.options.filter,
-                callback = self.master.options.setter("filter")
+                text = self.master.options.view_filter,
+                callback = self.master.options.setter("view_filter")
             )
         elif key == "L":
             signals.status_prompt_path.send(
diff --git a/mitmproxy/tools/console/flowview.py b/mitmproxy/tools/console/flowview.py
index 90cca1c5..ba41c947 100644
--- a/mitmproxy/tools/console/flowview.py
+++ b/mitmproxy/tools/console/flowview.py
@@ -15,6 +15,7 @@ from mitmproxy.net.http import status_codes
 from mitmproxy.tools.console import common
 from mitmproxy.tools.console import flowdetailview
 from mitmproxy.tools.console import grideditor
+from mitmproxy.tools.console import overlay
 from mitmproxy.tools.console import searchable
 from mitmproxy.tools.console import signals
 from mitmproxy.tools.console import tabs
@@ -237,11 +238,10 @@ class FlowView(tabs.Tabs):
         return description, text_objects
 
     def viewmode_get(self):
-        override = self.view.settings[self.flow].get(
+        return self.view.settings[self.flow].get(
             (self.tab_offset, "prettyview"),
-            None
+            self.master.options.default_contentview
         )
-        return self.master.options.default_contentview if override is None else override
 
     def conn_text(self, conn):
         if conn:
@@ -483,11 +483,8 @@ class FlowView(tabs.Tabs):
         return self._view_nextprev_flow(self.view.index(flow) - 1, flow)
 
     def change_this_display_mode(self, t):
-        view = contentviews.get_by_shortcut(t)
-        if view:
-            self.view.settings[self.flow][(self.tab_offset, "prettyview")] = view.name
-        else:
-            self.view.settings[self.flow][(self.tab_offset, "prettyview")] = None
+        view = contentviews.get(t)
+        self.view.settings[self.flow][(self.tab_offset, "prettyview")] = view.name.lower()
         signals.flow_change.send(self, flow=self.flow)
 
     def keypress(self, size, key):
@@ -607,13 +604,14 @@ class FlowView(tabs.Tabs):
             signals.flow_change.send(self, flow = self.flow)
             signals.status_message.send(message="Loading all body data...")
         elif key == "m":
-            p = list(contentviews.view_prompts)
-            p.insert(0, ("Clear", "C"))
-            signals.status_prompt_onekey.send(
-                self,
-                prompt = "Display mode",
-                keys = p,
-                callback = self.change_this_display_mode
+            opts = [i.name.lower() for i in contentviews.views]
+            self.master.overlay(
+                overlay.Chooser(
+                    "display mode",
+                    opts,
+                    self.viewmode_get(),
+                    self.change_this_display_mode
+                )
             )
         elif key == "E":
             if self.tab_offset == TAB_REQ:
diff --git a/mitmproxy/tools/console/grideditor/editors.py b/mitmproxy/tools/console/grideditor/editors.py
index 39e51b2b..313495e4 100644
--- a/mitmproxy/tools/console/grideditor/editors.py
+++ b/mitmproxy/tools/console/grideditor/editors.py
@@ -258,7 +258,10 @@ class OptionsEditor(base.GridEditor):
         super().__init__(master, [[i] for i in vals], self.callback)
 
     def callback(self, vals):
-        setattr(self.master.options, self.name, [i[0] for i in vals])
+        try:
+            setattr(self.master.options, self.name, [i[0] for i in vals])
+        except exceptions.OptionsError as v:
+            signals.status_message.send(message=str(v))
 
     def is_error(self, col, val):
         pass
diff --git a/mitmproxy/tools/console/options.py b/mitmproxy/tools/console/options.py
index f38550f9..56d22715 100644
--- a/mitmproxy/tools/console/options.py
+++ b/mitmproxy/tools/console/options.py
@@ -30,6 +30,7 @@ def _mkhelp():
     keys = [
         ("enter", "edit option"),
         ("D", "reset all to defaults"),
+        ("d", "reset this option to default"),
         ("w", "save options"),
     ]
     text.extend(common.format_keyvals(keys, key="key", val="text", indent=4))
@@ -107,6 +108,7 @@ class OptionItem(urwid.WidgetWrap):
     def keypress(self, size, key):
         if self.editing:
             self._w[1].keypress(size, key)
+            return
         return key
 
 
@@ -184,15 +186,21 @@ class OptionsList(urwid.ListBox):
                 v = self.walker.get_edit_text()
                 try:
                     d = self.master.options.parse_setval(foc.opt.name, v)
+                    self.master.options.update(**{foc.opt.name: d})
                 except exceptions.OptionsError as v:
                     signals.status_message.send(message=str(v))
-                else:
-                    self.master.options.update(**{foc.opt.name: d})
                 self.walker.stop_editing()
             elif key == "esc":
                 self.walker.stop_editing()
         else:
-            if key == "g":
+            if key == "d":
+                foc, idx = self.get_focus()
+                setattr(
+                    self.master.options,
+                    foc.opt.name,
+                    self.master.options.default(foc.opt.name)
+                )
+            elif key == "g":
                 self.set_focus(0)
                 self.walker._modified()
             elif key == "G":
diff --git a/mitmproxy/tools/console/statusbar.py b/mitmproxy/tools/console/statusbar.py
index 3f18bbb3..c7132864 100644
--- a/mitmproxy/tools/console/statusbar.py
+++ b/mitmproxy/tools/console/statusbar.py
@@ -191,10 +191,10 @@ class StatusBar(urwid.WidgetWrap):
             r.append("[")
             r.append(("heading_key", "i"))
             r.append(":%s]" % self.master.options.intercept)
-        if self.master.options.filter:
+        if self.master.options.view_filter:
             r.append("[")
             r.append(("heading_key", "f"))
-            r.append(":%s]" % self.master.options.filter)
+            r.append(":%s]" % self.master.options.view_filter)
         if self.master.options.stickycookie:
             r.append("[")
             r.append(("heading_key", "t"))
@@ -207,7 +207,7 @@ class StatusBar(urwid.WidgetWrap):
             r.append("[")
             r.append(("heading_key", "M"))
             r.append(":%s]" % self.master.options.default_contentview)
-        if self.master.options.console_order:
+        if self.master.options.has_changed("console_order"):
             r.append("[")
             r.append(("heading_key", "o"))
             r.append(":%s]" % self.master.options.console_order)
diff --git a/mitmproxy/tools/main.py b/mitmproxy/tools/main.py
index b321e8f8..6db232fc 100644
--- a/mitmproxy/tools/main.py
+++ b/mitmproxy/tools/main.py
@@ -60,7 +60,11 @@ def process_options(parser, opts, args):
         return server.DummyServer(pconf)
 
 
-def run(MasterKlass, args):  # pragma: no cover
+def run(MasterKlass, args, extra=None):  # pragma: no cover
+    """
+        extra: Extra argument processing callable which returns a dict of
+        options.
+    """
     version_check.check_pyopenssl_version()
     debug.register_info_dumpers()
 
@@ -80,6 +84,8 @@ def run(MasterKlass, args):  # pragma: no cover
             print(optmanager.dump_defaults(opts))
             sys.exit(0)
         opts.set(*args.setoptions)
+        if extra:
+            opts.update(**extra(args))
 
         def cleankill(*args, **kwargs):
             master.shutdown()
@@ -107,7 +113,17 @@ def mitmproxy(args=None):  # pragma: no cover
 
 def mitmdump(args=None):  # pragma: no cover
     from mitmproxy.tools import dump
-    m = run(dump.DumpMaster, args)
+
+    def extra(args):
+        if args.filter_args:
+            v = " ".join(args.filter_args)
+            return dict(
+                view_filter = v,
+                streamfile_filter = v,
+            )
+        return {}
+
+    m = run(dump.DumpMaster, args, extra)
     if m and m.errorcheck.has_errored:
         sys.exit(1)
 
diff --git a/mitmproxy/utils/sliding_window.py b/mitmproxy/utils/sliding_window.py
index 4714b8e3..0a65f5e4 100644
--- a/mitmproxy/utils/sliding_window.py
+++ b/mitmproxy/utils/sliding_window.py
@@ -1,10 +1,10 @@
 import itertools
-from typing import TypeVar, Iterator, Tuple, Optional
+from typing import TypeVar, Iterable, Iterator, Tuple, Optional
 
 T = TypeVar('T')
 
 
-def window(iterator: Iterator[T], behind: int = 0, ahead: int = 0) -> Iterator[Tuple[Optional[T]]]:
+def window(iterator: Iterable[T], behind: int = 0, ahead: int = 0) -> Iterator[Tuple[Optional[T], ...]]:
     """
     Sliding window for an iterator.
 
diff --git a/test/mitmproxy/addons/test_dumper.py b/test/mitmproxy/addons/test_dumper.py
index fbcc4d16..d2cefe79 100644
--- a/test/mitmproxy/addons/test_dumper.py
+++ b/test/mitmproxy/addons/test_dumper.py
@@ -16,7 +16,7 @@ from mitmproxy import options
 def test_configure():
     d = dumper.Dumper()
     with taddons.context(options=options.Options()) as ctx:
-        ctx.configure(d, filtstr="~b foo")
+        ctx.configure(d, view_filter="~b foo")
         assert d.filter
 
         f = tflow.tflow(resp=True)
@@ -24,10 +24,10 @@ def test_configure():
         f.response.content = b"foo"
         assert d.match(f)
 
-        ctx.configure(d, filtstr=None)
+        ctx.configure(d, view_filter=None)
         assert not d.filter
         with pytest.raises(exceptions.OptionsError):
-            ctx.configure(d, filtstr="~~")
+            ctx.configure(d, view_filter="~~")
         assert not d.filter
 
 
diff --git a/test/mitmproxy/addons/test_streamfile.py b/test/mitmproxy/addons/test_streamfile.py
index 3f78521c..bcb27c79 100644
--- a/test/mitmproxy/addons/test_streamfile.py
+++ b/test/mitmproxy/addons/test_streamfile.py
@@ -15,10 +15,12 @@ def test_configure(tmpdir):
         with pytest.raises(exceptions.OptionsError):
             tctx.configure(sa, streamfile=str(tmpdir))
         with pytest.raises(Exception, match="Invalid filter"):
-            tctx.configure(sa, streamfile=str(tmpdir.join("foo")), filtstr="~~")
-        tctx.configure(sa, filtstr="foo")
+            tctx.configure(
+                sa, streamfile=str(tmpdir.join("foo")), streamfile_filter="~~"
+            )
+        tctx.configure(sa, streamfile_filter="foo")
         assert sa.filt
-        tctx.configure(sa, filtstr=None)
+        tctx.configure(sa, streamfile_filter=None)
         assert not sa.filt
 
 
diff --git a/test/mitmproxy/addons/test_view.py b/test/mitmproxy/addons/test_view.py
index b7842314..7fa3819e 100644
--- a/test/mitmproxy/addons/test_view.py
+++ b/test/mitmproxy/addons/test_view.py
@@ -264,7 +264,7 @@ def test_signals():
 def test_focus_follow():
     v = view.View()
     with taddons.context(options=options.Options()) as tctx:
-        tctx.configure(v, console_focus_follow=True, filter="~m get")
+        tctx.configure(v, console_focus_follow=True, view_filter="~m get")
 
         v.add(tft(start=5))
         assert v.focus.index == 0
@@ -378,9 +378,9 @@ def test_settings():
 def test_configure():
     v = view.View()
     with taddons.context(options=options.Options()) as tctx:
-        tctx.configure(v, filter="~q")
+        tctx.configure(v, view_filter="~q")
         with pytest.raises(Exception, match="Invalid interception filter"):
-            tctx.configure(v, filter="~~")
+            tctx.configure(v, view_filter="~~")
 
         tctx.configure(v, console_order="method")
         with pytest.raises(Exception, match="Unknown flow order"):
@@ -388,7 +388,5 @@ def test_configure():
 
         tctx.configure(v, console_order_reversed=True)
 
-        tctx.configure(v, console_order=None)
-
         tctx.configure(v, console_focus_follow=True)
         assert v.focus_follow
diff --git a/test/mitmproxy/test_optmanager.py b/test/mitmproxy/test_optmanager.py
index ef5ebd27..01636640 100644
--- a/test/mitmproxy/test_optmanager.py
+++ b/test/mitmproxy/test_optmanager.py
@@ -210,8 +210,12 @@ def test_rollback():
     o.errored.connect(errsub)
 
     assert o.one is None
-    o.one = 10
-    o.bool = True
+    with pytest.raises(exceptions.OptionsError):
+        o.one = 10
+    assert o.one is None
+    with pytest.raises(exceptions.OptionsError):
+        o.bool = True
+    assert o.bool is False
     assert isinstance(recerr[0]["exc"], exceptions.OptionsError)
     assert o.one is None
     assert o.bool is False
diff --git a/test/mitmproxy/tools/test_dump.py b/test/mitmproxy/tools/test_dump.py
index 8e2fa5b2..69a76d2e 100644
--- a/test/mitmproxy/tools/test_dump.py
+++ b/test/mitmproxy/tools/test_dump.py
@@ -12,7 +12,7 @@ from .. import tservers
 
 class TestDumpMaster(tservers.MasterTest):
     def mkmaster(self, flt, **opts):
-        o = options.Options(filtstr=flt, verbosity=-1, flow_detail=0, **opts)
+        o = options.Options(view_filter=flt, verbosity=-1, flow_detail=0, **opts)
         m = dump.DumpMaster(o, proxy.DummyServer(), with_termlog=False, with_dumper=False)
         return m