CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
401 stars 29 forks source link

HTTP1 parsing should not crash on invalid content_length header #129

Closed CrowdHailer closed 5 years ago

CrowdHailer commented 5 years ago

This applies to both parse_request and parse_response

Desired behaviour

{:error, :invalid_content_length} = HTTP1.parse_response("HTTP/1.1 200 OK\r\ncontent_length: nan\r\n\r\n")

current behaviour is to crash.

Suggestion

change this function to fetch_content_length that returns {:ok, nil | non_neg_integer} | {:error, :invalid_content_length https://github.com/CrowdHailer/raxx/blob/master/lib/raxx/http1.ex#L743-L751

NOTE. this should remain a private function. If get_content_length is added to Raxx module that should still only return nil | integer because the assumption is a Request or Response is always valid.

Checklist

Further The parser should never error, content received over the net can be anything. When this issue is closed a follow up issue can be to check the rest of the parser for failure cases.

https://tools.ietf.org/html/rfc7230#section-3.3.3 explains the details of parsing. Things that should be checked are sending case insensitive transfer encodings. multiple content_length headers

CrowdHailer commented 5 years ago

I have not updated ace because it should be handled by the standardised error case #131