[RFC]: Remove `scheme` filtering
boesing opened this issue · 3 comments
RFC
Q | A |
---|---|
Proposed Version(s) | 4.0.0 |
BC Break? | Yes |
Goal
Remove the scheme filtering which prevents creating non-HTTP URIs.
Background
Diactoros is providing a PSR-7 UriInterface
implementation but Uri
is not tied to HTTP
only.
In mezzio/mezzio-cors
, we do make use of the UriInterface
for Origin
.
We do use the CORS implementation in our project which seems to be requested from within whatever chrome extension.
When that chrome extension does execute requests, we receive the following Origin:
Origin: chrome-extension://knohfebhibeknbfioecpdmdkjkjdnjnl
When parsing that origin header and after passing to UriFactoryInterface#createUri
, an exception is thrown:
Laminas\Diactoros\Exception\InvalidArgumentException: Unsupported scheme "chrome-extension"; must be any empty string or in the set (http, https)
Considerations
I do not really see a huge benefit in having this kind of restriction unless the class is named HttpUri
.
I am aware that extending the Uri
might be a solution (as I could extend allowedSchemes
), but since we do add final
to a bunch of components lately, at some point, the Uri
of diactoros will receive that as well.
Due to the fact that the CORS component does not use diactoros directly, I am unsure if that issue should be handled by upstream users.
However, I've checked nyholm/psr7
and guzzlehttp/psr7
and both are "fine" with having chrome-extension
(or non-HTTP related schemes) in place. For the sake of interoperability, I'd love to get rid of throwing exceptions when Uris are created with non-HTTP schemes.
Proposal(s)
Just remove the Uri#filterScheme
logic where we throw an exception when the scheme is not part of the allowedSchemes
map. We should just verify that the scheme is valid as per RFC definition.
Appendix
One important consideration: PSR-7 packages are about abstracting HTTP messaging, so that needs to be considered upfront.
I'm fine with the proposal, just throwing in this data point.
My takes:
- PSR-7 very explicitly states: "UriInterface models HTTP and HTTPS URIs as specified in RFC 3986 (the primary use case)". When developing PSR-7, there was some discussion about modeling any URI, and we chose NOT to do this, as there are a number of URIs that are formed quite differently; many would NOT work in an HTTP context. The consensus was that something targeting all of RFC 3986 would need a separate PSR, and that most likely if we abstracted only HTTP/HTTPS URIs, it could likely later extend such a PSR and still retain compatibility with PSR-7.
- Technically, if other PSR-7 implementations are supporting anything other than HTTP/HTTPS in their
UriInterface
implementations, they are deviating. - All that said, I have no problem with changing this, but we need to be careful to note that our implementation does NOT cover all of RFC 3986, and that it's potentially possible to obtain and/or construct a URI that cannot be used within your application.
After some more thoughts (including the knowledge I've gathered from Marcos and Matthews comments), I decided to tackle the underlying issue in mezzio-cors
.
To summarize:
- diactoros does follow PSR-7 specs
- diactoros does not allow more than HTTP/HTTPS and is not required to do so (as per PSR-7 spec)
- PSR-7 spec allows other schemes than HTTP/HTTPS but also states that implementation MAY NOT implement that
- PSR-7 spec actually states that
RequestInterface#withScheme
may throwInvalidArgumentException
for unsupported schemes
Thanks for the feedback here and in Slack.