Stop stripping ProxyCommand none, make it None

Also apparently the old, old test for this had the wrong issue number in
it :(

3 files changed, 23 insertions, 16 deletions

diff --git a/paramiko/config.py b/paramiko/config.py
index 2358102b..48bcb101 100644
--- a/paramiko/config.py
+++ b/paramiko/config.py
@@ -159,9 +159,8 @@ class SSHConfig:
                     context["matches"] = self._get_matches(value)
             # Special-case for noop ProxyCommands
             elif key == "proxycommand" and value.lower() == "none":
-                # Store 'none' as None; prior to 3.x, it will get stripped out
-                # at the end (for compatibility with issue #415). After 3.x, it
-                # will simply not get stripped, leaving a nice explicit marker.
+                # Store 'none' as None - not as a string implying that the
+                # proxycommand is the literal shell command "none"!
                 context["config"][key] = None
             # All other keywords get stored, directly or via append
             else:
@@ -267,9 +266,6 @@ class SSHConfig:
         # Expand variables in resulting values (besides 'Match exec' which was
         # already handled above)
         options = self._expand_variables(options, hostname)
-        # TODO: remove in 3.x re #670
-        if "proxycommand" in options and options["proxycommand"] is None:
-            del options["proxycommand"]
         return options
 
     def canonicalize(self, hostname, options, domains):
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index 443f0b78..6df416a0 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -2,6 +2,17 @@
 Changelog
 =========
 
+- :support:`732` (also re: :issue:`630`) `~paramiko.config.SSHConfig` used to
+  straight-up delete the ``proxycommand`` key from config lookup results when
+  the source config said ``ProxyCommand none``. This has been altered to
+  preserve the key and give it the Python value ``None``, thus making the
+  Python representation more in line with the source config file.
+
+  .. warning::
+    This change is backwards incompatible if you were relying on the old (1.x,
+    2.x) behavior for some reason (eg assuming all ``proxycommand`` values were
+    valid).
+
 - :support:`-` The behavior of private key classes' (ie anything inheriting
   from `~paramiko.pkey.PKey`)  private key writing methods used to perform a
   manual, extra ``chmod`` call after writing. This hasn't been strictly
diff --git a/tests/test_config.py b/tests/test_config.py
index 2fd875e8..a2c60a32 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -460,7 +460,7 @@ Host param3 parara
         with raises(ConfigParseError):
             load_config("invalid")
 
-    def test_proxycommand_none_issue_418(self):
+    def test_proxycommand_none_issue_415(self):
         config = SSHConfig.from_text(
             """
 Host proxycommand-standard-none
@@ -472,10 +472,12 @@ Host proxycommand-with-equals-none
         )
         for host, values in {
             "proxycommand-standard-none": {
-                "hostname": "proxycommand-standard-none"
+                "hostname": "proxycommand-standard-none",
+                "proxycommand": None,
             },
             "proxycommand-with-equals-none": {
-                "hostname": "proxycommand-with-equals-none"
+                "hostname": "proxycommand-with-equals-none",
+                "proxycommand": None,
             },
         }.items():
 
@@ -495,13 +497,11 @@ Host *
     ProxyCommand default-proxy
 """
         )
-        # When bug is present, the full stripping-out of specific-host's
-        # ProxyCommand means it actually appears to pick up the default
-        # ProxyCommand value instead, due to cascading. It should (for
-        # backwards compatibility reasons in 1.x/2.x) appear completely blank,
-        # as if the host had no ProxyCommand whatsoever.
-        # Threw another unrelated host in there just for sanity reasons.
-        assert "proxycommand" not in config.lookup("specific-host")
+        # In versions <3.0, 'None' ProxyCommands got deleted, and this itself
+        # caused bugs. In 3.0, we more cleanly map "none" to None. This test
+        # has been altered accordingly but left around to ensure no
+        # regressions.
+        assert config.lookup("specific-host")["proxycommand"] is None
         assert config.lookup("other-host")["proxycommand"] == "other-proxy"
         cmd = config.lookup("some-random-host")["proxycommand"]
         assert cmd == "default-proxy"