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.

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

bpo-39259: poplib.POP3/POP3_SSL now reject timeout = 0 #17912

Merged
merged 4 commits into from Jan 10, 2020

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Jan 9, 2020

@corona10 corona10 requested a review from a team as a code owner Jan 9, 2020
@corona10 corona10 requested a review from vstinner Jan 9, 2020
@@ -505,17 +505,17 @@ def server(self, evt, serv):

def testTimeoutDefault(self):
self.assertIsNone(socket.getdefaulttimeout())
socket.setdefaulttimeout(30)
socket.setdefaulttimeout(test_support.SHORT_TIMEOUT)
Copy link
Member

@vstinner vstinner Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I added timeout constants to the test.support module, I decided to leave this test unchanged. I understood that the test doesn't connect to real socket. Or does it? I'm not sure. If it does, LOOPBACK_TIMEOUT is preferred.

Copy link
Member Author

@corona10 corona10 Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.welcome = self._getresp()

As you can see poplib client's constructor actually makes a connection to the server and get a response.

self.thread = threading.Thread(target=self.server, args=(self.evt,self.sock))

So this test intends to check whether poplib client is set by a global socket timeout and also check whether the connection is succeeded.

@corona10
Copy link
Member Author

corona10 commented Jan 9, 2020

@vstinner I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Jan 9, 2020

Thanks for making the requested changes!

: please review the changes made to this pull request.

Doc/library/poplib.rst Outdated Show resolved Hide resolved
Co-Authored-By: Victor Stinner <vstinner@python.org>
Doc/library/poplib.rst Outdated Show resolved Hide resolved
Co-Authored-By: Victor Stinner <vstinner@python.org>
Copy link
Member Author

@corona10 corona10 left a comment

@vstinner I 've updated the PR.
For other modules, I will submit the PR seperately.

@vstinner vstinner merged commit c39b52f into python:master Jan 10, 2020
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
poplib.POP3 and poplib.POP3_SSL now raise a ValueError
if the given timeout for their constructor is zero to
prevent the creation of a non-blocking socket.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants