bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR #17311

aeros · 2019-11-21T03:00:17Z

Edit: This PR has been revised based on @pitrou's proposal, due to Windows compatibility concerns.

https://bugs.python.org/issue37228

1st1

I actually think that Antoine solution is more preferable. @asvetlov?

bedevere-bot · 2019-11-21T03:11:58Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

aeros · 2019-11-21T03:21:07Z

@1st1

I actually think that Antoine solution is more preferable

I prefer Antoine's solution as well as far as elegance and API design goes, my only concern is that it would be fairly disruptive with regards to backwards compatibility (from raising an error upon passing reuse_address=True) to apply all the way back to 3.5+. I'll wait for @asvetlov's feedback though.

Edit: If the consensus is to use Antoine's proposal instead, I can modify this PR accordingly. As long as it's within the next couple of days, I should have the time to do so. If I don't have enough time, I'll close this PR and let someone else implement it.

aeros · 2019-11-21T08:51:26Z

Due to Windows compatibility concerns, I no longer think that changing SO_REUSEADDR -> SO_REUSEPORT is going to be a viable option. As a result, I will start working on implementing Antoine's proposal. For more details, see https://bugs.python.org/msg357143.

aeros · 2019-11-21T09:49:00Z

Doc/library/asyncio-eventloop.rst

@@ -527,6 +529,10 @@ Opening network connections
      The *family*, *proto*, *flags*, *reuse_address*, *reuse_port,
      *allow_broadcast*, and *sock* parameters were added.

+   .. versionchanged:: 3.5.10


I'm assuming that version 3.5.10 would be the next released 3.5.x, and the earliest version that this change will apply to. Is this correct?

