-
Notifications
You must be signed in to change notification settings - Fork 90
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
Chokes on URL with + sign #408
Comments
In GitLab by @maerwald on Aug 5, 2022, 09:56 Did you try with url encoding |
In GitLab by @maerwald on Aug 5, 2022, 10:01 This works for me. So it's not really a bug. We just expect properly url encoded urls. Accepting these types of URLs is an enhancement. |
In GitLab by @ulysses4ever on Aug 5, 2022, 10:09 Good point, thank you. I think silently replacing + with 'Reference to deleted milestone 20' is a bug though. Correct behavior would be to abort and signal about unexpected symbol in the URL. |
In GitLab by @maerwald on Aug 5, 2022, 10:49 I'm on company network, so can't do much investigation now. Check:
So when we read from optparse, we use When we pass to curl, we do uri-bytestring docs: |
In GitLab by @maerwald on Aug 5, 2022, 11:00 |
In GitLab by @maerwald on Aug 5, 2022, 11:03 This seems to be correct behavior: https://www.w3schools.com/tags/ref_urlencode.asp
|
In GitLab by @maerwald on Aug 5, 2022, 11:15 |
In GitLab by @ulysses4ever on Aug 5, 2022, 11:24 Very interesting, thank you for the investigation. I guess, feel free to close as an upstream bug… |
I actually changed my mind and I believe my PR fixes this: Soostone/uri-bytestring#64 RFC 3986 does not specify that |
In GitLab by @ulysses4ever on Aug 5, 2022, 06:34
Doing
I get an error:
Notice how URL got changed: instead of
+
I now have a'Reference to deleted milestone 20'
.The text was updated successfully, but these errors were encountered: