New submission from Dan Hemberger :
When using the CacheFTPHandler in the most basic of contexts, a URLError will
be thrown if you try to reuse any of the FTP instances stored in the handler.
This makes CacheFTPHandler unusable for its intended purpose. Note that the
default FTPHandler circumvents this issue by creating a new ftplib.FTP instance
for each connection (and thus never reuses any of them).
Here is a simple example illustrating the problem:
"""
import urllib.request as req
import ftplib
opener = req.build_opener(req.CacheFTPHandler)
req.install_opener(opener)
ftplib.FTP.debugging = 2
for _ in range(2):
req.urlopen("ftp://www.pythontest.net/README";, timeout=10)
"""
>From the ftplib debugging output, we see the following communication between
>the client and server:
"""
*cmd* 'TYPE I'
*put* 'TYPE I\r\n'
*get* '200 Switching to Binary mode.\n'
*resp* '200 Switching to Binary mode.'
*cmd* 'PASV'
*put* 'PASV\r\n'
*get* '227 Entering Passive Mode (159,89,235,38,39,111).\n'
*resp* '227 Entering Passive Mode (159,89,235,38,39,111).'
*cmd* 'RETR README'
*put* 'RETR README\r\n'
*get* '150 Opening BINARY mode data connection for README (123 bytes).\n'
*resp* '150 Opening BINARY mode data connection for README (123 bytes).'
*cmd* 'TYPE I'
*put* 'TYPE I\r\n'
*get* '226 Transfer complete.\n'
*resp* '226 Transfer complete.'
*cmd* 'PASV'
*put* 'PASV\r\n'
*get* '200 Switching to Binary mode.\n'
*resp* '200 Switching to Binary mode.'
ftplib.error_reply: 200 Switching to Binary mode.
"""
The client and the server have gotten out of sync due to the missing voidresp()
call, i.e. the client sends 'Type I' but receives the response from the
previous 'RETR README' command. When ftp.voidresp() is added anywhere between
after the ftp.ntransfercmd() and before the next command is sent (i.e. by
reverting 2d51f687e133fb8141f1a6b5a6ac51c9d5eddf58), we see the correct
send/receive pattern:
"""
*cmd* 'TYPE I'
*put* 'TYPE I\r\n'
*get* '200 Switching to Binary mode.\n'
*resp* '200 Switching to Binary mode.'
*cmd* 'PASV'
*put* 'PASV\r\n'
*get* '227 Entering Passive Mode (159,89,235,38,39,107).\n'
*resp* '227 Entering Passive Mode (159,89,235,38,39,107).'
*cmd* 'RETR README'
*put* 'RETR README\r\n'
*get* '150 Opening BINARY mode data connection for README (123 bytes).\n'
*resp* '150 Opening BINARY mode data connection for README (123 bytes).'
*get* '226 Transfer complete.\n'
*resp* '226 Transfer complete.'
*cmd* 'TYPE I'
*put* 'TYPE I\r\n'
*get* '200 Switching to Binary mode.\n'
*resp* '200 Switching to Binary mode.'
*cmd* 'PASV'
*put* 'PASV\r\n'
*get* '227 Entering Passive Mode (159,89,235,38,39,107).\n'
*resp* '227 Entering Passive Mode (159,89,235,38,39,107).'
*cmd* 'RETR README'
*put* 'RETR README\r\n'
*get* '150 Opening BINARY mode data connection for README (123 bytes).\n'
*resp* '150 Opening BINARY mode data connection for README (123 bytes).'
*get* '226 Transfer complete.\n'
*resp* '226 Transfer complete.'
"""
By inspecting the methods of ftplib.FTP, we can see that every use of
ntransfercmd() is followed by a voidresp(), see e.g. retrbinary, retrlines,
storbinary, storlines, and voidcmd.
I hope that some experts in urllib and ftplib can weigh in on any of the
subtleties of this issue, but I think it's clear that the missing
ftp.voidresp() call is a significant bug.
--
Some historical notes about this issue
--
This issue has been documented in a number of other bug reports, but I don't
think any have addressed the complete breakage of the CachedFTPHandler that it
causes.
The breaking change was originally introduced as a resolution to issue16270.
However, it's not clear from the comments why it was believed that removing
ftp.voidresp() from endtransfer() was the correct solution. In either case,
with this commit reverted, both the test outlined in this report and in
issue16270 work correctly.
@orsenthil has suggested this fix (to revert the change to endtransfer) in
msg286020, and has explained his reasoning in detail in msg286016.
@Ivan.Pozdeev has also explained this issue in msg282797 and provided a similar
patch in issue28931, though it does more than just revert the breaking commit
and I'm not sure what the additional changes are intending to fix.
--
components: Library (Lib)
messages: 345155
nosy: danh
priority: normal
severity: normal
status: open
title: urllib missing voidresp breaks CacheFTPHandler
type: behavior
versions: Python 3.9
___
Python tracker
<https://bugs.python.org/issue37222>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
