<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<title>Re: [Python-Dev] Asking for feedback about fixing `ftplib'
(issue25458)</title>
On 25.12.2016 17:21, Giampaolo Rodola' wrote:<br>
<blockquote
cite="mid:CAFYqXL_pyyOiB6nc0V_mk-QzwbDXeVUgCnB1FKAOFFVjSFv7wA@mail.gmail.com"
type="cite">
<div dir="ltr">From what I understand the problem should be fixed
in urllib so that it always closes the FTP connection. I
understand this is what happens in recent 3.x versions but not
on 2.7.</div>
</blockquote>
urllib in 2.7 should indeed be fixed to close FTP connections rather
than leave them dangling, no question about that.<br>
(ftpcache probably has to go in all branches as a result since it's
now useless - but that's another issue entirely)<br>
<br>
The question is, in both 2.x and 3.x, urllib currently doesn't close
the control connection gracefully. It just closes the socket
immediately after reading all data on the data connection - without
issuing QUIT or even reading the end-of-transfer response.<br>
By doing this, it gets away with not calling voidresp().<br>
(RFC 939 specifies that the server should execute the equivalent of
ABOR and QUIT upon unexpected close of control connection.)<br>
<blockquote
cite="mid:CAFYqXL_pyyOiB6nc0V_mk-QzwbDXeVUgCnB1FKAOFFVjSFv7wA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>As for ftplib, I don't like the solution of using a `<span style="WHITE-SPACE: pre-wrap; COLOR: rgb(0,0,0); FONT-SIZE: 11px">transfer_in_progress` flag in all commands</span><span style="WHITE-SPACE: pre-wrap; COLOR: rgb(0,0,0); FONT-SIZE: 11px">. I see that as a cheap way to work around the fact that the user may forget to call
</span><span style="WHITE-SPACE: pre-wrap; COLOR: rgb(0,0,0); FONT-SIZE: 11px">voidresp() and close the socket after </span><span style="WHITE-SPACE: pre-wrap; COLOR: rgb(0,0,0); FONT-SIZE: 11px">nt/transfercmd(). </span><span style="WHITE-SPACE: pre-wrap; COLOR: rgb(0,0,0); FONT-SIZE: 11px">It probably makes sense to just update nt/transfercmd() doc instead and make that clear.</span></div>
</div>
</blockquote>
I tried to fix urllib so that it's guaranteed to call voidresp(),
and failed. Too complicated, effectively reimplementing a sizable
part of FTP client logic is required, perhaps even monkey-patching
ftplib to be able to set flags in the right places. That's why I
thought that just requiring the user to call it themselves and
calling it a day would be asking too much from users and the easy
way out - ftplib currently dumps too much work on its users that it
ought to do itself instead.<br>
<br>
The fact that in FTP, one cannot send another command before the
previous one is complete (or rather, one can, but the server won't
respond to it until then) means that FTP is inherently stateful. So,
ftplib, to be a usable library, needs to either encapsulate this
statefulness, or provide building blocks with all the stock logic
parts so that only truly application-specific logic has to be added
to make a robust solution.<br>
<br>
With simple commands (<code class="descname">voidcmd</code>) and
self-contained transfer commands (retrlines/retrbinary), ftplib does
incapsulate statefulness, by handling them atomically. But with
transfercmd(), it returns control to the user halfway through.<br>
At this point, if ftplib doesn't itself keep track that the control
connection is currently in invalid state for the next command, the
user is forced to do that themselves instead. And guess what -
urllib has to use ftplib through a wrapper class that does exactly
that. That's definitely a "stock logic part" 'cuz it's an integral
part of FTP rather than something specific to user logic.<br>
<br>
The other "stock logic part" currently missing is error handling
during transfer. If the error's cause is local (like a local
exception that results in closing the data socket prematurely, or
the user closing it by hand, or an ABOR they sent midway), the error
code from the end-of-transfer response should be ignored, otherwise,
it shouldn't.<br>
Nothing currently provides this logic. ftplib.retr*() never ignore
the code - because they never abort the transfer - but they don't
handle exceptions, either, so they wouldn't read the response if one
is raised on data socket read or in the callback. urllib used to
handle the response in an overridden socket close handler, and it
was forced to always ignore the code because it's impossible to
check from there if there was an exception or if the data socket was
closed prematurely.<br>
--<br>
Other than making ftplib keep track of the session's internal state
while the user has control and providing the custom error handling
logic to them, another solution is to never release control to the
user midway, i.e. ban transfercmd() entirely and only provide
self-contained retr*()/dir()/whatever, but let the callback signal
them to abort the transfer. That way, the state would be kept
implicitly - in the execution pointer.<br>
<br>
<br>
<blockquote
cite="mid:CAFYqXL_pyyOiB6nc0V_mk-QzwbDXeVUgCnB1FKAOFFVjSFv7wA@mail.gmail.com"
type="cite">-- <br>
<div class="gmail_extra">
<div class="gmail_signature" data-smartmail="gmail_signature">
<div dir="ltr">
<div>Giampaolo - <a href="http://grodola.blogspot.com"
target="_blank" moz-do-not-send="true">http://grodola.blogspot.com</a></div>
<div><br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Regards,
Ivan</pre>
</body>
</html>