slimphp / Slim-Psr7

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

Construction of Uri via UriFactory fails for non-HTTP-related URIs #282

Open dakujem opened 1 year ago

dakujem commented 1 year ago

URIs are universal. I assumed that the implementation of Uri object would be too.

TLDR; The following snippet produces InvalidArgumentException: Uri scheme must be one of: "", "http", "https" exception because of scheme filtering in Uri::filterScheme.

use Slim\Psr7\Factory\UriFactory;
(new UriFactory())->createUri('smtp://smtp.dakujem.dev:25');

Reasoning

I tried using Slim/Psr7 for handling generic URIs that I'm using for DSN-style configuration variables.

There are plenty of valid schemes, so setting Uri::SUPPORTED_SCHEMES is impractical for this purpose.

Now, Guzzle handles this correctly, as does the native parse_url function.

use GuzzleHttp\Psr7\Uri;
use Slim\Psr7\Factory\UriFactory;

$uri = 'smtp://smtp.dakujem.dev:25';
// tel:+1-816-555-1212
// ftp://ftp.is.co.za/rfc/rfc1808.txt
// ldap://[2001:db8::7]/c=GB?objectClass?one
(string)(new Uri($uri));
// InvalidArgumentException: Uri scheme must be one of: "", "http", "https"
(string)(new UriFactory())->createUri($uri); // exception here

Suggested solution

I suggest using opt-in approach: keep Slim/Psr7 universal and activate the filter when booting Slim framework.

public const SUPPORTED_SCHEMES = null; // initialize the class constant with null

// within filterScheme method
        if (static::SUPPORTED_SCHEMES !== null && !key_exists($scheme, static::SUPPORTED_SCHEMES)) {
            throw new InvalidArgumentException( ... );
        }

 // and somewhere in Slim bootstrapping phase
 Uri::SUPPORTED_SCHEMES = ['', 'http', 'https']; 

I believe you get the idea.

This might be related to other parts of the Uri object, in particular the other filter* methods.

akrabat commented 5 months ago

I believe that this is solved by https://github.com/slimphp/Slim-Psr7/pull/190

dakujem commented 5 months ago

Yeah... an acceptable compromise.

The opt-in approach would have been better though. This still forces users to enumerate all of the schemas beforehand, which is not usable in generic scenarios like the one I describe above.

akrabat commented 5 months ago

Good point. Reopening as we can do better.

dakujem commented 5 months ago

Would you accept a PR or is someone else going to work on this? I can't promise to meet any deadline though :-)

akrabat commented 5 months ago

Would definitely accept a PR, but can't promise how quickly I can look at it :)