Fix new Uri() for relative URLs with param colon

[distantnative]

Description

With this change an existing unit test for Environment fails. I think the test has been wrong:

detectRequestUri() receives index.php?foo=bar as input and expects

'path'  => '',
'query' => 'foo=bar'

as result. However, before this PR detectRequestUri would concat the string with https://getkirby.com without trailing slash.

So it would actually examine https://getkirby.comindex.php?foo=bar for the test in question. Which seems wrong. When actually running it on https://getkirby.com/index.php?foo=bar as with this PR now, the result is 'path' => 'index.php',.

@lukasbestle @bastianallgeier Is this unexpected/a problem if we change this and adapt the test?

Changelog

Fixes

• Fixed new Http\Uri() for relative URLs with a colon inside
  can't parse_url() for creating URIs that use pagination logic #6331

Ready?

• In-code documentation (wherever needed)
• Unit tests for fixed bug/feature
• Tests and CI checks all pass

For review team

• Add changes & docs to release notes draft in Notion

[bastianallgeier]

The URI class used to remove index.php from the path. I think it was mostly about making sure that https://domain.com/something and https://domain.com/something/index.php lead to the same thing. I remember that it was important for some reason further down under the hood. I suppose it was about auto URL detection. But I'm not entirely sure anymore. The concatenated path without slash is definitely wrong though.

[distantnative]

@bastianallgeier I cannot reproduce that on develop-minor, index.php was already present there:

$uri = new Uri('index.php?foo=bar');
$uri->toString(); // /index.php?foo=bar
$uri->path()->toString(); // index.php

$uri = new Uri('https://getkirby.com/index.php?foo=bar');
$uri->toString(); // https://getkirby.com/index.php?foo=bar
$uri->path()->toString(); // index.php

So what you describe doesn't seem to hold true for v4.3 at least.

This PR only exposes the bug from Environment::detectRequestUri() resulting from the wrongly concatenated path. The question is whether the Environment somehow depends on the false result or if changing it is just fine and only exposes the false negative unit test now, which we could simply change.

[bastianallgeier]

Ok, I think my outdated assumptions are a good thing. I would vote to fix the environment unit test in this case. I double-checked the results and it is indeed more consistent and correct this way. If we really run into a regression because of it (which would surprise me) we can still consider what to do. But I guess it would be more useful to work around having index.php in the path in a different place, if that is what we want to achieve.