[merge] SFTP tests run and pass on win32

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robey Pointer wrote:
> 
> On 29 Jun 2006, at 14:37, John Arbash Meinel wrote:

...

>>
>>      def __init__(self, server, root, home=None):
>>          SFTPServerInterface.__init__(self, server)
>> +        # All paths are actually relative to 'root'.
>> +        # this is like implementing chroot().
>>          self.root = root
>>          if home is None:
>> +            # XXX: if 'home' is None, shouldn't it
>> +            #       be set to '', since it should
>> +            #       be relative to 'root'?
>>              self.home = self.root
> 
> You're right here.  This should be: self.home = ''

I'm glad I understood correctly. It hasn't mattered for the test suite,
because root was always '/'

...

>>
>>      def _realpath(self, path):
>> -        return self.root + self.canonicalize(path)
>> +        if sys.platform == 'win32':
>> +            # Win32 sftp paths end up looking like
>> +            # sftp://host@foo/h:/foo/bar
>> +            # which gets translated here to:
>> +            # /h:/foo/bar
>> +            # Local paths stay 'foo/bar', though.
>> +            # Also, win32 needs to use the Unicode APIs.
>> +            thispath = path.decode('utf8')
>> +            if path.startswith('/'):
>> +                # Abspath
>> +                realpath = os.path.normpath(thispath[1:])
>> +            else:
>> +                realpath = os.path.normpath(os.path.join(self.home,
>> thispath))
>> +        else:
>> +            realpath = self.root + self.canonicalize(path)
>> +        return realpath
> 
> I think theoretically most of this logic should be in canonicalize,
> since the sftp server calls 'canonicalize' to do all (sftp path -> local
> path) translation.  It probably doesn't matter here since we're using
> this only for tests and there are therefore no security issues.  It
> would matter for things like chdir and symlinks.

I went ahead and moved the logic. Just looking through the code, all the
paths were calling _realpath(), so I worked there.

> 
> Btw, I'm embarrassed to say that I just did a little research and found
> out that "canonicalize" isn't a word.  I think I wanted "canonize".  And
> now it's enshrined in an API.  Doh.
> 

You could certainly upgrade your api, and just include a default thunk
which explains 'this code path is deprecated'.

Just something like:
def canonize(self, path):
    # not to be confused with cannonize :)
    warn('You should be overriding canonize, not canonicalize')
    return self.canonicalize(path)

Or if you are providing a default implementation as well as allowing
them to override:

def canonize(self, path):
    old_func = getattr(self, 'canonicalize', None)
    if old_func:
	warn('You should be overriding canonize, not canonicalize')
        return old_func(path)
    # Default implemenation...

Python introspection makes it *very* easy to provide compatible apis.

> I only looked at the sftp-related sections, but aside from the comments
> above, they looked fine.
> 
> robey
> 
> 

Thanks, there was one other small thing that I fixed:
def _realpath(self, path):
    # paths returned from self.canonicalize() always start with
    # a path separator. So if 'root' is just '/', this would cause
    # a double slash at the beginning '//home/dir'.
    if self.root == '/':
        return self.canonicalize(path)
    return self.root + self.canonicalize(path)

You always make sure to put a beginning '/' during canonicalize, so the
way you were doing it was creating paths like '//foo/bar'. Which is
technically legal, but really ugly (and may be confused with network paths).

Also, if you want this to really work, we need to test things out with
root != '' on win32.

There are potentially weird things because of how you have to supply a
path. (if you root is '', then you need to supply '/h:/foo' and strip
off the first slash, but if root is 'h:', then you supply '/foo' you
*don't* want to strip the first slash).

I got it working for bzr, but since I don't *really* know what an sftp
server on win32 should be doing I didn't push for absolute correctness.
(can it export all drives, or should it always be rooted somewhere
inside the path?) Its just that win32 has this special sequence based on
the drive, whereas posix unifies the whole thing.

Anyway, I cleaned it up per your comments, and submitted it. Thanks for
your help.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEpTyiJdeBCYSNAAMRAhMtAJ9FX+1+CCPklIqUkcMNUlv6emDmpACeK192
dtsN0A9EzDf3dn6xsahQb6Y=
=1Iz/
-----END PGP SIGNATURE-----