Rev 2900: (Andrew Bennetts) Fix bug 146715: bzr+ssh:// and sftp:// should not assume port-not-specified means port 22. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Oct 9 09:43:49 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2900
revision-id: pqm at pqm.ubuntu.com-20071009084347-m6boff8rxmzsz28f
parent: pqm at pqm.ubuntu.com-20071009080820-civ61gexahohpats
parent: andrew.bennetts at canonical.com-20071008022217-8gxkimbur9vzgbav
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2007-10-09 09:43:47 +0100
message:
  (Andrew Bennetts) Fix bug 146715: bzr+ssh:// and sftp:// should not assume port-not-specified means port 22.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
    ------------------------------------------------------------
    revno: 2892.1.1
    merged: andrew.bennetts at canonical.com-20071008022217-8gxkimbur9vzgbav
    parent: pqm at pqm.ubuntu.com-20071006144547-0e1mpht72yd6wyfz
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: ssh-port-bug-146715
    timestamp: Mon 2007-10-08 12:22:17 +1000
    message:
      Fix bug 146715: bzr+ssh:// and sftp:// should not assume port-not-specified means port 22
=== modified file 'NEWS'
--- a/NEWS	2007-10-08 23:27:01 +0000
+++ b/NEWS	2007-10-09 08:43:47 +0000
@@ -77,6 +77,11 @@
 
   BUG FIXES:
 
+   * ``bzr+ssh://`` and ``sftp://`` URLs that do not specify ports explicitly
+     no longer assume that means port 22.  This allows people using OpenSSH to
+     override the default port in their ``~/.ssh/config`` if they wish.  This
+     fixes a bug introduced in bzr 0.91.  (Andrew Bennetts, #146715)
+
    * Commands reporting exceptions can now be profiled and still have their
      data correctly dumped to a file. For example, a ``bzr commit`` with
      no changes still reports the operation as pointless but doing so no

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2007-10-04 07:12:14 +0000
+++ b/bzrlib/tests/test_transport.py	2007-10-08 02:22:17 +0000
@@ -15,9 +15,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 
-import os
-import sys
-import stat
 from cStringIO import StringIO
 
 import bzrlib
@@ -26,15 +23,11 @@
     osutils,
     urlutils,
     )
-from bzrlib.errors import (ConnectionError,
-                           DependencyNotPresent,
+from bzrlib.errors import (DependencyNotPresent,
                            FileExists,
                            InvalidURLJoin,
                            NoSuchFile,
                            PathNotChild,
-                           TransportNotPossible,
-                           ConnectionError,
-                           DependencyNotPresent,
                            ReadError,
                            UnsupportedProtocol,
                            )
@@ -49,7 +42,6 @@
                               LateReadError,
                               register_lazy_transport,
                               register_transport_proto,
-                              _clear_protocol_handlers,
                               Transport,
                               )
 from bzrlib.transport.chroot import ChrootServer
@@ -625,14 +617,14 @@
     """Tests for connected to remote server transports"""
 
     def test_parse_url(self):
-        t = ConnectedTransport('sftp://simple.example.com/home/source')
+        t = ConnectedTransport('http://simple.example.com/home/source')
         self.assertEquals(t._host, 'simple.example.com')
-        self.assertEquals(t._port, 22)
+        self.assertEquals(t._port, 80)
         self.assertEquals(t._path, '/home/source/')
         self.failUnless(t._user is None)
         self.failUnless(t._password is None)
 
-        self.assertEquals(t.base, 'sftp://simple.example.com/home/source/')
+        self.assertEquals(t.base, 'http://simple.example.com/home/source/')
 
     def test_parse_quoted_url(self):
         t = ConnectedTransport('http://ro%62ey:h%40t@ex%41mple.com:2222/path')
@@ -765,6 +757,80 @@
         self.assertEquals('bzr://host.com:666/', t.base)
 
 
