Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
corona10 opened this issue Jan 8, 2020 · 13 comments
Closed

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

corona10 opened this issue Jan 8, 2020 · 13 comments
Labels
3.9 only security fixes stdlib Python modules in the Lib dir

Comments

@corona10
Copy link
Member

corona10 commented Jan 8, 2020

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
    Copy link
    Member Author

    corona10 commented Jan 8, 2020

    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.

    @corona10 corona10 added 3.9 only security fixes stdlib Python modules in the Lib dir labels Jan 8, 2020
    @vstinner
    Copy link
    Member

    vstinner commented Jan 8, 2020

    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?

    @vstinner vstinner changed the title poplib.POP3/POP3_SSL should reject timeout = 0 poplib.POP3/POP3_SSL should reject timeout = 0 (non-blocking mode) Jan 8, 2020
    @vstinner vstinner changed the title poplib.POP3/POP3_SSL should reject timeout = 0 poplib.POP3/POP3_SSL should reject timeout = 0 (non-blocking mode) Jan 8, 2020
    @corona10
    Copy link
    Member Author

    corona10 commented Jan 8, 2020

    Sure, I will submit the PR by tomorrow :)

    @corona10
    Copy link
    Member Author

    corona10 commented Jan 9, 2020

    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
    Copy link
    Member

    vstinner commented Jan 9, 2020

    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
    Copy link
    Member

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

    @vstinner
    Copy link
    Member

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

    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

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

    @corona10
    Copy link
    Member Author

    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
    Copy link
    Member

    Thanks Dong-hee Na for all these nice changes!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @corona10