Skip to content

Conversation

@awoimbee
Copy link
Contributor

@awoimbee awoimbee commented Jun 4, 2025

https://seclists.org/fulldisclosure/2025/Jun/2

Honestly I have no idea why this lib used netloc + manual parsing instead of hostname as I can see references to hostname as early as from the python 2.6 docs.

Copy link
Contributor

@danigm danigm left a comment

Choose a reason for hiding this comment

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

About the test, I created another test for the same issue in a different PR. So at this point, the issue is fixed and it could be nice to have a test for this case, I'm okay to close my PR if this test is good enough, or keep it if maintainers are okay to have two tests for this case.


try:
_netrc = netrc(netrc_path).authenticators(host)
_netrc = netrc(netrc_path).authenticators(ri.hostname)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fixed now in main: 96ba401

@sigmavirus24
Copy link
Contributor

This is another way to test this that is less end to end but I'm open to accepting it here as well

@awoimbee
Copy link
Contributor Author

awoimbee commented Jun 5, 2025

Well OK, there's not much left after merging main.
Keeping this open since it contains an actual unit test and removes legacy cruft but feel free to close, I don't care

@sigmavirus24 sigmavirus24 merged commit 5b4b64c into psf:main Jun 5, 2025
29 checks passed
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.

3 participants