Skip to content

Conversation

sethmlarson
Copy link
Contributor

@sethmlarson sethmlarson commented Jan 28, 2025

Opening this pull request in favor of #111261, as the original author appears to not be responding to reviews. This moves the error to occur during parsing instead of on the ParseResult properties.

cc @gpshead @orsenthil @pschoen-itsc as reviewers on the previous PR.

self.assertRaises(ValueError, urllib.parse.urlsplit, 'scheme://prefix.[::1]?')
self.assertRaises(ValueError, urllib.parse.urlsplit, 'scheme://[::1].suffix?')
self.assertRaises(ValueError, urllib.parse.urlsplit, 'scheme://user@prefix.[v6a.ip]')
self.assertRaises(ValueError, urllib.parse.urlsplit, 'scheme://user@[v6a.ip].suffix')
Copy link
Member

Choose a reason for hiding this comment

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

It might be helpful to add tests only with [, and tests only with ]. Such URL is also invalid, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll add tests with that structure too! Thanks :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in ebf92bb

@sethmlarson sethmlarson requested a review from vstinner January 30, 2025 17:48
Copy link
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

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

LGTM.

Thank you!

@sethmlarson
Copy link
Contributor Author

The failure in Windows free-threading looks unrelated, I can't merge without all the checks passing so if someone else can that would be appreciated 💜

@orsenthil orsenthil enabled auto-merge (squash) January 31, 2025 16:50
@orsenthil orsenthil disabled auto-merge January 31, 2025 16:50
@orsenthil
Copy link
Member

Looks like that's a required to succeed.

@orsenthil orsenthil merged commit d89a5f6 into python:main Jan 31, 2025
43 checks passed
@miss-islington-app
Copy link

Thanks @sethmlarson for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10, 3.11, 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 31, 2025
…es for parsed URLs (pythonGH-129418)

* pythongh-105704: Disallow square brackets ( and ) in domain names for parsed URLs

* Use Sphinx references

Co-authored-by: Peter Bierma <[email protected]>

* Add mismatched bracket test cases, fix news format

* Add more test coverage for ports

---------

(cherry picked from commit d89a5f6)

Co-authored-by: Seth Michael Larson <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jan 31, 2025

GH-129526 is a backport of this pull request to the 3.13 branch.

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Feb 7, 2025
…es for parsed URLs (python#129418)

* pythongh-105704: Disallow square brackets ( and ) in domain names for parsed URLs

* Use Sphinx references

Co-authored-by: Peter Bierma <[email protected]>

* Add mismatched bracket test cases, fix news format

* Add more test coverage for ports

---------

Co-authored-by: Peter Bierma <[email protected]>
ambv pushed a commit that referenced this pull request Feb 19, 2025
…mes for parsed URLs (GH-129418) (#129528)

Co-authored-by: Seth Michael Larson <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
ambv added a commit that referenced this pull request Feb 19, 2025
…mes for parsed URLs (GH-129418) (#129529)

(cherry picked from commit d89a5f6)

Co-authored-by: Seth Michael Larson <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
ambv added a commit that referenced this pull request Feb 19, 2025
…es for parsed URLs (GH-129418) (#129530)

(cherry picked from commit d89a5f6)

Co-authored-by: Seth Michael Larson <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
gentoo-bot pushed a commit to gentoo/cpython that referenced this pull request Apr 9, 2025
…in names for parsed URLs (pythonGH-129418) (python#129530)

(cherry picked from commit d89a5f6)

Co-authored-by: Seth Michael Larson <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-security A security issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants