Skip to content

🔗 Fix connecting to link-local IPv6 addresses#4556

Merged
asvetlov merged 5 commits intomasterfrom
ipv6ll
Oct 16, 2020
Merged

🔗 Fix connecting to link-local IPv6 addresses#4556
asvetlov merged 5 commits intomasterfrom
ipv6ll

Conversation

@socketpair
Copy link
Copy Markdown
Contributor

@socketpair socketpair commented Feb 5, 2020

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Fixes #4554

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

@socketpair socketpair requested a review from asvetlov as a code owner February 5, 2020 16:04
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Feb 5, 2020
@socketpair
Copy link
Copy Markdown
Contributor Author

socketpair commented Feb 6, 2020

@cooperlees I would recommend you to use IPv6 ULA addresses. I have painful experience with LL addresses. Especially in Avahi project. So, anyway, this fix should work. Please check.

@socketpair
Copy link
Copy Markdown
Contributor Author

@asvetlov

saghul/pycares#122 (comment)

aiodns uses pycares. Unfortunately, pycares is an ancient library with obsolete (and definitely buggy) code. I would drop using pycares anywhere. It's much better to move code with resolving using threads to aiodns. Would you like it ?

@webknjaz
Copy link
Copy Markdown
Member

webknjaz commented Feb 6, 2020

@socketpair mind filling out the PR template?

hosts.append({
'hostname': host,
'host': host,
'port': port,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe normalize the port here instead of the if-block?

Suggested change
'port': port,
'port': int(port),

@@ -0,0 +1 @@
Fix connecting to link-local IPv6 addresses.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any chance you could add a test for this?

@webknjaz webknjaz changed the title 🔗 Fix connecting to link-local IPv6 addresses (#4554) 🔗 Fix connecting to link-local IPv6 addresses Feb 6, 2020
@webknjaz
Copy link
Copy Markdown
Member

@socketpair bump

@wuisawesome
Copy link
Copy Markdown

wuisawesome commented Aug 24, 2020

Hey, is there still interest in getting this merged? I work on a downstream project (Ray) and we've had a lot of users come across this problem.

If there's interest in getting this merged, I think we'd be willing to fix up this PR.

(btw this is almost objectively no longer a rare issue, we've seen it on reproduced on popular ubuntu and redhat distros)

@cooperlees
Copy link
Copy Markdown

I’m happy to lend some cycles to polish this up to if it will be accepted.

@wuisawesome
Copy link
Copy Markdown

@webknjaz bump

Copy link
Copy Markdown
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

LGTM

@socketpair
Copy link
Copy Markdown
Contributor Author

wow

@asvetlov
Copy link
Copy Markdown
Member

I've finally found time for the project :)

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 16, 2020

Codecov Report

Merging #4556 into master will decrease coverage by 0.03%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4556      +/-   ##
==========================================
- Coverage   97.53%   97.50%   -0.04%     
==========================================
  Files          43       43              
  Lines        8979     8983       +4     
  Branches     1417     1418       +1     
==========================================
+ Hits         8758     8759       +1     
- Misses        101      103       +2     
- Partials      120      121       +1     
Flag Coverage Δ
#unit 97.50% <50.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiohttp/resolver.py 93.18% <50.00%> (-6.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2940484...6948941. Read the comment docs.

@github-actions
Copy link
Copy Markdown
Contributor

💚 Backport successful

The PR was backported to the following branches:

asvetlov pushed a commit that referenced this pull request Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GET Requests to link-local IPv6 addresses don't work on Python 3.7+

6 participants