Revert "Fix parse_url(): can not recognize port without scheme" #9569
Conversation
This reverts commit 72d8370.
|
@Girgias according to #9545 (comment) it just needs revert |
There was a problem hiding this comment.
Indeed, I don't see a way to support both scenarios (at least not without possibly breaking some other scenario). As such, we should revert, since #9545 is about a BC break, while #7844 has not been released as GA version.
We should have a test for what we broke in #9545, i.e. add the following cases to ext/standard/tests/strings/url_t.phpt:
'internal:#feeding', // used by drupal
'magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C', // used by drupal
@PandaLIU-1111, sorry for that.
Note to self: I shall not attempt to fix parse_url() – I shall not attempt to fix parse_url() – I shall not attempt to fix parse_url() – I shall not attempt to fix parse_url() – I shall not attempt to fix parse_url().
| @@ -12,19 +12,19 @@ var_dump(parse_url('127.0.0.1:9999#')); | |||
| *** Testing parse_url() :can not recognize port without scheme *** | |||
There was a problem hiding this comment.
I don't see the point in keeping this test. It has been introduced to show and verify that the other issue has been fixed; since we're going to revert, we should drop the test as well.
There was a problem hiding this comment.
🤷♂️ Since in the original change no other test was touched that's a pretty clear indication that this behavior is untested. Keeping this would make it more obvious if it ever changes again. But I'm fine either way.
There was a problem hiding this comment.
Well, it shouldn't hurt to keep it, and it may help for future developement.
There was a problem hiding this comment.
I think it's ok to keep the comment as it documents current behavior
| @@ -12,19 +12,19 @@ var_dump(parse_url('127.0.0.1:9999#')); | |||
| *** Testing parse_url() :can not recognize port without scheme *** | |||
There was a problem hiding this comment.
Well, it shouldn't hurt to keep it, and it may help for future developement.
This reverts commit 72d8370.
Closes #9545