Message 94294 - Python tracker (original) (raw)

The patch looks good to me. Only two little remarks:

1: I'd create two new separate tests rather than appending them to existent ones.

2: > self.client.retrbinary('retr', received.append, rest=str(rest))

str() should be useless here.

According to the RFC, the argument to REST can be any string of printable characters. However, does it happen for clients to put non-digits in there?

It shouldn't happen but in any case I woulnd't want ftplib to check for such a kind of thing. Deciding whether the REST argument is invalid is up to the server, in which case it will send a 4xx/5xx error response.

But I've tested some ftp servers and they don't return an error code, they just set REST to 0 and return some OK code.

IMHO a bad design choice.