Since this is being added to multiple releases mid-stream, I think the right approach is for each branch to have its own versionchanged value, except perhaps master which would also have the 3.8 version (since 3.9 isn't released yet). So perhaps change this to the right 3.8.x version and then either manually cherry-pick for 3.7 and earlier (which might need to be done anyway if there are merge conflicts) or submit separate post-merge PRs for them to change the versionadded value.

@ned-deily

So perhaps change this to the right 3.8.x version and then either manually cherry-pick for 3.7 and earlier (which might need to be done anyway if there are merge conflicts) or submit separate post-merge PRs for them to change the versionadded value.

Sounds good, I can do that. I'll set master and the 3.8 branch to 3.8.1; 3.7 to 3.7.6; 3.6 to 3.6.10; and 3.5 to 3.5.10. The easiest way to handle this would probably be through using cherry-pick to open the backport PRs manually. As a result, I'll remove the ``needs backport to x` labels, so that @miss-islington doesn't automatically open the backport PRs.

Should we check with @larryhastings regarding the 3.5 backport? I believe it would be appropriate since this is a significant security vulnerability.

Lib/asyncio/base_events.py

aeros · 2019-11-21T10:30:50Z

@1st1 I finished changing the PR based on @pitrou's proposal, due to the Windows compatibility issues with the previous implementation.

I have made the requested changes; please review again.

bedevere-bot · 2019-11-21T10:30:52Z

Thanks for making the requested changes!

@1st1: please review the changes made to this pull request.

aeros · 2019-11-21T21:32:09Z

I'll add a Misc/NEWS entry.

aeros · 2019-12-08T00:32:24Z

Based on @ned-deily's suggestion to manually set the versionchanged to separate values for the 3.7, 3.6, and 3.5 branches, I'll remove the needs backport to 3.7 and needs backport to 3.6 labels so that @miss-islington doesn't open the backport PRs automatically.

If this PR is accepted and merged, I'll use cherry-picker to manually open the backport PRs for 3.7, 3.6, and 3.5.

Regarding the "What's New" entries, I would prefer to do those separately to simplify the merge conflicts. I'll have definitely time to take care of this on Monday, but for today and tomorrow I'll likely be busy with finishing up my final exams (last week of the Fall semester).

I should have time to make very small changes or reply to feedback, but I will likely not have time to do anything substantial until Monday (Dec 9th), unless I'm able to finish my work early on Sunday.

Edit: If any larger changes are needed before Monday, feel free to open a PR to my branch at https://github.com/aeros/cpython/tree/bpo37228-create_datagram_endpoint-reuse_address and I should be able to merge it. I've also ticked allow edits from maintainers, which allows minor changes to the contents of the PR through the GitHub UI.

gvanrossum

LGTM

bedevere-bot · 2019-12-09T14:21:14Z

@ambv: Please replace # with GH- in the commit message next time. Thanks!

miss-islington · 2019-12-09T14:21:14Z

Thanks @aeros for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

pythonGH-17311) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

bedevere-bot · 2019-12-09T14:21:25Z

GH-17529 is a backport of this pull request to the 3.8 branch.

GH-17311) (#17529) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

ambv · 2019-12-09T14:43:45Z

Note: merged for 3.8 and 3.9. @aeros removed backport labels to 3.6 and 3.7 here. The issue on BPO still lists those versions as affected. Please fix either way.

aeros · 2019-12-10T06:00:45Z

@ambv

@aeros removed backport labels to 3.6 and 3.7 here. The issue on BPO still lists those versions as affected. Please fix either way.

This is because they need to be manually backported, due to a difference in the versionchanged note. I'll take care of this process and attach the backport PRs to the bpo issue.

pythonGH-17311) (pythonGH-17529) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

GH-17311) (GH-17570) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

python#17311) (cherry picked from commit ab513a3)

…USEADDR (pythonGH-17311). (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

…USEADDR (GH-17311). (GH-17571) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

python#17311)

* refs python/cpython#17311

aeros requested review from 1st1 and asvetlov as code owners November 21, 2019 03:00

the-knights-who-say-ni added the CLA signed label Nov 21, 2019

bedevere-bot added the awaiting review label Nov 21, 2019

aeros added needs backport to 3.6 needs backport to 3.8 only security fixes skip news type-security A security issue topic-asyncio labels Nov 21, 2019

1st1 requested changes Nov 21, 2019

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Nov 21, 2019

aeros force-pushed the bpo37228-create_datagram_endpoint-reuse_address branch from aae05e6 to e3c895c Compare November 21, 2019 06:22

aeros changed the title ~~bpo-37228: Fix loop.create_datagram_endpoint() reuse_address~~ bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR Nov 21, 2019

aeros force-pushed the bpo37228-create_datagram_endpoint-reuse_address branch from e3c895c to 397c9be Compare November 21, 2019 07:59

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR

d748ca1

aeros force-pushed the bpo37228-create_datagram_endpoint-reuse_address branch from 397c9be to d748ca1 Compare November 21, 2019 09:45

Formatting fix

69faa22

aeros commented Nov 21, 2019

View reviewed changes

Lib/asyncio/base_events.py Show resolved Hide resolved

bedevere-bot removed the awaiting changes label Nov 21, 2019

bedevere-bot added the awaiting change review label Nov 21, 2019

bedevere-bot requested a review from 1st1 November 21, 2019 10:30

aeros requested a review from 1st1 December 3, 2019 03:06

Modify versionchanged to 3.8.1

dc521c5

gvanrossum approved these changes Dec 8, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Dec 8, 2019

aeros removed needs backport to 3.6 labels Dec 8, 2019

ambv merged commit ab513a3 into python:master Dec 9, 2019

bedevere-bot removed the awaiting merge label Dec 9, 2019

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 9, 2019

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR (

d7af7ea

pythonGH-17311) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

bedevere-bot removed the needs backport to 3.8 only security fixes label Dec 9, 2019

ambv pushed a commit that referenced this pull request Dec 9, 2019

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR (

79c2974

GH-17311) (#17529) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

ned-deily added a commit that referenced this pull request Dec 11, 2019

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR (

95157c6

GH-17311) (GH-17570) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

aeros added a commit to aeros/cpython that referenced this pull request Dec 11, 2019

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR (

e47edfc

python#17311) (cherry picked from commit ab513a3)

aeros added a commit to aeros/cpython that referenced this pull request Dec 11, 2019

[3.6] bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_RE…

f963ca5

…USEADDR (pythonGH-17311). (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

ned-deily pushed a commit that referenced this pull request Dec 11, 2019

[3.6] bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_RE…

b23c084

…USEADDR (GH-17311). (GH-17571) (cherry picked from commit ab513a3) Co-authored-by: Kyle Stanley <aeros167@gmail.com>

aeros mentioned this pull request Dec 12, 2019

bpo-37228: Fix warnings in test_asyncio.test_base_events #17577

Merged

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR (

cd1c94b

python#17311)

fantix added a commit to fantix/uvloop that referenced this pull request May 9, 2020

apply bpo-37228

9005b8b

* refs python/cpython#17311

fantix mentioned this pull request May 9, 2020

apply bpo-37228 MagicStack/uvloop#343

Merged

fantix added a commit to fantix/uvloop that referenced this pull request May 9, 2020

apply bpo-37228

19806c7

* refs python/cpython#17311

fantix added a commit to MagicStack/uvloop that referenced this pull request May 13, 2020

apply bpo-37228

506a2aa

* refs python/cpython#17311

Olaf1022 added a commit to Olaf1022/Ultra-fast-asyncio that referenced this pull request Aug 13, 2023

apply bpo-37228

67cb48e

* refs python/cpython#17311

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR #17311

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR #17311

aeros commented Nov 21, 2019 •

edited

Loading

1st1 left a comment

bedevere-bot commented Nov 21, 2019

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Nov 21, 2019 •

edited

Loading

aeros Nov 21, 2019

ned-deily Dec 7, 2019

aeros Dec 7, 2019

aeros commented Nov 21, 2019

bedevere-bot commented Nov 21, 2019

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Dec 8, 2019 •

edited

Loading

gvanrossum left a comment

bedevere-bot commented Dec 9, 2019

miss-islington commented Dec 9, 2019

bedevere-bot commented Dec 9, 2019

ambv commented Dec 9, 2019

aeros commented Dec 10, 2019

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR #17311

bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR #17311

Conversation

aeros commented Nov 21, 2019 • edited Loading

1st1 left a comment

Choose a reason for hiding this comment

bedevere-bot commented Nov 21, 2019

aeros commented Nov 21, 2019 • edited Loading

aeros commented Nov 21, 2019 • edited Loading

aeros Nov 21, 2019

Choose a reason for hiding this comment

ned-deily Dec 7, 2019

Choose a reason for hiding this comment

aeros Dec 7, 2019

Choose a reason for hiding this comment

aeros commented Nov 21, 2019

bedevere-bot commented Nov 21, 2019

aeros commented Nov 21, 2019 • edited Loading

aeros commented Dec 8, 2019 • edited Loading

gvanrossum left a comment

Choose a reason for hiding this comment

bedevere-bot commented Dec 9, 2019

miss-islington commented Dec 9, 2019

bedevere-bot commented Dec 9, 2019

ambv commented Dec 9, 2019

aeros commented Dec 10, 2019

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Nov 21, 2019 •

edited

Loading

aeros commented Dec 8, 2019 •

edited

Loading