Start consolidating core options

This is a preparatory patch that paves the way to consolidating our core
options in the core addon. It amalgamates the core_option_validation and core
addons, prepares the test suite for a world where options live in core, and
moves over two trivial options as a trial balloon.

From here, things will get harder, but at the end of the process we'll have a
core that's responsive to options.

mitmproxy/addons/__init__.py

@@ -4,7 +4,6 @@ from mitmproxy.addons import anticomp
 from mitmproxy.addons import browser
 from mitmproxy.addons import check_ca
 from mitmproxy.addons import clientplayback
-from mitmproxy.addons import core_option_validation
 from mitmproxy.addons import core
 from mitmproxy.addons import cut
 from mitmproxy.addons import disable_h2c
@@ -25,7 +24,6 @@ from mitmproxy.addons import upstream_auth
 def default_addons():
     return [
         core.Core(),
-        core_option_validation.CoreOptionValidation(),
         browser.Browser(),
         allowremote.AllowRemote(),
         anticache.AntiCache(),

mitmproxy/addons/core.py

@@ -1,15 +1,72 @@
 import typing
 
+from mitmproxy.utils import human
 from mitmproxy import ctx
 from mitmproxy import exceptions
 from mitmproxy import command
 from mitmproxy import flow
 from mitmproxy import optmanager
+from mitmproxy import platform
+from mitmproxy.net import server_spec
 from mitmproxy.net.http import status_codes
 import mitmproxy.types
 
 
+CA_DIR = "~/.mitmproxy"
+LISTEN_PORT = 8080
+
+
 class Core:
+    def load(self, loader):
+        loader.add_option(
+            "body_size_limit", typing.Optional[str], None,
+            """
+            Byte size limit of HTTP request and response bodies. Understands
+            k/m/g suffixes, i.e. 3m for 3 megabytes.
+            """
+        )
+        loader.add_option(
+            "keep_host_header", bool, False,
+            """
+            Reverse Proxy: Keep the original host header instead of rewriting it
+            to the reverse proxy target.
+            """
+        )
+
+    def configure(self, updated):
+        opts = ctx.options
+        if opts.add_upstream_certs_to_client_chain and not opts.upstream_cert:
+            raise exceptions.OptionsError(
+                "The no-upstream-cert and add-upstream-certs-to-client-chain "
+                "options are mutually exclusive. If no-upstream-cert is enabled "
+                "then the upstream certificate is not retrieved before generating "
+                "the client certificate chain."
+            )
+        if "body_size_limit" in updated:
+            try:
+                human.parse_size(opts.body_size_limit)
+            except ValueError as e:
+                raise exceptions.OptionsError(
+                    "Invalid body size limit specification: %s" %
+                    opts.body_size_limit
+                )
+        if "mode" in updated:
+            mode = opts.mode
+            if mode.startswith("reverse:") or mode.startswith("upstream:"):
+                try:
+                    server_spec.parse_with_mode(mode)
+                except ValueError as e:
+                    raise exceptions.OptionsError(str(e)) from e
+            elif mode == "transparent":
+                if not platform.original_addr:
+                    raise exceptions.OptionsError(
+                        "Transparent mode not supported on this platform."
+                    )
+            elif mode not in ["regular", "socks5"]:
+                raise exceptions.OptionsError(
+                    "Invalid mode specification: %s" % mode
+                )
+
     @command.command("set")
     def set(self, *spec: str) -> None:
         """

mitmproxy/addons/core_option_validation.py

@@ -1,45 +0,0 @@
-"""
-    The core addon is responsible for verifying core settings that are not
-    checked by other addons.
-"""
-from mitmproxy import exceptions
-from mitmproxy import platform
-from mitmproxy import ctx
-from mitmproxy.net import server_spec
-from mitmproxy.utils import human
-
-
-class CoreOptionValidation:
-    def configure(self, updated):
-        opts = ctx.options
-        if opts.add_upstream_certs_to_client_chain and not opts.upstream_cert:
-            raise exceptions.OptionsError(
-                "The no-upstream-cert and add-upstream-certs-to-client-chain "
-                "options are mutually exclusive. If no-upstream-cert is enabled "
-                "then the upstream certificate is not retrieved before generating "
-                "the client certificate chain."
-            )
-        if "body_size_limit" in updated:
-            try:
-                human.parse_size(opts.body_size_limit)
-            except ValueError as e:
-                raise exceptions.OptionsError(
-                    "Invalid body size limit specification: %s" %
-                    opts.body_size_limit
-                )
-        if "mode" in updated:
-            mode = opts.mode
-            if mode.startswith("reverse:") or mode.startswith("upstream:"):
-                try:
-                    server_spec.parse_with_mode(mode)
-                except ValueError as e:
-                    raise exceptions.OptionsError(str(e)) from e
-            elif mode == "transparent":
-                if not platform.original_addr:
-                    raise exceptions.OptionsError(
-                        "Transparent mode not supported on this platform."
-                    )
-            elif mode not in ["regular", "socks5"]:
-                raise exceptions.OptionsError(
-                    "Invalid mode specification: %s" % mode
-                )

mitmproxy/addons/upstream_auth.py

@@ -57,5 +57,5 @@ class UpstreamAuth():
         if self.auth:
             if f.mode == "upstream" and not f.server_conn.via:
                 f.request.headers["Proxy-Authorization"] = self.auth
-            elif ctx.options.mode == "reverse":
+            elif ctx.options.mode.startswith("reverse"):
                 f.request.headers["Proxy-Authorization"] = self.auth

mitmproxy/options.py

@@ -10,37 +10,6 @@ LISTEN_PORT = 8080
 
 class Options(optmanager.OptManager):
 
-    if False:
-        # This provides type hints for IDEs (e.g. PyCharm) and mypy.
-        # Autogenerated using test/helper_tools/typehints_for_options.py
-        add_upstream_certs_to_client_chain = None  # type: bool
-        body_size_limit = None  # type: Optional[str]
-        cadir = None  # type: str
-        certs = None  # type: Sequence[str]
-        ciphers_client = None  # type: Optional[str]
-        ciphers_server = None  # type: Optional[str]
-        client_certs = None  # type: Optional[str]
-        http2 = None  # type: bool
-        http2_priority = None  # type: bool
-        ignore_hosts = None  # type: Sequence[str]
-        keep_host_header = None  # type: bool
-        listen_host = None  # type: str
-        listen_port = None  # type: int
-        mode = None  # type: str
-        rawtcp = None  # type: bool
-        server = None  # type: bool
-        showhost = None  # type: bool
-        spoof_source_address = None  # type: bool
-        ssl_insecure = None  # type: bool
-        ssl_verify_upstream_trusted_ca = None  # type: Optional[str]
-        ssl_verify_upstream_trusted_cadir = None  # type: Optional[str]
-        ssl_version_client = None  # type: str
-        ssl_version_server = None  # type: str
-        tcp_hosts = None  # type: Sequence[str]
-        upstream_bind_address = None  # type: str
-        upstream_cert = None  # type: bool
-        websocket = None  # type: bool
-
     def __init__(self, **kwargs) -> None:
         super().__init__()
         self.add_option(
@@ -60,13 +29,6 @@ class Options(optmanager.OptManager):
             that will be served to the proxy client, as extras.
             """
         )
