slimphp / Slim-Psr7

PSR-7 implementation for use with Slim 4
MIT License
133 stars 45 forks source link

Enable opt-out of URI scheme filtering #313

Closed dakujem closed 1 month ago

dakujem commented 2 months ago

addresses issue #282

This PR makes it possible to opt-out of URI scheme filtering. This may be used for generic scenarios where the schemes are not know and it is not desirable to limit them.

The filtering is opted out in the Uri class by inheriting it and overriding the Uri::SUPPORTED_SCHEMES constant, setting it to null:

class MyUri extends Uri {
    public const SUPPORTED_SCHEMES = null;
}

It is then possible to create URIs with any scheme:

new MyUri('whatever', 'example.com');
new MyUri('ldap', 'ldap.example.com');

To use the new class with UriFactory, one has to, again, inherit it, having two options:

class MyUriFactory extends UriFactory
{
    public const URI_CLASS = MyUri::class;
}

or

class MyUriFactory extends UriFactory
{
    protected function makeUriObject(
        string $scheme,
        string $host,
        ?int $port = null,
        string $path = '/',
        string $query = '',
        string $fragment = '',
        string $user = '',
        string $password = ''
    ): Uri {
        return new class($scheme, $host, $port, $path, $query, $fragment, $user, $password) extends Uri {
            public const SUPPORTED_SCHEMES = null;
        };
    }
}

Personally, I'm not happy with this inheritance-based approach, it seems to me that the filtering should be opt-in in and a function of the factory, not the Uri object. That would be breaking change. But even it the functionality was moved, the standard port omission would stop working in the Uri object.

I first tried using static variable, that could be manipulated globally, but that approach broke the existing inheritance due to restrictions in PHP.

dakujem commented 2 months ago

Eh, I'm not sure how to fix the PHPStan issue...

odan commented 1 month ago

Hi @dakujem

Thanks so much for your work on this! It’s been a while since we’ve seen any updates, so I’m going to close this PR for now just to keep things tidy. No worries though, if you’re able to pick it back up or have new changes, feel free to reopen it or start a new one.

dakujem commented 2 weeks ago

@odan I expected a helping hand with the phpstan issue I mentioned in my previous post (which i no longer see here though) so we could finish and close this PR.