poplib.POP3/POP3_SSL should reject timeout = 0 (non-blocking mode)

[corona10]

BPO	39259
Nosy	@vstinner, @corona10
PRs	bpo-39259: poplib.POP3/POP3_SSL now reject timeout = 0 #17912 bpo-39259: nntplib.NNTP/NNTP_SSL now reject timeout = 0 #17936 bpo-39259: nntplib.NNTP/NNTP_SSL refactoring #17939 bpo-39259: smtp.SMTP/SMTP_SSL now reject timeout = 0 #17958 bpo-39259: ftplib.FTP/FTP_TLS now reject timeout = 0 #17959

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-01-14.12:05:17.399>
created_at = <Date 2020-01-08.14:55:46.591>
labels = ['library', '3.9']
title = 'poplib.POP3/POP3_SSL should reject timeout = 0 (non-blocking mode)'
updated_at = <Date 2020-01-14.12:05:40.618>
user = 'https://github.com/corona10'

bugs.python.org fields:

activity = <Date 2020-01-14.12:05:40.618>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-01-14.12:05:17.399>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2020-01-08.14:55:46.591>
creator = 'corona10'
dependencies = []
files = []
hgrepos = []
issue_num = 39259
keywords = ['patch']
message_count = 13.0
messages = ['359596', '359599', '359604', '359646', '359660', '359727', '359799', '359815', '359930', '359951', '359952', '359954', '359968']
nosy_count = 2.0
nosy_names = ['vstinner', 'corona10']
pr_nums = ['17912', '17936', '17939', '17958', '17959']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39259'
versions = ['Python 3.9']

[corona10]

Since poplib.POP3/POP3_SSL's implementation depends on socket.makefile, the client should reject if the timeout is zero.
Because socket.makefile said that 'The socket must be in blocking mode' and if we set timeout to zero, the client does not operate as normal.

[vstinner]

I agree, I was the one who suggest to reject timeout=0 when you added the parameter to imaplib :-)

Do you want to work on a PR?

[corona10]

Sure, I will submit the PR by tomorrow :)

[corona10]

ftplib:

cpython/Lib/ftplib.py

Line 155 in efa3b51

self.file = self.sock.makefile('r', encoding=self.encoding)

nntplib:

cpython/Lib/nntplib.py

Line 1049 in 2e6a8ef

file = self.sock.makefile("rwb")

smtplib:

cpython/Lib/smtplib.py

Line 386 in 3faf826

self.file = self.sock.makefile('rb')

I found more things that depends on socket.makefile.
IMHO, we can apply this change to those modules. What do you think?

[vstinner]

ftplib:

cpython/Lib/ftplib.py

Line 155 in efa3b51

self.file = self.sock.makefile('r', encoding=self.encoding)

nntplib:

cpython/Lib/nntplib.py

Line 1049 in 2e6a8ef

file = self.sock.makefile("rwb")

smtplib:

cpython/Lib/smtplib.py

Line 386 in 3faf826

self.file = self.sock.makefile('rb')

It seems like these 3 modules use makefile() and have a timeout parameter. If makefile() doesn't support non-blocking mode, sure, it's a good idea to also reject explicitly timeout=0 in these modules.

[vstinner]

New changeset c39b52f by Victor Stinner (Dong-hee Na) in branch 'master':
bpo-39259: poplib now rejects timeout = 0 (GH-17912)
c39b52f

[vstinner]

New changeset 5d978a2 by Victor Stinner (Dong-hee Na) in branch 'master':
bpo-39259: nntplib.NNTP/NNTP_SSL refactoring (GH-17939)
5d978a2

[vstinner]

New changeset 1b335ae by Victor Stinner (Dong-hee Na) in branch 'master':
bpo-39259: nntplib.NNTP/NNTP_SSL now reject timeout = 0 (GH-17936)
1b335ae

[vstinner]

New changeset a190e2a by Victor Stinner (Dong-hee Na) in branch 'master':
bpo-39259: ftplib.FTP/FTP_TLS now reject timeout = 0 (GH-17959)
a190e2a

[vstinner]

New changeset 62e3973 by Victor Stinner (Dong-hee Na) in branch 'master':
bpo-39259: smtp.SMTP/SMTP_SSL now reject timeout = 0 (GH-17958)
62e3973

[vstinner]

Can we now close this issue? Or is there still something to do?

[corona10]

Can we now close this issue? Or is there still something to do?

There is no case on xxlib series except LMTP.
I am going to open a new issue about LMTP.
So let's close this :)

[vstinner]

Thanks Dong-hee Na for all these nice changes!