laminas/laminas-diactoros

[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 throw InvalidArgumentException for unsupported schemes

Thanks for the feedback here and in Slack.