laminas / laminas-diactoros

PSR HTTP Message implementations
https://docs.laminas.dev/laminas-diactoros/
BSD 3-Clause "New" or "Revised" License
487 stars 63 forks source link

[RFC]: Remove `scheme` filtering #179

Closed boesing closed 11 months ago

boesing commented 1 year ago

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

Ocramius commented 1 year ago

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.

weierophinney commented 11 months ago

My takes:

boesing commented 11 months ago

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:

Thanks for the feedback here and in Slack.