[HttpFoundation] Fix request uri when it starts with double slashes

[alquerci]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#29478
License	MIT
Doc PR	~

When the REQUEST_URI starts with a slash no need to parse_url(). However to keep the same behaviour regarding the fragment we need to add a logic to remove it. While parse_url() handle all cases itself.

[alquerci]

All code review notes answered by a code modification and the branch has been rebased and squashed.

[danijelk]

Maybe extract the if( empty $requestUri string ) to jump over the if/else, unless you actually want to send an empty string into parse_url()? Just micro optimizations. I tested it with our setup and it worked well.

[alquerci]

@danijelk Thank you for your feedback.

On the majority of cases REQUEST_URI starts with a slash. The parse_url() usage is just to handle an edge case.

Your idea is good in order to skip null and empty strings. However, it is a bit overkill regarding the occurrence of this case while parse_url() return an empty path without an huge performance impact.

So to keep the code simple (as we cannot use here an early return) I think that this micro optimization adds useless complexity.

@nicolas-grekas Do you think that we can turn this PR as Reviewed status?

[nicolas-grekas]

Is there a way to keep existing tests the same?
When tests change too much it raises a red flag because it means a regression is possible.
Please reduce the diff to the strict minimum to ease the work of reviewers.

[alquerci]

Is there a way to keep existing tests the same?

Yes for sure. I can revert them.

@nicolas-grekas FYI: Tests has been refactored on this commit 95e9fe0 then squashed.

Its tests has been added on #29256.

cc @thomasbisignani

[Dylan-DutchAndBold]

Isn't this guy's solution where he is just using the parse_url function correctly, much more obvious and readable? a3cbb65

[nicolas-grekas]

@Dylan-DutchAndBold it's not: see how many tests this "solution" breaks.

[Dylan-DutchAndBold]

@Dylan-DutchAndBold it's not: see how many tests this "solution" breaks.

You're right, missed that, didn't get what you meant by your original comment at first.

[fabpot]

Thank you @alquerci.