This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: ftplib Persistent data connection
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.11
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: ethan.furman, giampaolo.rodola, gregory.p.smith, jbell, mikecmcleod
Priority: normal Keywords: patch

Created on 2008-04-13 23:50 by jbell, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
debug.log jbell, 2008-04-13 23:54 Expected debug output
ftplib.py.blockmode.patch jbell, 2008-05-12 21:24 Patch (revised) to add block mode review
ftplib.rst.blockmode.patch jbell, 2008-05-12 21:25 Documentation patch review
Pull Requests
URL Status Linked Edit
PR 29337 closed jbell, 2021-10-30 23:59
Messages (20)
msg65454 - (view) Author: Jonathan Bell (jbell) * Date: 2008-04-13 23:50
About a year ago I found myself fighting a broken FTP server that
couldn't handle multiple passive data transfers through a firewall or
NATed connection. Thankfully, this same problem server supports block
transmission mode, which allows a client to create a single data
connection for transferring multiple files.

I've attached a patch (and sample debug output) for the latest trunk.

Might this be useful to anyone else in any way? I realize any MODE
option rather than S is widely unsupported and possibly an edge case,
but getting this into trunk would be preferable. We've been running this
code under Python2.3 for nearly a year -- the jobs run several times per
hour -- and are extremely happy with the results.
msg65455 - (view) Author: Jonathan Bell (jbell) * Date: 2008-04-13 23:54
Here's the debug output when the transfers are going well.
msg66305 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-05-06 07:23
rather than using the array.array for your three byte header with manual
parsing, please use struct.unpack.

Whats a good way to test that this works?  It'd be nice to have a unit
test (a test_ftplib_net.py perhaps?) though I realize ftplib currently
has poor test coverage with no such explicit network test existing
beyond things the urllib tests might do.

also, could you make the appropriate documentation updates against
python trunk's Doc/lib/ftplib.rst.  Mention when & what servers you
found using this mode useful with if at all possible.
msg66762 - (view) Author: Jonathan Bell (jbell) * Date: 2008-05-12 21:47
I've attached two new files. The first swaps the array.array usage for
struct.unpack. The second simply modifies the rst documentation.

I'm not sure how we'd do any tests for FTP without making use of an
actual server. In a quick check of servers, MadGoat (for OpenVMS) was
the only BLOCK-supporting server I found; neither vsftpd nor proftpd
support BLOCK. (I didn't check wuftpd.) Sadly, I've no publicly
accessible servers available to me for others to test against.
msg76248 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-11-22 14:26
An actual test suite for ftplib is now available.
Should we reconsider revamping this issue?
msg76293 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-11-24 01:54
sure go for it, i haven't had time to look at the latest patch myself.
msg76336 - (view) Author: Jonathan Bell (jbell) * Date: 2008-11-24 16:44
Yeah, I'm glad to see a test suite. I've only skimmed the test, but it
seems like an excellent starting point. Initial thoughts for updating
the tests:
- Need a cmd_mode to handle the MODE command.
- Suite cmd_retr logic needs to keep dtp connection open and write a
simple header depending on MODE.

Due to the Thanksgiving holiday, I may be without network access (or
time) until next week, however.
msg404920 - (view) Author: mike mcleod (mikecmcleod) * Date: 2021-10-24 08:59
Hi,

I would like to help on this issue.
msg405014 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2021-10-26 00:36
We don't have a CLA from jbell.  I've sent an email asking him to do so... we'll see what happens.
msg405036 - (view) Author: mike mcleod (mikecmcleod) * Date: 2021-10-26 12:05
Hi Ethan,

Thanks, awaiting reply..

Regards,
Mike

On Tue, 26 Oct 2021 at 01:36, Ethan Furman <report@bugs.python.org> wrote:

>
> Ethan Furman <ethan@stoneleaf.us> added the comment:
>
> We don't have a CLA from jbell.  I've sent an email asking him to do so...
> we'll see what happens.
>
> ----------
> nosy: +ethan.furman
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue2628>
> _______________________________________
>
msg405386 - (view) Author: Jonathan Bell (jbell) * Date: 2021-10-30 21:23
The CLA is signed, and I'm again able to work on this.

I was able to update this locally for Python 3 with a minimal test case. What specifically were you looking for?
msg405388 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2021-10-30 22:01
If you're familiar with git and GitHub, can you create a PR?  Otherwise, an updated patch here and we'll work on getting it merged.
msg405402 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2021-10-31 13:32
Hello.
I added some initial comments to the PR, but I'm sort of skeptical about this. It must be noted that:

1) very few FTP servers probably support this feature (https://en.wikipedia.org/wiki/File_Transfer_Protocol#Data_transfer_modes)

2) the specs are very old (RFC-959 is from 1985), and I doubt they've been upgraded in later RFCs. The fact that a header is sent *before every data block* seems inefficient (why not just send the  file size once?), probably more inefficient that opening a new connection each time (unless files are small, I suppose).

Was this tested against an actual FTP server(s)? If yes, which one(s)? IMO, it would be good if some actual research/testing is done first, to see how actual FTP server products implement this feature.

Another thing to note is that the PR supports RETR (download) only, and not STOR (upload). Is this on purpose or does the original RFC/spec limits this functionality to RETR?
msg405407 - (view) Author: Jonathan Bell (jbell) * Date: 2021-10-31 16:46
This issue is 13 years old. The original 2008 patch was used in a production environment against an OpenVMS server identifying itself as MadGoat. That use case involved downloading documents only, and no write permission was available. Therefore the patch only supports RETR. See the debug.log file attached to this issue for the server interaction. 

I no longer have a need for BLOCK mode, and don't know what modern servers would support it. mikecmcleod revived this issue so perhaps they can provide some ability for testing, or perspective on the current needs.

The PR updates the patch to Python 3, and includes a test written against the minimal changes required for that 2.7->3.x update.
msg405426 - (view) Author: mike mcleod (mikecmcleod) * Date: 2021-11-01 10:35
I am happy to do any testing.
My reason for getting involved is I am new to helping with Cpython and thought this may be the least intrusive way of getting started with something that nobody really cares about that much.
Hence, the oldest issue I see can be either completed as first envisioned or can be closed and I am ok with either.
msg405443 - (view) Author: Jonathan Bell (jbell) * Date: 2021-11-01 14:22
No practical method exists to verify BLOCK transmission mode, which as mentioned earlier, was rarely implemented even when this issue was opened. Given that reality, I'm inclined to close this issue.
msg405457 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2021-11-01 17:50
I would like to have the issue fixed instead of just closed.

Jonathan, you say there is no practical method to verify that the block transmission mode exists -- so it's only useful if the user knows that it exists?  If the user tries it and the server does not support it, is a useful exception raised?  If the answer is yes (to the useful exception) I'm would like to see it added.
msg405480 - (view) Author: Jonathan Bell (jbell) * Date: 2021-11-02 03:14
I should rephrase: There doesn't seem to be a practical way to verify BLOCK transmission mode against actual servers in the wild. As the Wikipedia article that Giampaolo referenced points out, BLOCK mode is a rarity that was primarily supported only by mainframe and minicomputer systems.

Any compliant server not supporting BLOCK should respond with a non-200 response. The PR sends its request to enter BLOCK mode with self.voidcmd(), which handles non-200 responses by raising error_reply.

When I originally wrote that patch in 2008, such a system was running on a DEC Alpha under OpenVMS. Within months of the first test suite appearing for ftplib, that same vendor replaced their systems. The new server had no BLOCK transmission support, but was capable of handling multiple consecutive passive mode STREAM data connections without fault.

Even at the time, I couldn't find any other freely available FTP servers supporting BLOCK. But STREAM was and continues to be the standard.

Essentially this means that any changes to the existing PR may not be work properly with actual servers.
msg405492 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2021-11-02 05:52
Ah.  Well, in that case closing seems like the best idea.

Thank you, Jonathan, for getting the CLA signed and providing perspective.

Thank you, Mike, for moving this issue forward.
msg405497 - (view) Author: mike mcleod (mikecmcleod) * Date: 2021-11-02 09:33
You're welcome.
Regards,
Mike

On Tue, 2 Nov 2021 at 05:52, Ethan Furman <report@bugs.python.org> wrote:

>
> Ethan Furman <ethan@stoneleaf.us> added the comment:
>
> Ah.  Well, in that case closing seems like the best idea.
>
> Thank you, Jonathan, for getting the CLA signed and providing perspective.
>
> Thank you, Mike, for moving this issue forward.
>
> ----------
> resolution:  -> rejected
> stage: patch review -> resolved
> status: open -> closed
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue2628>
> _______________________________________
>
History
Date User Action Args
2022-04-11 14:56:33adminsetgithub: 46880
2021-11-02 09:33:31mikecmcleodsetmessages: + msg405497
2021-11-02 05:52:28ethan.furmansetstatus: open -> closed
resolution: rejected
messages: + msg405492

stage: patch review -> resolved
2021-11-02 03:14:17jbellsetmessages: + msg405480
2021-11-01 17:50:35ethan.furmansetmessages: + msg405457
versions: + Python 3.11, - Python 3.2
2021-11-01 14:22:20jbellsetmessages: + msg405443
2021-11-01 10:35:06mikecmcleodsetmessages: + msg405426
2021-10-31 16:46:04jbellsetmessages: + msg405407
2021-10-31 13:32:18giampaolo.rodolasetmessages: + msg405402
2021-10-30 23:59:34jbellsetstage: test needed -> patch review
pull_requests: + pull_request27604
2021-10-30 22:01:31ethan.furmansetmessages: + msg405388
2021-10-30 21:23:57jbellsetmessages: + msg405386
2021-10-26 12:05:24mikecmcleodsetmessages: + msg405036
2021-10-26 00:36:45ethan.furmansetnosy: + ethan.furman
messages: + msg405014
2021-10-24 08:59:30mikecmcleodsetnosy: + mikecmcleod
messages: + msg404920
2010-08-07 16:33:11terry.reedysetstage: test needed
2010-08-07 16:22:17terry.reedysetversions: + Python 3.2, - Python 2.6
2010-08-07 16:17:14terry.reedysetfiles: - ftplib.py.blockmode.patch
2008-11-24 16:44:13jbellsetmessages: + msg76336
2008-11-24 01:54:35gregory.p.smithsetmessages: + msg76293
2008-11-22 14:26:27giampaolo.rodolasetmessages: + msg76248
2008-05-12 21:47:35jbellsetmessages: + msg66762
2008-05-12 21:25:13jbellsetfiles: + ftplib.rst.blockmode.patch
2008-05-12 21:24:47jbellsetfiles: + ftplib.py.blockmode.patch
2008-05-06 07:23:26gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg66305
2008-04-14 15:09:51giampaolo.rodolasetnosy: + giampaolo.rodola
2008-04-13 23:54:53jbellsetfiles: + debug.log
messages: + msg65455
2008-04-13 23:50:10jbellcreate