-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-126601: pathname2url()
: handle NTFS alternate data streams
#126760
Conversation
Adjust `pathname2url()` to encode embedded colon characters in Windows paths, rather than bailing out with an `OSError`.
Nevermind - I found a nice middle-ground I think! |
LGTM with one trivial change (or a reason we can't make it). We should probably also |
Thanks Steve!
I'm intending to backport this to 3.13 and 3.12 because it seems like a straightforward bugfix to me. So I'm not sure if |
I haven't tracked down what functions are impacted, but my assumption would be that existing users may notice behaviour changes where the path is an arbitrary input. I'm most concerned about opening up a potential exploit, especially since alternate streams are not commonly used (and would rarely traverse a URL). Perhaps there's a scenario that's worth the risk to have the change, but I'm not aware of it, so I'm inclined to play it safe on a strict correctness issue that isn't actively hurting anyone. |
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Thanks again. I've added a
|
So the only question now is whether to backport, and all I can say is "I'm not sure" (and so default to saying no). Users running into this issue in real code would be pretty persuasive. |
I'm erring towards not backporting, on reflection. I don't think the current behaviour is great, but it's better to get an exception than an incorrect result. I only logged this issue because I spotted it in the code (and I've fixed similar issues in pathlib reported by Eryk Sun) - I'm not aware of any user running into it. |
Adjust
pathname2url()
to encode embedded colon characters in Windows paths, rather than bailing out with anOSError
.