Merge pull request #1382 from cortesi/cleanup

[wip] ProxyConf-related refactoring

5 files changed, 72 insertions, 61 deletions

diff --git a/mitmproxy/cmdline.py b/mitmproxy/cmdline.py
index 696542f6..b3d3e6dd 100644
--- a/mitmproxy/cmdline.py
+++ b/mitmproxy/cmdline.py
@@ -222,6 +222,9 @@ def get_common_options(args):
             "the client certificate chain."
         )
 
+    if args.quiet:
+        args.verbose = 0
+
     return dict(
         app=args.app,
         app_host=args.app_host,
diff --git a/mitmproxy/main.py b/mitmproxy/main.py
index 2d7299e4..6d44108e 100644
--- a/mitmproxy/main.py
+++ b/mitmproxy/main.py
@@ -30,25 +30,20 @@ def assert_utf8_env():
         sys.exit(1)
 
 
-def get_server(dummy_server, options):
-    if dummy_server:
-        return server.DummyServer(options)
-    else:
-        try:
-            return server.ProxyServer(options)
-        except exceptions.ServerException as v:
-            print(str(v), file=sys.stderr)
-            sys.exit(1)
-
-
 def process_options(parser, options, args):
     if args.sysinfo:
         print(debug.sysinfo())
         sys.exit(0)
-    if args.quiet:
-        args.verbose = 0
     debug.register_info_dumpers()
-    return config.ProxyConfig(options)
+    pconf = config.ProxyConfig(options)
+    if options.no_server:
+        return server.DummyServer(pconf)
+    else:
+        try:
+            return server.ProxyServer(pconf)
+        except exceptions.ServerException as v:
+            print(str(v), file=sys.stderr)
+            sys.exit(1)
 
 
 def mitmproxy(args=None):  # pragma: no cover
@@ -76,8 +71,7 @@ def mitmproxy(args=None):  # pragma: no cover
         console_options.limit = args.limit
         console_options.no_mouse = args.no_mouse
 
-        proxy_config = process_options(parser, console_options, args)
-        server = get_server(console_options.no_server, proxy_config)
+        server = process_options(parser, console_options, args)
         m = console.master.ConsoleMaster(server, console_options)
     except exceptions.OptionsError as e:
         print("mitmproxy: %s" % e, file=sys.stderr)
@@ -103,8 +97,7 @@ def mitmdump(args=None):  # pragma: no cover
         dump_options.flow_detail = args.flow_detail
         dump_options.keepserving = args.keepserving
         dump_options.filtstr = " ".join(args.args) if args.args else None
-        proxy_config = process_options(parser, dump_options, args)
-        server = get_server(dump_options.no_server, proxy_config)
+        server = process_options(parser, dump_options, args)
         master = dump.DumpMaster(server, dump_options)
 
         def cleankill(*args, **kwargs):
@@ -141,8 +134,7 @@ def mitmweb(args=None):  # pragma: no cover
         web_options.whtpasswd = args.whtpasswd
         web_options.process_web_options(parser)
 
-        proxy_config = process_options(parser, web_options, args)
-        server = get_server(web_options.no_server, proxy_config)
+        server = process_options(parser, web_options, args)
         m = web.master.WebMaster(server, web_options)
     except exceptions.OptionsError as e:
         print("mitmweb: %s" % e, file=sys.stderr)
diff --git a/test/mitmproxy/test_protocol_http2.py b/test/mitmproxy/test_protocol_http2.py
index a7a3ba3f..d11570d9 100644
--- a/test/mitmproxy/test_protocol_http2.py
+++ b/test/mitmproxy/test_protocol_http2.py
@@ -90,8 +90,8 @@ class _Http2TestBase(object):
     @classmethod
     def setup_class(cls):
         cls.masteroptions = options.Options()
-        cnf, opts = cls.get_proxy_config()
-        cls.config = ProxyConfig(opts, **cnf)
+        opts = cls.get_options()
+        cls.config = ProxyConfig(opts)
 
         tmaster = tservers.TestMaster(opts, cls.config)
         tmaster.start_app(APP_HOST, APP_PORT)
@@ -103,11 +103,10 @@ class _Http2TestBase(object):
         cls.proxy.shutdown()
 
     @classmethod
-    def get_proxy_config(cls):
+    def get_options(cls):
         opts = options.Options(listen_port=0, no_upstream_cert=False)
         opts.cadir = os.path.join(tempfile.gettempdir(), "mitmproxy")
-        d = dict()
-        return d, opts
+        return opts
 
     @property
     def master(self):
diff --git a/test/mitmproxy/test_server.py b/test/mitmproxy/test_server.py
index b8b057fd..233af597 100644
--- a/test/mitmproxy/test_server.py
+++ b/test/mitmproxy/test_server.py
@@ -486,11 +486,11 @@ class TestSocks5(tservers.SocksModeTest):
 class TestHttps2Http(tservers.ReverseProxyTest):
 
     @classmethod
-    def get_proxy_config(cls):
-        d, opts = super(TestHttps2Http, cls).get_proxy_config()
+    def get_options(cls):
+        opts = super(TestHttps2Http, cls).get_options()
         s = parse_server_spec(opts.upstream_server)
         opts.upstream_server = "http://%s" % s.address
-        return d, opts
+        return opts
 
     def pathoc(self, ssl, sni=None):
         """
@@ -766,9 +766,14 @@ class TestFakeResponse(tservers.HTTPProxyTest):
 
 class TestServerConnect(tservers.HTTPProxyTest):
     masterclass = MasterFakeResponse
-    no_upstream_cert = True
     ssl = True
 
+    @classmethod
+    def get_options(cls):
+        opts = tservers.HTTPProxyTest.get_options()
+        opts.no_upstream_cert = True
+        return opts
+
     def test_unnecessary_serverconnect(self):
         """A replayed/fake response with no_upstream_cert should not connect to an upstream server"""
         assert self.pathod("200").status_code == 200
@@ -1034,20 +1039,34 @@ class AddUpstreamCertsToClientChainMixin:
             if receivedCert.digest('sha256') == upstreamCert.digest('sha256'):
                 upstream_cert_found_in_client_chain = True
                 break
-        assert(upstream_cert_found_in_client_chain == self.add_upstream_certs_to_client_chain)
-
+        assert(upstream_cert_found_in_client_chain == self.master.options.add_upstream_certs_to_client_chain)
 
-class TestHTTPSAddUpstreamCertsToClientChainTrue(AddUpstreamCertsToClientChainMixin, tservers.HTTPProxyTest):
 
+class TestHTTPSAddUpstreamCertsToClientChainTrue(
+    AddUpstreamCertsToClientChainMixin,
+    tservers.HTTPProxyTest
+):
     """
-    If --add-server-certs-to-client-chain is True, then the client should receive the upstream server's certificates
+    If --add-server-certs-to-client-chain is True, then the client should
+    receive the upstream server's certificates
     """
-    add_upstream_certs_to_client_chain = True
-
+    @classmethod
+    def get_options(cls):
+        opts = super(tservers.HTTPProxyTest, cls).get_options()
+        opts.add_upstream_certs_to_client_chain = True
+        return opts
 
-class TestHTTPSAddUpstreamCertsToClientChainFalse(AddUpstreamCertsToClientChainMixin, tservers.HTTPProxyTest):
 
+class TestHTTPSAddUpstreamCertsToClientChainFalse(
+    AddUpstreamCertsToClientChainMixin,
+    tservers.HTTPProxyTest
+):
     """
-    If --add-server-certs-to-client-chain is False, then the client should not receive the upstream server's certificates
+    If --add-server-certs-to-client-chain is False, then the client should not
+    receive the upstream server's certificates
     """
-    add_upstream_certs_to_client_chain = False
+    @classmethod
+    def get_options(cls):
+        opts = super(tservers.HTTPProxyTest, cls).get_options()
+        opts.add_upstream_certs_to_client_chain = False
+        return opts
diff --git a/test/mitmproxy/tservers.py b/test/mitmproxy/tservers.py
index 495765da..30980e40 100644
--- a/test/mitmproxy/tservers.py
+++ b/test/mitmproxy/tservers.py
@@ -77,8 +77,8 @@ class ProxyTestBase(object):
     # Test Configuration
     ssl = None
     ssloptions = False
-    no_upstream_cert = False
     masterclass = TestMaster
+
     add_upstream_certs_to_client_chain = False
 
     @classmethod
@@ -90,8 +90,8 @@ class ProxyTestBase(object):
             ssl=cls.ssl,
             ssloptions=cls.ssloptions)
 
-        cnf, opts = cls.get_proxy_config()
-        cls.config = ProxyConfig(opts, **cnf)
+        opts = cls.get_options()
+        cls.config = ProxyConfig(opts)
         tmaster = cls.masterclass(opts, cls.config)
         tmaster.start_app(APP_HOST, APP_PORT)
         cls.proxy = ProxyThread(tmaster)
@@ -117,13 +117,11 @@ class ProxyTestBase(object):
         return self.proxy.tmaster
 
     @classmethod
-    def get_proxy_config(cls):
+    def get_options(cls):
         cls.cadir = os.path.join(tempfile.gettempdir(), "mitmproxy")
-        cnf = dict()
-        return cnf, options.Options(
+        return options.Options(
             listen_port=0,
             cadir=cls.cadir,
-            no_upstream_cert = cls.no_upstream_cert,
             add_upstream_certs_to_client_chain=cls.add_upstream_certs_to_client_chain
         )
 
@@ -198,10 +196,10 @@ class TransparentProxyTest(ProxyTestBase):
         super(TransparentProxyTest, cls).teardown_class()
 
     @classmethod
-    def get_proxy_config(cls):
-        d, opts = ProxyTestBase.get_proxy_config()
+    def get_options(cls):
+        opts = ProxyTestBase.get_options()
         opts.mode = "transparent"
-        return d, opts
+        return opts
 
     def pathod(self, spec, sni=None):
         """
@@ -230,8 +228,8 @@ class ReverseProxyTest(ProxyTestBase):
     ssl = None
 
     @classmethod
-    def get_proxy_config(cls):
-        d, opts = ProxyTestBase.get_proxy_config()
+    def get_options(cls):
+        opts = ProxyTestBase.get_options()
         opts.upstream_server = "".join(
             [
                 "https" if cls.ssl else "http",
@@ -241,7 +239,7 @@ class ReverseProxyTest(ProxyTestBase):
             ]
         )
         opts.mode = "reverse"
-        return d, opts
+        return opts
 
     def pathoc(self, sni=None):
         """
@@ -269,10 +267,10 @@ class ReverseProxyTest(ProxyTestBase):
 class SocksModeTest(HTTPProxyTest):
 
     @classmethod
-    def get_proxy_config(cls):
-        d, opts = ProxyTestBase.get_proxy_config()
+    def get_options(cls):
+        opts = ProxyTestBase.get_options()
         opts.mode = "socks5"
-        return d, opts
+        return opts
 
 
 class ChainProxyTest(ProxyTestBase):
@@ -291,16 +289,16 @@ class ChainProxyTest(ProxyTestBase):
         cls.chain = []
         super(ChainProxyTest, cls).setup_class()
         for _ in range(cls.n):
-            cnf, opts = cls.get_proxy_config()
-            config = ProxyConfig(opts, **cnf)
+            opts = cls.get_options()
+            config = ProxyConfig(opts)
             tmaster = cls.masterclass(opts, config)
             proxy = ProxyThread(tmaster)
             proxy.start()
             cls.chain.insert(0, proxy)
 
         # Patch the orginal proxy to upstream mode
-        cnf, opts = cls.get_proxy_config()
-        cls.config = cls.proxy.tmaster.config = cls.proxy.tmaster.server.config = ProxyConfig(opts, **cnf)
+        opts = cls.get_options()
+        cls.config = cls.proxy.tmaster.config = cls.proxy.tmaster.server.config = ProxyConfig(opts)
 
     @classmethod
     def teardown_class(cls):
@@ -315,14 +313,14 @@ class ChainProxyTest(ProxyTestBase):
             proxy.tmaster.state.clear()
 
     @classmethod
-    def get_proxy_config(cls):
-        d, opts = super(ChainProxyTest, cls).get_proxy_config()
+    def get_options(cls):
+        opts = super(ChainProxyTest, cls).get_options()
         if cls.chain:  # First proxy is in normal mode.
             opts.update(
                 mode="upstream",
                 upstream_server="http://127.0.0.1:%s" % cls.chain[0].port
             )
-        return d, opts
+        return opts
 
 
 class HTTPUpstreamProxyTest(ChainProxyTest, HTTPProxyTest):