Paramiko throws EOFError rather than returning a 0 length file or ENOENT

[John Arbash Meinel]

Robey Pointer wrote:
> 
> On 12 Sep 2006, at 8:06, John Arbash Meinel wrote:
> 

...

>> I think it is just a question of what should a file raise if it gets
>> truncated. A standard file just returns a short read, but no exception.
>> Would bzr handle that properly? Or do we really prefer getting a
>> Disconnected error.
>>
>> I'm tempted to say Disconnected, because I don't know what a short-read
>> would really do. I *think* it would be safe, as long as the next action
>> was Disconnected. The readv stuff doesn't actually assert that the
>> lengths it reads are correct. It should, though.
> 
> I agree; I don't think that kind of error should be hidden.  It could
> lead to silent corruption.
> 

I looked into it closer (see my [merge] requests for patches)

But it turns out that paramiko uses an 'assert' to do a safety check,
and I can easily add something similar for the other code.

> 
>> On that note, should it keep trying to read more until it gets the full
>> length, and fail only if it gets a 0-length read, or should it just fail
>> on the first wrong-length read?
> 
> As Vincent points out, a short read isn't necessarily an error,
> especially over sockets.  Paramiko tries to hide this but it's probably
> still an issue for http, for example.
> 
> I believe an error like "server disconnected" should always be reported
> as some kind of exception (at the API level).
> 
> robey

Well, at the moment we wrap the result of HTTP in another file-like
object (cStringIO), because urllib files don't have tell or seek. And
for ranged requests, we just read in the entire contents, and parse it
into a RangeFile object.

In time, I'd like to get rid of RangeFile, and work similarly to
sftp/local, which parses the return requests on the fly, rather than
buffering the whole contents.

I'd also be happier if we didn't have to buffer the whole contents of a
plain get() in memory, but the best alternative I can think of is to use
a disk cache, which we don't really want to do either.

Also, HTTP.readv() also has asserts in there to check the size of the
returned content. My other patch tries to upgrade these things to a real
Exception that can be caught.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060912/12d2ce00/attachment.pgp