new SFTP implementation unicode issues with OpenSSH sftp client #1038

New Issue

tahoe-lafs · 2010-05-13T11:15:09Z

francois commented

2010-05-13 11:15:09 +00:00

sftp> version
SFTP protocol version 3
sftp> ls tmp
tmp/Archives                            tmp/Latest                              
tmp/puppetmaster-git-submodules.pdf     tmp/small-file.txt                      
sftp> cd tmp
Couldn't canonicalise: Failure
sftp>

Here's the exception which gets logged in twistd.log.

2010-05-13 12:58:11+0200 [SSHChannel session (0) on SSHService ssh-connection on SSHServerTransport,1,127.0.0.1] Unhandled Error
	Traceback (most recent call last):
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 53, in dataReceived
	    f(data)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 398, in packet_REALPATH
	    d.addCallback(self._cbReadLink, requestId) # same return format
	  File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 224, in addCallback
	    callbackKeywords=kw)
	  File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 213, in addCallbacks
	    self._runCallbacks()
	--- <exception caught here> ---
	  File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 371, in _runCallbacks
	    self.result = callback(self.result, *args, **kw)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 381, in _cbReadLink
	    self._cbSendDirectory([(result, '', {})], requestId)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 314, in _cbSendDirectory
	    struct.pack('!L', len(result))+data)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 30, in sendPacket
	    self.transport.write(struct.pack('!LB', len(data)+1, kind) + data)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/session.py", line 260, in write
	    self.session.write(data)
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/channel.py", line 201, in write
	    write(self, data[offset: offset+rmp])
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/connection.py", line 451, in sendData
	    common.NS(data))
	  File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/transport.py", line 222, in sendPacket
	    payload + randbytes.secureRandom(lenPad))
	exceptions.UnicodeDecodeError: 'ascii' codec can't decode byte 0x95 in position 0: ordinal not in range(128)

I don't see any Tahoe code in the stacktrace, so it might be a Twisted
SFTP implmentation bug ?

``` sftp> version SFTP protocol version 3 sftp> ls tmp tmp/Archives tmp/Latest tmp/puppetmaster-git-submodules.pdf tmp/small-file.txt sftp> cd tmp Couldn't canonicalise: Failure sftp> ``` Here's the exception which gets logged in twistd.log. ``` 2010-05-13 12:58:11+0200 [SSHChannel session (0) on SSHService ssh-connection on SSHServerTransport,1,127.0.0.1] Unhandled Error Traceback (most recent call last): File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 53, in dataReceived f(data) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 398, in packet_REALPATH d.addCallback(self._cbReadLink, requestId) # same return format File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 224, in addCallback callbackKeywords=kw) File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 213, in addCallbacks self._runCallbacks() --- <exception caught here> --- File "/usr/lib/python2.6/dist-packages/twisted/internet/defer.py", line 371, in _runCallbacks self.result = callback(self.result, *args, **kw) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 381, in _cbReadLink self._cbSendDirectory([(result, '', {})], requestId) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 314, in _cbSendDirectory struct.pack('!L', len(result))+data) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py", line 30, in sendPacket self.transport.write(struct.pack('!LB', len(data)+1, kind) + data) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/session.py", line 260, in write self.session.write(data) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/channel.py", line 201, in write write(self, data[offset: offset+rmp]) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/connection.py", line 451, in sendData common.NS(data)) File "/usr/lib/python2.6/dist-packages/twisted/conch/ssh/transport.py", line 222, in sendPacket payload + randbytes.secureRandom(lenPad)) exceptions.UnicodeDecodeError: 'ascii' codec can't decode byte 0x95 in position 0: ordinal not in range(128) ``` I don't see any Tahoe code in the stacktrace, so it might be a Twisted SFTP implmentation bug ?

tahoe-lafs added the

labels 2010-05-13 11:15:09 +00:00

tahoe-lafs added this to the undecided milestone 2010-05-13 11:15:09 +00:00

francois commented

2010-05-13 11:16:32 +00:00

This was tested with the new SFTP implementation from #1037. But it was perhaps already failing with the old implementation.

davidsarah commented

2010-05-13 23:33:59 +00:00

From the stacktrace, it looks like payload is (correctly) expected to be a byte string but is actually a Unicode string. The bug could either be in Twisted code, or I could be passing a Unicode string into conch's API that is being appended to the payload causing it to be coerced to Unicode.

(Calling all language designers: implicit coercions are evil.)

From the stacktrace, it looks like `payload` is (correctly) expected to be a byte string but is actually a Unicode string. The bug could either be in Twisted code, or I could be passing a Unicode string into conch's API that is being appended to the payload causing it to be coerced to Unicode. (Calling all language designers: implicit coercions are evil.)

tahoe-lafs added

and removed

labels 2010-05-13 23:33:59 +00:00

tahoe-lafs modified the milestone from undecided to 1.7.0

2010-05-13 23:33:59 +00:00

davidsarah commented

2010-05-14 05:48:51 +00:00

This should be fixed on the ticket1037 branch. The problem was that realPath was returning a Unicode string when it should have been returning a UTF-8 bytestring. There was actually a test that it returned a bytestring, but that test wasn't working correctly, because failUnlessEqual can treat Unicode and bytestrings as being equal, e.g. failUnlessEqual(u"a", "a") succeeds.

(So implicit coercions were doubly evil in this case -- once in the Twisted code and once in failUnlessEqual.)

This should be fixed on the ticket1037 branch. The problem was that `realPath` was returning a Unicode string when it should have been returning a UTF-8 bytestring. There was actually a test that it returned a bytestring, but that test wasn't working correctly, because `failUnlessEqual` can treat Unicode and bytestrings as being equal, e.g. `failUnlessEqual(u"a", "a")` succeeds. (So implicit coercions were doubly evil in this case -- once in the Twisted code and once in `failUnlessEqual`.)

tahoe-lafs added the

fixed

label 2010-05-14 05:48:51 +00:00

tahoe-lafs closed this issue

2010-05-14 05:48:51 +00:00

Sign in to join this conversation.