This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author gregory.p.smith
Recipients Mike.Lissner, apollo13, gregory.p.smith, lukasz.langa, mgorny, miss-islington, orsenthil, sethmlarson, xtreak
Date 2021-05-05.18:13:45
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1620238425.79.0.641673296651.issue43882@roundup.psfhosted.org>
In-reply-to
Content
Thanks Florian!  Indeed, I'm glad you have tests for this.  (I expect anyone writing their own validation code will have such tests)

Making urlsplit raise an exception where it never has before has other consequences:

In CPython's own test suite test_urllib fails as many of its tests for validation that these characters are either ignored or cause a specific http.client.InvalidURL error on urlopen() start failing.  I draw no conclusions from that other than we'd need to rework some of those tests.  It just reflects the state of our test suite and even inconsistent behavior between excluding the characters or erroring within the http.client code on them based on past CVEs.

Regardless, if people would people prefer to see urlsplit `raise SomeExistingException(f'Invalid characters in URL or scheme. url={url!r} scheme={scheme!r}')` in 3.9.6 and the security patch releases for other 3.x versions, evidence that it wouldn't cause alternate problems would be helpful.

I've kicked off tests at work on our huge codebase with both variants as a datapoint to see if that is informative or not.

If we went the exception route: SomeExistingException might make sense as `http.client.InvalidURL`, but that'd be a circular dependency (no big deal) and heavyweight import for urllib.parse to have.  `urllib.error.URLError` could also make sense, but that's an OSError descendant and identifies itself as a urlopen error which would be surprising.  `ValueError` is a reasonable fallback.  But using that guarantees someone will wonder why it isn't one of the other two...  As this is a bugfix, defining a new exception isn't an option.

We as a community currently lack a way for security patches to CPython to be tested against a broad swath of projects in advance of them appearing in a release.  Once upon a time there were release candidates for patches releases that could serve this purpose...
History
Date User Action Args
2021-05-05 18:13:45gregory.p.smithsetrecipients: + gregory.p.smith, orsenthil, lukasz.langa, mgorny, apollo13, Mike.Lissner, miss-islington, xtreak, sethmlarson
2021-05-05 18:13:45gregory.p.smithsetmessageid: <1620238425.79.0.641673296651.issue43882@roundup.psfhosted.org>
2021-05-05 18:13:45gregory.p.smithlinkissue43882 messages
2021-05-05 18:13:45gregory.p.smithcreate