Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.15k stars 75 forks source link

Empty host value not allowed #249

Closed georgeboot closed 4 months ago

georgeboot commented 1 year ago

We experienced a weird error where our load balancers does a health request, and they always failed. It turns out, that the load balancer sends a HTTP request without any heather – so notably also without Host header.

Our Sentry SDK runs a middleware that tries to cache a PSR7 request for debugging reasons, and that uses this package.

Creating a PSR7 request fails because an InvalidArgumentException is thrown: Unable to parse URI: "http://:8000/health?source=lb"

As far as I know, a host header SHOULD be including in modern versions of HTTP, but HTTP1.0 probably doesn't require them.

Should I submit a PR to allow empty host values?

Zegnat commented 4 months ago

This library does not put together a URI, it is up to the caller of the PSR-17 factory to handle the URL creation. We do not read the Host header to create the URI, so whether there is a Host header or not has no effect on the creation of a ServerRequest object.

In this case it looks like the caller failed to create the URI. I am unsure if Sentry is using their own logic? But it does look like they closed the error on their end. So I will close this issue here.

If you still think there is an error in Nyholm/psr7, feel free to show a test case that recreates this issue!