Don't catch AssertionError

[Robey Pointer]

[cross-posting to paramiko]

On 27 Dec 2006, at 0:52, John Arbash Meinel wrote:

> Robey Pointer wrote:
>>
>> On 21 Dec 2006, at 15:08, John Arbash Meinel wrote:
>>
>>> I just wanted to remind everyone that if you find yourself writing a
>>> test that catches AssertionError, it is usually a sign that  
>>> something is
>>> wrong with the code.
>>>
>>> I know of one place where we *have* to. But it is because  
>>> paramiko has
>>> an assert, rather than something we are doing.
>>
>> I just grepped bzrlib for this and I think the assert was in
>> transport/sftp.py, not paramiko, so we can easily avoid doing it.
>> Here's a quick bundle that replaces the (commented-out) assert with a
>> PathNotChild exception, and uncomments the corresponding unit tests.
>> The tests all still pass.
>>
>> robey
>
> Well, you are correct that this can be changed as you say it is.
>
> But my comment was regarding around line 455 in sftp_file.py:
>
> [...]
>
> The 'assert length == len(data)' is making sure that the returned data
> is the exact same length as what was requested. Which could be  
> violated
> if you allowed requests >32KB, or if there was a readv for 100 bytes,
> and there is only 80 bytes left in the file.
>
> paramiko shouldn't be using 'assert' to check an invariant in  
> something
> that it is reading from the real world. It should be doing a real  
> check,
> and raising a real error if it fails.

Hmmm... Requests over 32K should be caught elsewhere, but it's true  
that EOF could cause a short response.  Originally the assert was  
meant to verify that the readv tables were internally consistent, but  
I guess there are other checks on that now (plus unit tests, of course).

I'll remove that assert.

robey