Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable opt-out of URI scheme filtering #313

Closed

Conversation

dakujem
Copy link
Contributor

@dakujem dakujem commented Aug 21, 2024

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:

  • either override the newly introduced UriFactory::makeUriObject method
  • or override the newly introduced class constant UriFactory::URI_CLASS
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 dakujem force-pushed the dakujem/uri-disable-scheme-filtering branch from 85d0e95 to 78c1694 Compare August 21, 2024 13:56
@dakujem dakujem marked this pull request as ready for review August 21, 2024 13:57
@dakujem dakujem changed the title Enable opt-out of URI shceme filtering Enable opt-out of URI scheme filtering Aug 21, 2024
@dakujem dakujem force-pushed the dakujem/uri-disable-scheme-filtering branch from 78c1694 to 469f4f9 Compare August 21, 2024 14:01
@dakujem
Copy link
Contributor Author

dakujem commented Aug 21, 2024

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

@odan
Copy link

odan commented Oct 10, 2024

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.

@odan odan closed this Oct 10, 2024
@dakujem
Copy link
Contributor Author

dakujem commented Oct 24, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants