classification
Title: Error calling .storlines from ftplib
Type: behavior Stage: resolved
Components: Documentation, Library (Lib) Versions: Python 3.3, Python 3.1, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: BreamoreBoy, amaury.forgeotdarc, aymanhs, danonymous, giampaolo.rodola, haypo, pitrou, python-dev, rdevaughn
Priority: normal Keywords: patch

Created on 2009-09-02 13:01 by rdevaughn, last changed 2013-04-02 20:14 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
module.txt rdevaughn, 2009-09-02 13:01 Sample code- private info redacted
ftplib-2.patch haypo, 2009-09-08 22:48
ftplib.patch giampaolo.rodola, 2010-05-01 19:16
ftplib_doc.patch haypo, 2010-08-27 18:28
Messages (18)
msg92166 - (view) Author: Robert DeVaughn (rdevaughn) Date: 2009-09-02 13:01
When attempting to store a file via FTP, the following error occurs. See 
attached code.

Traceback (most recent call last):
  File "C:\Documents and 
Settings\rdevaughn\Desktop\HTTP\src\FTP_directory.py", line 14, in 
<module>
    ftp.storlines("STOR source.txt",f)
  File "C:\Python30\lib\ftplib.py", line 477, in storlines
    if buf[-1] in B_CRLF: buf = buf[:-1]
TypeError: Type str doesn't support the buffer AP
msg92434 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-09-08 21:59
Oh yes, FTP.storlines() fails if the file is a text file. Here is a
patch. My patch encodes each line with self.encoding, which is latin1 by
default.
msg92435 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-09-08 22:23
Does this still pass the test suite? in test_ftplib, storlines() is given 
a BytesIO object.
msg92436 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-09-08 22:48
@amaury.forgeotdarc: Oops, no, it doesn't.

The new patch includes a new test (for the unicode file).
msg104740 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-01 19:16
Attached is a patch which fixes FTP_TLS class as well and changes the test server a little bit.
Shouldn't retrlines() suffer the same issue?
msg112786 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-04 10:49
To me this isn't a bug, and the patch introduces incorrect behaviour. If you want to store data on an FTP server, you have to provide binary data, not text data. The FTP class is not supposed to guess in which charset your data should be encoded.

(the "encoding" argument on the FTP class is meant for protocol commands (such as file names), not for file contents)
msg113166 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-07 11:09
I agree with Antoine. Maybe it would make sense to check the file object passed to storlines() and raise an exception if mode != binary.
msg114321 - (view) Author: Ayman (aymanhs) Date: 2010-08-19 05:46
I do not agree with Antoine.  For binary transfer, another method is called, and it does work fine.  However, storelines would be called for ASCII mode, in which encoding and decoding should be done by the FTP program.  ASCII mode would translate files, so a file loaded on a PC may not be identical to the same file when loaded on a Mac.
msg114346 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-19 11:45
> I do not agree with Antoine.  For binary transfer, another method is
> called, and it does work fine.  However, storelines would be called
> for ASCII mode, in which encoding and decoding should be done by the
> FTP program.  ASCII mode would translate files, so a file loaded on a
> PC may not be identical to the same file when loaded on a Mac.

Well if it's really "ASCII" mode, it should refuse any character > 127,
in which case I would agree to hardcode ascii as an encoding.

But, in any case, storing "lines" has a meaning in binary mode, as
witnessed by the fact that binary files in Python 3 have a readline()
method. This means storlines should still succeed for binary input, even
if it is made compatible with text input.

I would personally argue that the text transfer mode in FTP is an
ill-conceived feature from the start and ftplib should do whatever is
reasonable nowadays, rather than what looked reasonable 30 years ago.
msg114519 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-21 17:14
Just my 2 cents to add that FTP ASCII mode should consists in nothing but replacing os.sep in "\r\n" before sending file data.
The opposite operation must be done by the receiving peer which has to read the data from the socket and replace "\r\n" with os.sep.
That should be all: no further conversion should be done against the data.
msg114790 - (view) Author: danonymous (danonymous) Date: 2010-08-24 14:29
There seems to be a lot of confusion on this thread.  So I'll add mine and hopefully clear things up a bit.    In short I think aymanhs has it right.

This is what I get when trying to send a simple text file on a Windows Xp PC using python 3.x
-----------------------------------------------------------------
    ftp.storlines('STOR ' + localFile, fd)                  # Send the file by issuing the STOR command.
  File "C:\Python30\lib\ftplib.py", line 477, in storlines
    if buf[-1] in B_CRLF: buf = buf[:-1]
TypeError: Type str doesn't support the buffer API

-----------------------------------------------------------------

getting up on my soap box....  :-)

FTP has two modes binary and ascii.  When doing binary you expect your data to be sent as is with NO changes.  When doing ascii, you expect to send a text file with your line endings to be adjusted if they are not the same on the two platforms involved in the FTP transfer (MAC, Linux, PC, etc.).  

So I do think it's an bug.  Every other FTP program I've used, and it's a long list, has two distinct modes ASCII and Binary.  When using ASCII, the line endings are translated.  I don't know enough to comment on whether the ascii code for every character is supposed to be 127 or less.  But I can tell you that I've never had another FTP program fail like Python 3 is failing for me trying to what seems like a simple transfer of a text file (expecting the line endings to be adjusted).  

I think the goal here should be (must be) to make it work like it is expected to work, based on the expectations of millions of people who have been using FTP programs which have come before python for several decades.  The other finer points are really not important.

stepping off the soap box.
msg114794 - (view) Author: danonymous (danonymous) Date: 2010-08-24 15:33
One other observation on this issue.

I just 'backleveled' the python on my PC from 3.x to 2.7.  My test script works fine on 2.7.

- Dan
msg114795 - (view) Author: Ayman (aymanhs) Date: 2010-08-24 16:18
My other 2 cents worth.

Actually that is my main issue.  Things used to work in 2.x, and
"suddenly" refused to work after my 3.x upgrade.

I'm not saying it worked means it is correct, but the Exception being
thrown does not look right.  I think we should always have core
libraries work out of the box, and have workarounds even if it's not
"our" error.

 Ayman
msg115106 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-27 17:58
> But I can tell you that I've never had another FTP program fail like
> Python 3 is failing for me trying to what seems like a simple transfer
> of a text file (expecting the line endings to be adjusted).  

Python 3 is not failing, you are just using it the wrong way. Open your
file in binary mode (as opposed to text mode) and it should work. "Text
files" in Python 3 are files decoded to unicode while reading (and
encoded from unicode while writing); they are much more than simply
binary files with adaptive line endings. This is more or less documented
here (although I agree it would perhaps need a better introductory
paragraph):
http://docs.python.org/py3k/library/io.html
msg115112 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-08-27 18:28
pitrou> The FTP class is not supposed to guess in which charset 
pitrou> your data should be encoded.
pitrou> (the "encoding" argument on the FTP class is meant 
pitrou> for protocol commands (such as file names), not for file
pitrou> contents)

I agree, my patch is completly wrong :-) You should open your file in binary mode, not in text (unicode) mode. The issue is more a documentation bug, than a bug in ftplib behaviour.

Here is a new patch: very short patch on the doc (just add "binary" to "open binary file object *file*").
msg185791 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2013-04-02 01:52
ftplib_doc.patch is so simple I'm assuming this has slipped under the radar, so would someone like to do the honours and commit the patch please.
msg185851 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2013-04-02 17:55
Apparently the doc has already been changed and mentions that a binary file is necessary. The unit test is legitimate and can be committed though (Victor, you want to do it?).
msg185855 - (view) Author: Roundup Robot (python-dev) Date: 2013-04-02 20:14
New changeset c78dfc6ce37a by Victor Stinner in branch '3.3':
Close #6822: ftplib.FTP.storlines() expects a binary file, not a text file
http://hg.python.org/cpython/rev/c78dfc6ce37a

New changeset 9328e2b8a397 by Victor Stinner in branch 'default':
(Merge 3.3) Close #6822: ftplib.FTP.storlines() expects a binary file, not a text file
http://hg.python.org/cpython/rev/9328e2b8a397
History
Date User Action Args
2013-04-02 20:14:01python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg185855

resolution: fixed
stage: resolved
2013-04-02 17:55:37giampaolo.rodolasetmessages: + msg185851
2013-04-02 01:52:37BreamoreBoysetnosy: + BreamoreBoy
messages: + msg185791
2010-08-27 18:28:23hayposetfiles: + ftplib_doc.patch

messages: + msg115112
components: + Documentation
nosy: amaury.forgeotdarc, pitrou, haypo, giampaolo.rodola, aymanhs, rdevaughn, danonymous
2010-08-27 17:58:03pitrousetmessages: + msg115106
2010-08-24 16:18:04aymanhssetmessages: + msg114795
2010-08-24 15:33:59danonymoussetmessages: + msg114794
versions: + Python 3.1, Python 3.2, Python 3.3
2010-08-24 14:29:33danonymoussetnosy: + danonymous
messages: + msg114790
2010-08-24 14:15:02danonymoussetversions: - Python 3.1, Python 3.2
2010-08-21 17:14:05giampaolo.rodolasetmessages: + msg114519
2010-08-19 11:45:58pitrousetmessages: + msg114346
2010-08-19 05:46:27aymanhssetnosy: + aymanhs
messages: + msg114321
2010-08-07 11:09:28giampaolo.rodolasetmessages: + msg113166
2010-08-04 10:49:32pitrousetnosy: + pitrou
messages: + msg112786
2010-05-01 19:16:59giampaolo.rodolasetfiles: + ftplib.patch

nosy: + giampaolo.rodola
versions: + Python 3.1, Python 3.2, - Python 3.0
messages: + msg104740

assignee: giampaolo.rodola
2010-04-17 21:13:52r.david.murraylinkissue6789 superseder
2009-09-08 22:48:17hayposetfiles: - ftplib.patch
2009-09-08 22:48:08hayposetfiles: + ftplib-2.patch

messages: + msg92436
2009-09-08 22:23:21amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg92435
2009-09-08 21:59:22hayposetfiles: + ftplib.patch

nosy: + haypo
messages: + msg92434

keywords: + patch
2009-09-02 14:24:09benjamin.petersonsettype: compile error -> behavior
2009-09-02 13:01:28rdevaughncreate