+class SSHPortTestMixin(object):
+    """Mixin class for testing SSH-based transports' use of ports in URLs.
+    
+    Unlike other connected transports, SSH-based transports (sftp, bzr+ssh)
+    don't have a default port, because the user may have OpenSSH configured to
+    use a non-standard port.
+    """
+
+    def make_url(self, netloc):
+        """Make a url for the given netloc, using the scheme defined on the
+        TestCase.
+        """
+        return '%s://%s/' % (self.scheme, netloc)
+
+    def test_relpath_with_implicit_port(self):
+        """SSH-based transports with the same URL are the same.
+        
+        Note than an unspecified port number is different to port 22 (because
+        OpenSSH may be configured to use a non-standard port).
+
+        So t.relpath(url) should always be '' if t.base is the same as url, but
+        raise PathNotChild if the ports in t and url are not both specified (or
+        both unspecified).
+        """
+        url_implicit_port = self.make_url('host.com')
+        url_explicit_port = self.make_url('host.com:22')
+
+        t_implicit_port = get_transport(url_implicit_port)
+        self.assertEquals('', t_implicit_port.relpath(url_implicit_port))
+        self.assertRaises(
+            PathNotChild, t_implicit_port.relpath, url_explicit_port)
+        
+        t_explicit_port = get_transport(url_explicit_port)
+        self.assertRaises(
+            PathNotChild, t_explicit_port.relpath, url_implicit_port)
+        self.assertEquals('', t_explicit_port.relpath(url_explicit_port))
+
+    def test_construct_with_no_port(self):
+        """If no port is specified, then the SSH-based transport's _port will
+        be None.
+        """
+        t = get_transport(self.make_url('host.com'))
+        self.assertEquals(None, t._port)
+
+    def test_url_with_no_port(self):
+        """If no port was specified, none is shown in the base URL."""
+        t = get_transport(self.make_url('host.com'))
+        self.assertEquals(self.make_url('host.com'), t.base)
+
+    def test_url_includes_port(self):
+        """An SSH-based transport's base will show the port if one was
+        specified, even if that port is 22, because we do not assume 22 is the
+        default port.
+        """
+        # 22 is the "standard" port for SFTP.
+        t = get_transport(self.make_url('host.com:22'))
+        self.assertEquals(self.make_url('host.com:22'), t.base)
+        # 666 is not a standard port.
+        t = get_transport(self.make_url('host.com:666'))
+        self.assertEquals(self.make_url('host.com:666'), t.base)
+
+
+class SFTPTransportPortTest(TestCase, SSHPortTestMixin):
+    """Tests for sftp:// transport (SFTPTransport)."""
+
+    scheme = 'sftp'
+
+
+class BzrSSHTransportPortTest(TestCase, SSHPortTestMixin):
+    """Tests for bzr+ssh:// transport (RemoteSSHTransport)."""
+
+    scheme = 'bzr+ssh'
+
+
 class TestTransportTrace(TestCase):
 
     def test_get(self):

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-10-09 01:59:50 +0000
+++ b/bzrlib/transport/__init__.py	2007-10-09 08:43:47 +0000
@@ -1707,9 +1707,10 @@
 register_lazy_transport('file://', 'bzrlib.transport.local', 'LocalTransport')
 transport_list_registry.set_default_transport("file://")
 
+# Note that sftp:// has no default_port, because the user's ~/.ssh/config
+# can set it to arbitrary values based on hostname.
 register_transport_proto('sftp://',
-            help="Access using SFTP (most SSH servers provide SFTP).",
-            default_port=22)
+            help="Access using SFTP (most SSH servers provide SFTP).")
 register_lazy_transport('sftp://', 'bzrlib.transport.sftp', 'SFTPTransport')
 # Decorated http transport
 register_transport_proto('http+urllib://',
@@ -1802,9 +1803,10 @@
 register_lazy_transport('bzr+https://',
                         'bzrlib.transport.remote',
                         'RemoteHTTPTransport')
+# Note that bzr+ssh:// has no default_port, because the user's ~/.ssh/config
+# can set it to arbitrary values based on hostname.
 register_transport_proto('bzr+ssh://',
-            help="Fast access using the Bazaar smart server over SSH.",
-            default_port=22)
+            help="Fast access using the Bazaar smart server over SSH.")
 register_lazy_transport('bzr+ssh://',
                         'bzrlib.transport.remote',
                         'RemoteSSHTransport')




More information about the bazaar-commits mailing list