BUGFIX: Fix UriConstraints port constraints for default ports

[bwaidelich]

Previously, if UriConstraints were applied to an URL with a non-default
port (e.g. "8080") this port constraint was applied to the target URL even
if no explicit port constraint was set.

Fixes: #2714

[albe]

How does this relate to #2654 and #2276? Sorry, haven't looked into this PR details yet, but had those other coming up in my head that seemed related. I can take a look this evening unless you have some clear separation already up your sleeve :)

[bwaidelich]

@albe good question! They're related indeed, this one is complemental:

#2654 makes sure that

UriConstraints::create()
    ->withScheme('https')
    ->applyTo(new Uri('http://some-domain.tld'), false);

results in https://some-domain.tld (rather than https://some-domain.tld:80)

#2276 makes sure that

UriConstraints::create()
    ->withPort(8080)
    ->applyTo(new Uri('http://some-domain.tld:8080'), true);

results in http://some-domain.tld:8080 (rather than http://some-domain.tld)

and this PR makes sure that

UriConstraints::fromUri(new Uri('https://some-other-domain.tld'))
    ->applyTo(new Uri('https://some-domain.tld:8080'), false);

results in https://some-other-domain.tld (rather than https://some-other-domain.tld:8080).

This whole UriConstraints topic got rather confusing because there are much more cases than I had anticipated at first.
But I'm pretty sure that all three fixes make sense

[bwaidelich]

I don't understand why the Psalm test fails

ERROR: MissingFile - bin/psalm:112:9 - Cannot find file phpvfscomposer:/home/runner/work/flow-development-collection/flow-development-collection/flow-development-distribution/Packages/Libraries/vimeo/psalm/psalm to include (see https://psalm.dev/107)
include("phpvfscomposer://" . DIR . '/..'.'/Packages/Libraries/vimeo/psalm/psalm');

But I don't think that it's related to this change

[albe]

But I don't think that it's related to this change

Yes, unrelated. This error is around still some time now - see #2704 which tries to solve the issue in master after psalm has been disabled for now.

Thanks for the clear explanation! The first two make a lot of sense. The third one, which this PR targets is not so straightforward.

Given UriConstraints::fromUri(new Uri('https://some-other-domain.tld'))->applyTo(new Uri('https://some-domain.tld:8080'), false); I wouldn't even know intuitively what I'd expect 🤔 The (new) logic is then "If the constraint URI has a scheme, it's default port is applied unless an explicit port is set in the constraint URI, but explicit ports in the target URI are ignored". The latter feels weird. If someone has somehow configured the URI to be port 8080 on https, that must have good reasons, no? I'm likely just missing the actual use-case, because I never came across such a scenario with weird ports (lucky me?!)

[bwaidelich]

I wouldn't even know intuitively what I'd expect

UriConstraints::fromUri() means that all constraints from the given URI should be applied. For absolute URLs (with a scheme and host specified) that basically means that the whole URI should be replaced – this will be used in the reworked routing in order to create URLs pointing to external targets (i.e. for external ShortCut nodes in Neos).

I'm likely just missing the actual use-case, because I never came across such a scenario

The scenario in Neos 8.0 will be: Your development machine is configured to run on port 8080 but you don't want to include that port to all external shortcuts :)

[kdambekalns]

Sounds reasonable.

[albe]

Thanks for the explanation! :)