-        self.add_option(
-            "body_size_limit", Optional[str], None,
-            """
-            Byte size limit of HTTP request and response bodies. Understands
-            k/m/g suffixes, i.e. 3m for 3 megabytes.
-            """
-        )
         self.add_option(
             "cadir", str, CA_DIR,
             "Location of the default mitmproxy CA files."
@@ -128,13 +90,6 @@ class Options(optmanager.OptManager):
             "upstream_cert", bool, True,
             "Connect to upstream server to look up certificate details."
         )
-        self.add_option(
-            "keep_host_header", bool, False,
-            """
-            Reverse Proxy: Keep the original host header instead of rewriting it
-            to the reverse proxy target.
-            """
-        )
 
         self.add_option(
             "http2", bool, True,

mitmproxy/test/taddons.py

@@ -6,7 +6,7 @@ import mitmproxy.options
 from mitmproxy import addonmanager
 from mitmproxy import command
 from mitmproxy import eventsequence
-from mitmproxy.addons import script
+from mitmproxy.addons import script, core
 
 
 class TestAddons(addonmanager.AddonManager):
@@ -59,7 +59,7 @@ class context:
         provides a number of helper methods for common testing scenarios.
     """
 
-    def __init__(self, *addons, options=None):
+    def __init__(self, *addons, options=None, loadcore=True):
         options = options or mitmproxy.options.Options()
         self.master = RecordingMaster(
             options
@@ -67,6 +67,9 @@ class context:
         self.options = self.master.options
         self.wrapped = None
 
+        if loadcore:
+            self.master.addons.add(core.Core())
+
         for a in addons:
             self.master.addons.add(a)
 

mitmproxy/tools/cmdline.py

@@ -1,10 +1,10 @@
 import argparse
 import os
 
-from mitmproxy import options
+from mitmproxy.addons import core
 
 
-CONFIG_PATH = os.path.join(options.CA_DIR, "config.yaml")
+CONFIG_PATH = os.path.join(core.CA_DIR, "config.yaml")
 
 
 def common_options(parser, opts):

test/mitmproxy/addons/test_core.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from mitmproxy.addons import core
 from mitmproxy.test import taddons
 from mitmproxy.test import tflow
@@ -7,9 +9,7 @@ import pytest
 
 def test_set():
     sa = core.Core()
-    with taddons.context() as tctx:
+    with taddons.context(loadcore=False) as tctx:
-        tctx.master.addons.add(sa)
-
         assert tctx.master.options.server
         tctx.command(sa.set, "server=false")
         assert not tctx.master.options.server
@@ -20,7 +20,7 @@ def test_set():
 
 def test_resume():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow()
         assert not sa.resume([f])
         f.intercept()
@@ -30,7 +30,7 @@ def test_resume():
 
 def test_mark():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow()
         assert not f.marked
         sa.mark([f], True)
@@ -44,7 +44,7 @@ def test_mark():
 
 def test_kill():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow()
         f.intercept()
         assert f.killable
@@ -54,7 +54,7 @@ def test_kill():
 
 def test_revert():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow()
         f.backup()
         f.request.content = b"bar"
@@ -65,7 +65,7 @@ def test_revert():
 
 def test_flow_set():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow(resp=True)
         assert sa.flow_set_options()
 
@@ -101,7 +101,7 @@ def test_flow_set():
 
 def test_encoding():
     sa = core.Core()
-    with taddons.context():
+    with taddons.context(loadcore=False):
         f = tflow.tflow()
         assert sa.encode_options()
         sa.encode([f], "request", "deflate")
@@ -152,3 +152,40 @@ def test_options(tmpdir):
             f.write("'''")
         with pytest.raises(exceptions.CommandError):
             sa.options_load(p)
+
+
+def test_validation_simple():
+    sa = core.Core()
+    with taddons.context() as tctx:
+        with pytest.raises(exceptions.OptionsError):
+            tctx.configure(sa, body_size_limit = "invalid")
+        tctx.configure(sa, body_size_limit = "1m")
+
+        with pytest.raises(exceptions.OptionsError, match="mutually exclusive"):
+            tctx.configure(
+                sa,
+                add_upstream_certs_to_client_chain = True,
+                upstream_cert = False
+            )
+        with pytest.raises(exceptions.OptionsError, match="Invalid mode"):
+            tctx.configure(
+                sa,
+                mode = "Flibble"
+            )
+
+
+@mock.patch("mitmproxy.platform.original_addr", None)
+def test_validation_no_transparent():
+    sa = core.Core()
+    with taddons.context() as tctx:
+        with pytest.raises(Exception, match="Transparent mode not supported"):
+            tctx.configure(sa, mode = "transparent")
+
+
+@mock.patch("mitmproxy.platform.original_addr")
+def test_validation_modes(m):
+    sa = core.Core()
+    with taddons.context() as tctx:
+        tctx.configure(sa, mode = "reverse:http://localhost")
+        with pytest.raises(Exception, match="Invalid server specification"):
+            tctx.configure(sa, mode = "reverse:")

test/mitmproxy/addons/test_core_option_validation.py

@@ -1,42 +0,0 @@
-from mitmproxy import exceptions
-from mitmproxy.addons import core_option_validation
-from mitmproxy.test import taddons
-import pytest
-from unittest import mock
-
-
-def test_simple():
-    sa = core_option_validation.CoreOptionValidation()
-    with taddons.context() as tctx:
-        with pytest.raises(exceptions.OptionsError):
-            tctx.configure(sa, body_size_limit = "invalid")
-        tctx.configure(sa, body_size_limit = "1m")
-
-        with pytest.raises(exceptions.OptionsError, match="mutually exclusive"):
-            tctx.configure(
-                sa,
-                add_upstream_certs_to_client_chain = True,
-                upstream_cert = False
-            )
-        with pytest.raises(exceptions.OptionsError, match="Invalid mode"):
-            tctx.configure(
-                sa,
-                mode = "Flibble"
-            )
-
-
-@mock.patch("mitmproxy.platform.original_addr", None)
-def test_no_transparent():
-    sa = core_option_validation.CoreOptionValidation()
-    with taddons.context() as tctx:
-        with pytest.raises(Exception, match="Transparent mode not supported"):
-            tctx.configure(sa, mode = "transparent")
-
-
-@mock.patch("mitmproxy.platform.original_addr")
-def test_modes(m):
-    sa = core_option_validation.CoreOptionValidation()
-    with taddons.context() as tctx:
-        tctx.configure(sa, mode = "reverse:http://localhost")
-        with pytest.raises(Exception, match="Invalid server specification"):
-            tctx.configure(sa, mode = "reverse:")

test/mitmproxy/addons/test_proxyauth.py

@@ -49,7 +49,7 @@ class TestProxyAuth:
     ])
     def test_is_proxy_auth(self, mode, expected):
         up = proxyauth.ProxyAuth()
-        with taddons.context(up) as ctx:
+        with taddons.context(up, loadcore=False) as ctx:
             ctx.options.mode = mode
             assert up.is_proxy_auth() is expected
 
@@ -133,7 +133,7 @@ class TestProxyAuth:
 
     def test_authenticate(self):
         up = proxyauth.ProxyAuth()
-        with taddons.context(up) as ctx:
+        with taddons.context(up, loadcore=False) as ctx:
             ctx.configure(up, proxyauth="any", mode="regular")
 
             f = tflow.tflow()

test/mitmproxy/addons/test_script.py

@@ -189,7 +189,7 @@ class TestScriptLoader:
 
     def test_simple(self):
         sc = script.ScriptLoader()
-        with taddons.context() as tctx:
+        with taddons.context(loadcore=False) as tctx:
             tctx.master.addons.add(sc)
             sc.running()
             assert len(tctx.master.addons) == 1
@@ -231,7 +231,7 @@ class TestScriptLoader:
 
     def test_load_err(self):
         sc = script.ScriptLoader()
-        with taddons.context(sc) as tctx:
+        with taddons.context(sc, loadcore=False) as tctx:
             tctx.configure(sc, scripts=[
                 tutils.test_data.path("mitmproxy/data/addonscripts/load_error.py")
             ])

test/mitmproxy/addons/test_upstream_auth.py

@@ -41,7 +41,7 @@ def test_simple():
         up.requestheaders(f)
         assert "proxy-authorization" not in f.request.headers
 
-        tctx.configure(up, mode="reverse")
+        tctx.configure(up, mode="reverse:127.0.0.1")
         f = tflow.tflow()
         f.mode = "transparent"
         up.requestheaders(f)

test/mitmproxy/proxy/protocol/test_http2.py

@@ -10,6 +10,7 @@ import h2
 from mitmproxy import options
 
 import mitmproxy.net
+from mitmproxy.addons import core
 from ...net import tservers as net_tservers
 from mitmproxy import exceptions
 from mitmproxy.net.http import http1, http2
@@ -90,6 +91,7 @@ class _Http2TestBase:
     def setup_class(cls):
         cls.options = cls.get_options()
         tmaster = tservers.TestMaster(cls.options)
+        tmaster.addons.add(core.Core())
         cls.proxy = tservers.ProxyThread(tmaster)
         cls.proxy.start()
 

test/mitmproxy/proxy/protocol/test_websocket.py

@@ -6,6 +6,7 @@ import traceback
 
 from mitmproxy import options
 from mitmproxy import exceptions
+from mitmproxy.addons import core
 from mitmproxy.http import HTTPFlow
 from mitmproxy.websocket import WebSocketFlow
 
@@ -52,6 +53,7 @@ class _WebSocketTestBase:
     def setup_class(cls):
         cls.options = cls.get_options()
         tmaster = tservers.TestMaster(cls.options)
+        tmaster.addons.add(core.Core())
         cls.proxy = tservers.ProxyThread(tmaster)
         cls.proxy.start()
 

test/mitmproxy/proxy/test_server.py

@@ -10,7 +10,6 @@ from mitmproxy import certs
 from mitmproxy import exceptions
 from mitmproxy import http
 from mitmproxy import options
-from mitmproxy.addons import proxyauth
 from mitmproxy.addons import script
 from mitmproxy.net import socks
 from mitmproxy.net import tcp
@@ -306,46 +305,6 @@ class TestHTTP(tservers.HTTPProxyTest, CommonMixin):
             assert self.server.last_log()["request"]["first_line_format"] == "relative"
 
 
-class TestHTTPAuth(tservers.HTTPProxyTest):
-    def test_auth(self):
-        self.master.addons.add(proxyauth.ProxyAuth())
-        self.master.addons.trigger(
-            "configure", self.master.options.keys()
-        )
-        self.master.options.proxyauth = "test:test"
-        assert self.pathod("202").status_code == 407
-        p = self.pathoc()
-        with p.connect():
-            ret = p.request("""
-                get
-                'http://localhost:%s/p/202'
-                h'%s'='%s'
-            """ % (
-                self.server.port,
-                "Proxy-Authorization",
-                proxyauth.mkauth("test", "test")
-            ))
-        assert ret.status_code == 202
-
-
-class TestHTTPReverseAuth(tservers.ReverseProxyTest):
-    def test_auth(self):
-        self.master.addons.add(proxyauth.ProxyAuth())
-        self.master.options.proxyauth = "test:test"
-        assert self.pathod("202").status_code == 401
-        p = self.pathoc()
-        with p.connect():
-            ret = p.request("""
-                get
-                '/p/202'
-                h'%s'='%s'
-            """ % (
-                "Authorization",
-                proxyauth.mkauth("test", "test")
-            ))
-        assert ret.status_code == 202
-
-
 class TestHTTPS(tservers.HTTPProxyTest, CommonMixin, TcpMixin):
     ssl = True
     ssloptions = pathod.SSLOptions(request_client_cert=True)

test/mitmproxy/test_addonmanager.py

@@ -107,7 +107,7 @@ def test_loader():
 
 
 def test_simple():
-    with taddons.context() as tctx:
+    with taddons.context(loadcore=False) as tctx:
         a = tctx.master.addons
 
         assert len(a) == 0

test/mitmproxy/tservers.py

@@ -5,6 +5,7 @@ import sys
 from unittest import mock
 
 import mitmproxy.platform
+from mitmproxy.addons import core
 from mitmproxy.proxy.config import ProxyConfig
 from mitmproxy.proxy.server import ProxyServer
 from mitmproxy import controller
@@ -132,6 +133,7 @@ class ProxyTestBase:
 
         cls.options = cls.get_options()
         tmaster = cls.masterclass(cls.options)
+        tmaster.addons.add(core.Core())
         cls.proxy = ProxyThread(tmaster)
         cls.proxy.start()
 
@@ -343,6 +345,7 @@ class ChainProxyTest(ProxyTestBase):
         for _ in range(cls.n):
             opts = cls.get_options()
             tmaster = cls.masterclass(opts)
+            tmaster.addons.add(core.Core())
             proxy = ProxyThread(tmaster)
             proxy.start()
             cls.chain.insert(0, proxy)