Additional validation for parse_url() for login/pass components

[iliaal]

Added validation to parse_url() to prohibit restricted characters inside login/pass components based on RFC3986

[jpauli]

This has been merged against master and will be part of PHP 7.1

[smalyshev]

@jpauli any reason why it wasn't merged in 5.6 and 7.0?

[jpauli]

Well, that broke tests, that needed tests change, so API break, so I did not commit that to stable branches

[smalyshev]

If it fixes RFC incompatibility, then I think it should be backported.

[jpauli]

Ok then I suggest you cherry-pick the needed commits if you want to

[nikic]

Multiple people have reported this as a regression here: https://bugs.php.net/bug.php?id=71822&edit=1

Given the ML discussion that has recently come up on implementing a compliant parse_url() alternative, I think it would be best to revert this change from all branches to avoid unnecessary BC breaks. Don't forget that historically parse_url() has always been obliging in parsing incomplete and invalid URLs, so starting to enforce additional checks at this point seems questionable to me.

/cc @weltling @dshafik

[smalyshev]

OK, let's revert it. But see https://bugs.php.net/bug.php?id=73192 then. I think it's the issue we need to address.

[nikic]

@smalyshev Hm, I see. I think we should do as the reporter suggests (if I understand it correctly) and make sure that host/password/etc are never recognized after the first ? or #, as these should always start query or fragment.

[nikic]

The problem is in

php-src/ext/standard/url.c

Line 222 in 6053987

if (!(p = memchr(s, '/', (ue - s)))) {

. e should be set to the first of /, ? or #, but the code will always set it to the / position if / is present at all.

[nikic]

Reverted in 1c468ee and bc3a0b8. Fixed the sec bug with b061fa9. This also caused some test fallout, but seems limited to completely broken URLs.