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

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

Open
dakujem opened this issue Mar 10, 2023 · 5 comments
Open

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

dakujem opened this issue Mar 10, 2023 · 5 comments

Comments

@dakujem
Copy link
Contributor

dakujem commented Mar 10, 2023

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.

@dakujem dakujem changed the title Construction of Uri via UriFactory fails for non-HTTP related URIs Construction of Uri via UriFactory fails for non-HTTP-related URIs Mar 10, 2023
@akrabat
Copy link
Member

akrabat commented Jun 8, 2024

I believe that this is solved by #190

@akrabat akrabat closed this as completed Jun 8, 2024
@dakujem
Copy link
Contributor Author

dakujem commented Jun 10, 2024

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
Copy link
Member

akrabat commented Jun 10, 2024

Good point. Reopening as we can do better.

@akrabat akrabat reopened this Jun 10, 2024
@dakujem
Copy link
Contributor Author

dakujem commented Jun 10, 2024

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

@akrabat
Copy link
Member

akrabat commented Jun 10, 2024

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

dakujem added a commit to dakujem/Slim-Psr7 that referenced this issue Aug 21, 2024
dakujem added a commit to dakujem/Slim-Psr7 that referenced this issue Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants