options cleanups

- Move default client ciphers - no need to have a 10-line string default.
- Add some style guidance on options help
- Be consistent about reflowing options help

4 files changed, 32 insertions, 27 deletions

diff --git a/mitmproxy/options.py b/mitmproxy/options.py
index 70392803..1b66790f 100644
--- a/mitmproxy/options.py
+++ b/mitmproxy/options.py
@@ -26,19 +26,11 @@ APP_PORT = 80
 CA_DIR = "~/.mitmproxy"
 LISTEN_PORT = 8080
 
-# We manually need to specify this, otherwise OpenSSL may select a non-HTTP2 cipher by default.
-# https://mozilla.github.io/server-side-tls/ssl-config-generator/?server=apache-2.2.15&openssl=1.0.2&hsts=yes&profile=old
-DEFAULT_CLIENT_CIPHERS = (
-    "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:"
-    "ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:"
-    "ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:"
-    "ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:"
-    "DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:"
-    "DHE-RSA-AES256-SHA:ECDHE-RSA-DES-CBC3-SHA:ECDHE-ECDSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:"
-    "AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:AES:DES-CBC3-SHA:"
-    "HIGH:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:"
-    "!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA"
-)
+# Some help text style guidelines:
+#
+# - Should be a single paragraph with no linebreaks. Help will be reflowed by
+# tools.
+# - Avoid adding information about the data type - we can generate that.
 
 
 class Options(optmanager.OptManager):
@@ -80,8 +72,9 @@ class Options(optmanager.OptManager):
         self.add_option(
             "keepserving", bool, False,
             """
-                Instructs mitmdump to continue serving after client playback,
-                server playback or file read. This option is ignored by interactive tools, which always keep serving.
+            Instructs mitmdump to continue serving after client playback, server
+            playback or file read. This option is ignored by interactive tools,
+            which always keep serving.
             """
         )
         self.add_option(
@@ -174,7 +167,7 @@ class Options(optmanager.OptManager):
             "server_replay_ignore_params", Sequence[str], [],
             """
             Request's parameters to be ignored while searching for a saved flow
-            to replay. Can be passed multiple times.
+            to replay.
             """
         )
         self.add_option(
@@ -230,12 +223,11 @@ class Options(optmanager.OptManager):
             The file at path is a certificate in PEM format. If a private key is
             included in the PEM, it is used, else the default key in the conf
             dir is used. The PEM file should contain the full certificate chain,
-            with the leaf certificate as the first entry. Can be passed multiple
-            times.
+            with the leaf certificate as the first entry.
             """
         )
         self.add_option(
-            "ciphers_client", str, DEFAULT_CLIENT_CIPHERS,
+            "ciphers_client", Optional[str], None,
             "Set supported ciphers for client connections using OpenSSL syntax."
         )
         self.add_option(
diff --git a/mitmproxy/optmanager.py b/mitmproxy/optmanager.py
index 86f833e4..5064ff44 100644
--- a/mitmproxy/optmanager.py
+++ b/mitmproxy/optmanager.py
@@ -36,7 +36,7 @@ class _Option:
         self.typespec = typespec
         self._default = default
         self.value = unset
-        self.help = textwrap.dedent(help or "").strip()
+        self.help = textwrap.dedent(help).strip().replace("\n", " ")
         self.choices = choices
 
     def __repr__(self):
@@ -396,9 +396,7 @@ def dump_defaults(opts):
                 raise NotImplementedError
             txt += " Type %s." % t
 
-        txt = "\n".join(
-            textwrap.wrap(txt)
-        )
+        txt = "\n".join(textwrap.wrap(txt))
         s.yaml_set_comment_before_after_key(k, before = "\n" + txt)
     return ruamel.yaml.round_trip_dump(s)
 
diff --git a/mitmproxy/proxy/protocol/tls.py b/mitmproxy/proxy/protocol/tls.py
index acc0c6e3..f55855f0 100644
--- a/mitmproxy/proxy/protocol/tls.py
+++ b/mitmproxy/proxy/protocol/tls.py
@@ -200,6 +200,21 @@ CIPHER_ID_NAME_MAP = {
 }
 
 
+# We manually need to specify this, otherwise OpenSSL may select a non-HTTP2 cipher by default.
+# https://mozilla.github.io/server-side-tls/ssl-config-generator/?server=apache-2.2.15&openssl=1.0.2&hsts=yes&profile=old
+DEFAULT_CLIENT_CIPHERS = (
+    "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:"
+    "ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:"
+    "ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:"
+    "ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:"
+    "DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:"
+    "DHE-RSA-AES256-SHA:ECDHE-RSA-DES-CBC3-SHA:ECDHE-ECDSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:"
+    "AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:AES:DES-CBC3-SHA:"
+    "HIGH:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:"
+    "!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA"
+)
+
+
 def is_tls_record_magic(d):
     """
     Returns:
@@ -475,7 +490,7 @@ class TlsLayer(base.Layer):
                 cert, key,
                 method=self.config.openssl_method_client,
                 options=self.config.openssl_options_client,
-                cipher_list=self.config.options.ciphers_client,
+                cipher_list=self.config.options.ciphers_client or DEFAULT_CLIENT_CIPHERS,
                 dhparams=self.config.certstore.dhparams,
                 chain_file=chain_file,
                 alpn_select_callback=self.__alpn_select_callback,
diff --git a/test/mitmproxy/test_optmanager.py b/test/mitmproxy/test_optmanager.py
index df392829..84e574f5 100644
--- a/test/mitmproxy/test_optmanager.py
+++ b/test/mitmproxy/test_optmanager.py
@@ -270,14 +270,14 @@ def test_merge():
 
 
 def test_option():
-    o = optmanager._Option("test", int, 1, None, None)
+    o = optmanager._Option("test", int, 1, "help", None)
     assert o.current() == 1
     with pytest.raises(TypeError):
         o.set("foo")
     with pytest.raises(TypeError):
-        optmanager._Option("test", str, 1, None, None)
+        optmanager._Option("test", str, 1, "help", None)
 
-    o2 = optmanager._Option("test", int, 1, None, None)
+    o2 = optmanager._Option("test", int, 1, "help", None)
     assert o2 == o
     o2.set(5)
     assert o2 != o