Add handling for invalid responses to Response.from_io

[straight-shoota]

Partly fixes #5621

[asterite]

Careful with making HTTP more strict than it should.

With this, does HTTP::Client.get("https://neuralegionsite.wixsite.com/siteBackHtml/\n\n5") works? Because curl that URL does work.

Servers are not well implemented and browsers still accept them. The http server should tolerate these small errors and act accordingly without failing fast.

[straight-shoota]

HTTP::Client.get("https://neuralegionsite.wixsite.com/siteBackHtml/\n\n5") doesn't work but that's because of the malformed URL. This needs another fix.

This PR just adds some sanity checks to make sure the io actually contains an HTTP response. This is very basic.
It should fail if the first line is not an HTTP status line or the HTTP version is not supported (can't handle something you don't even know about). Obviously, a status code must be a valid integer (otherwise to_i raises an ArgumentError). One could argue about restricting the range of the status code but I don't see any reason not to.

While HTTP:Client is solely an HTTP client, curl supports a number of protocols and applies some quirks if it encounters an error. If an HTTP request receives an invalid response, curl falls back to interpreting the response's body directly (thus eliminating the HTTP protocol).

I dont think a dedicated HTTP client API should have such quirky behaviour. At least not as default.
Browsers usually don't accept an invalid HTTP response either. Chrome and Edge fail with an appropriate error and Firefox displays the response as raw text.

[bararchy]

@straight-shoota This looks great and will benefit me greatly when merged.
@sdogruyol @RX14 @asterite can one of you approve for merge?

[asterite]

Before merging this, I'd like someone to test go's http client and see if it applies the same quirky things as curl. I'm pretty sure it does, and that this PR should not be merged. The internet is a jungle and not all servers are well implemented and http client must be super tolerant and flexible about this.

[straight-shoota]

@asterite I haven't actually verified everything through running code, but from reading Go
s ReadResponse function, it validates the first line read as a HTTP response:

• it consists of protocol and status, separated by one ore more spaces.
• status contains a status code and optional message, separated by space
• status code is a three digit integer.
• protocol is in the format HTTP/\d+\.\d+ (just using regex grammar for illustration).

This PR ends up doing the same except enforcing protocol to be HTTP/0.9, HTTP/1.0 or HTTP/1.1. In my opinion this makes sense, because that are the only HTTP protocol versions currently supported by Crystal's HTTP implementation.

[ysbaddaden]

Just for fun: we don't support HTTP 0.9! There are no headers, no status line in responses (only the body), no keep-alive, and it's not even supposed to support binary contents.

• requests are "a line of ASCII characters terminated by a CR LF (...) pair. This request consists of the word 'GET', a space, the document address."
• responses are "a message in hypertext mark-up language (HTML)."

See https://www.w3.org/Protocols/HTTP/AsImplemented.html

[asterite]

So I guess we can merge this, except dropping support for HTTP 0.9 (or just checking that it's a digit-dot-digit).

[straight-shoota]

@ysbaddaden Hm, I don't know why I was under the impression it was supported, but clearly it is not. It should be removed from supported versions then.

[straight-shoota]

I removed HTTP/0.9 from supported versions.