-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix: Colon in url path is not parsed by parse_url #12704
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
Conversation
Hey! So for your case would be Edit: sorry for previous confusion. |
Also: for bugs on the github bugtracker the filename should start with gh instead of bug. And the title "GH-..." instead of "Bug #..." |
Just to clarify, the |
Gah I mixed the two somehow in my answer, sorry, edited to be correct now. |
Should this target 8.1? |
@nicolas-grekas I don't know... I will happily re-do a PR to a lower branch if that's required... |
Related: #7890 (comment) Changes in the parser are risky, because people use them on invalid URLs. Thus, it's very hard to predict what side-effects a change has. Obviously we should prioritize parsing of valid URLs, but it might be better to be prudent and merge this for master only. |
Thanks for your comment @iluuu1994, I added an example for context (the case where query params are given) and the cases from the issue you referenced (which this PR solves 😃). |
Hello @bukka , I was wondering if they would be any follow up / merge of this PR or is it any thing else I shall do ? |
Closing following RFC: https://wiki.php.net/rfc/url_parsing_api |
This aims to solve #12703
Note: This is my first PR to php-src, could you clarify how the test file shall be named (temporarily named
bugX.phpt
) ?