#89 StringURI now handles validation of valid URIs which aren't URLs

[garethellis36]

At the moment this won't pass build, the first commit only contains failing tests. I copied the list of valid and invalid URIs from the league/uri package's tests. See: https://github.com/thephpleague/uri/blob/master/tests/UriStringTest.php

[garethellis36]

Second commit adds league/uri package and uses that to validate the value instead of filter_var()

[scaytrase]

Are all of these allowed according to OAS3 spec ?

[garethellis36]

Are all of these allowed according to OAS3 spec ?

According to the Json Schema definition, when something is type string and format uri, it means A universal resource identifier (URI), according to RFC3986.

league/uri states that the package is RFC3986-compliant. So all of the valid URIs from its test suite should be valid in the context of an OpenAPI spec as well.

[scaytrase]

LGTM 👍

[garethellis36]

LGTM 👍

Are you happy to add a dependency to the other package at that specific version? Won't it cause problems for someone who already has a dependency on (for example) league/uri: "6.2"?

I'm not really sure how libraries generally address this sort of thing. It looks like 6.0.0 has the same API, so it should be OK to change the dependency to 6.*. But you couldn't have your library and also install league/uri at version 5, for example (the API is different there).

As an alternative, I could adapt the code from the URI library and re-implement it in this library without the additional dependency. The obvious downside is that there is then more code for you to maintain, and it would be harder to keep up with upstream changes if it emerged that there was an issue with the parser in the URI lib.

[scaytrase]

As an alternative, I could adapt the code from the URI library and re-implement it in this library without the additional dependency. The obvious downside is that there is then more code for you to maintain, and it would be harder to keep up with upstream changes if it emerged that there was an issue with the parser in the URI lib.

This is not how libraries should be used for sure. No one wants to maitain two libs with same code instead of one

[garethellis36]

Hi @scaytrase, what else needs to happen before this PR can be merged? Thanks for your support :)

[scaytrase]

Oh, sorry, thanks for reminding. I'll check it asap