Mailing List Archive: Python: Bugs

[issue2054] add ftp-tls support to ftplib - RFC 4217

1 2

View All

Index | Next | Previous | View Threaded

report at bugs

Oct 15, 2009, 10:06 AM

Post #1 of 41 (706 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217

Changes by Lance Edgar <lance [at] edbob>:

----------
nosy: +lgedgar

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:26 AM

Post #2 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

Are there chances to see this reviewed?

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #3 of 41 (682 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file9775/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #4 of 41 (682 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file9782/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #5 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file9783/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #6 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file9807/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #7 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file10784/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #8 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file13161/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:53 AM

Post #9 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Antoine Pitrou <pitrou [at] free>:

Removed file: http://bugs.python.org/file13171/unnamed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 10:56 AM

Post #10 of 41 (684 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

Well, first some tests should be added.
As for reviewing, it needs to be done by someone competent with FTP and
SSL/TLS. If no such person is available and you are confident that the
patch is ok (and ready to do necessary maintenance), then perhaps you
can commit it. But please add tests first. It is really too easy to
accidentally break a functionality which is not exercised by the test suite.

Oh, and you need to update the documentation too :)

----------
stage: -> patch review
versions: +Python 2.7, Python 3.2 -Python 2.6, Python 3.0

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 11:08 AM

Post #11 of 41 (682 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

I can surely write tests altough I think that issue 3890 might cause
some problems since the test server I included some months ago is
asyncore-based and hence asynchronous.

I have a good knowledge of the FTP protocol but someone with a good
knowledge of SSL willing to review the patch would surely be useful.

In particular I would ask Bill an update about this comment:

> Probably what I should do is fix httplib, that would provide an
> example we could extend to the rest of the modules.
> Bill

I don't follow the development of other stdlib modules that use ssl so
I'm not sure how to proceed to conform with them.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 11:27 AM

Post #12 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

>From a quick look at the patch, if you call login() twice, the socket
will be wrapped twice as well? Perhaps auth_tls() should have a
protection against this.

In prot_p() and prot_c(), it seems that self._prot_p is updated
unconditionally, regardless of the FTP response.

In retrbinary(), retrlines(), storbinary() and storlines(), you
certainly want some try/finally blocks so that the data connection
always gets closed at the end.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 15, 2009, 11:34 AM

Post #13 of 41 (680 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

One more question, why is "ssl_version=ssl.PROTOCOL_TLSv1" hardwired?

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 16, 2009, 11:43 AM

Post #14 of 41 (659 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

> From a quick look at the patch, if you call login() twice, the socket
> will be wrapped twice as well? Perhaps auth_tls() should have a
> protection against this.

You're right. Done.

> In prot_p() and prot_c(), it seems that self._prot_p is updated
> unconditionally, regardless of the FTP response.

Both PROT P and PROT C commands expect a 2xx response.
That's why I used voidcmd(). If a response different than 2xx is
received voidcmd() automatically raises an error_reply exception already
and self._prot_p doesn't get set.

> One more question, why is "ssl_version=ssl.PROTOCOL_TLSv1" hardwired?

You're right, it shouldn't be.
This is now provided as a class attribute.
The reason I used PROTOCOL_TLSv1 instead of PROTOCOL_SSLv23 (the ssl.py
default) is because RFC-4217 recommends it.

> In retrbinary(), retrlines(), storbinary() and storlines(), you
> certainly want some try/finally blocks so that the data connection
> always gets closed at the end.

I agree, it's a good practice.
I avoided to do that because the original FTP class is coded like that.
Anyway, this is done too now.

Modified patch is in attachment.

I hope I'll be able to work on tests and documentation during this week-
end.

----------
Added file: http://bugs.python.org/file15146/ftplib.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 16, 2009, 11:47 AM

Post #15 of 41 (661 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Giampaolo Rodola' <billiejoex [at] users>:

Removed file: http://bugs.python.org/file15146/ftplib.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 16, 2009, 11:48 AM

Post #16 of 41 (661 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Changes by Giampaolo Rodola' <billiejoex [at] users>:

Added file: http://bugs.python.org/file15147/ftplib.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 17, 2009, 1:50 PM

Post #17 of 41 (653 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

A patch including tests and documentation is now in attachment.
The test TLS server is very similar to pyftpdlib's I draw on:
http://code.google.com/p/pyftpdlib/source/browse/trunk/demo/tls_ftpd.py
I wasn't able to "compile" the documentation so I'm not 100% sure it looks
perfectly.
Test suite run successfully on Windows XP, Ubuntu 9.04 and FreeBSD 7.1.

----------
nosy: +josiah.carlson, josiahcarlson
Added file: http://bugs.python.org/file15159/ftplib.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 18, 2009, 1:47 PM

Post #18 of 41 (623 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

You can build the docs by going to the Doc directory and typing "make
html" there. It isn't critical anyway.

The tests failed to run, I had to replace the KEYCERT declaration with:

KEYCERT = os.path.join(os.path.dirname(__file__), "keycert.pem")

(and add "import os" at the top)

Another remark: login() doesn't return the response, while it does on
the normal FTP class.

Apart from that, I'm trying to test on a TLS-enabled FTP server, but
even the regular FTP class doesn't seem to work with it (I get "500
Illegal PORT command" when calling retrlines('LIST')).

----------
keywords: -easy

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 5:55 AM

Post #19 of 41 (620 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

> You can build the docs by going to the Doc directory and typing "make
> html" there. It isn't critical anyway.

Done. It's well formatted now.

> The tests failed to run, I had to replace the KEYCERT declaration with:
> KEYCERT = os.path.join(os.path.dirname(__file__), "keycert.pem")
> (and add "import os" at the top)

Done.

> Another remark: login() doesn't return the response, while it does on
> the normal FTP class.

Good catch, thanks.
Done.

> Apart from that, I'm trying to test on a TLS-enabled FTP server, but
> even the regular FTP class doesn't seem to work with it (I get "500
> Illegal PORT command" when calling retrlines('LIST')).

Strange. I doubt this issue is related with my changes.
I tried FTP_TLS agains proftpd, filezilla server and pyftpdlib and it
works fine.

New patch in attachment btw.

----------
Added file: http://bugs.python.org/file15163/ftplib.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 8:33 AM

Post #20 of 41 (617 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

The patch is ok to me. Perhaps Bill wants to take a look, otherwise I
think you can commit.

----------
resolution: -> accepted
stage: patch review -> commit review

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 10:00 AM

Post #21 of 41 (628 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

A last problem:

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'classobj'

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 11:38 AM

Post #22 of 41 (615 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

> A last problem:
>
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> TypeError: attribute name must be string, not 'classobj'

Mmmm this doesn't say much.
When does it happen?
Is that the complete traceback message?

> The patch is ok to me. Perhaps Bill wants to take a look.

I'd like the opinion of Bill too, specifically about what he was talking
about in comments 62699 and 64093.

> otherwise I think you can commit.

I don't have commit privileges. Someone else should do it.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 11:47 AM

Post #23 of 41 (614 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

Ah, sorry, roundup's e-mail interface ate part of the message.
The error happens when doing "from ftplib import *". Apparently __all__
contains a non-string value.

> I don't have commit privileges. Someone else should do it.

Ok, I'll do it if Bill doesn't give a sign of life.

----------
assignee: -> pitrou

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 11:54 AM

Post #24 of 41 (623 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

Regarding msg64093, the only API change Bill's suggestion would entail
is an additional optional parameter to the constructor, so adding it
later would be backwards-compatible.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Oct 19, 2009, 12:02 PM

Post #25 of 41 (614 views)
Permalink

[issue2054] add ftp-tls support to ftplib - RFC 4217 [In reply to]

Giampaolo Rodola' <billiejoex [at] users> added the comment:

> Ah, sorry, roundup's e-mail interface ate part of the message.
> The error happens when doing "from ftplib import *". Apparently
__all__
> contains a non-string value.

Oh, shame on me! You're right.
Thanks for the great review you're doing.

> Regarding msg64093, the only API change Bill's suggestion would entail
> is an additional optional parameter to the constructor, so adding it
> later would be backwards-compatible.

Good.
Possibly there might be another change we might want to do in future,
which is adding support for CCC command, useful for reverting the
control connection back to clear-text.
I remember I tried to do that at the time I submitted the first patch
but there was a problem with ssl's unwrap() method (I really don't
remember what exactly).

Anyway, I'll try to put hands on that soon and let you know what
happens.
The change should be backward compatible as it just consists in adding a
ccc() method.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue2054>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

1 2

View All

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads