net-http icon indicating copy to clipboard operation
net-http copied to clipboard

Use Net::HTTPHeaderSyntaxError instead of ArgumentError

Open kevindew opened this issue 3 years ago • 0 comments

This change proposes changing the exception class used when invalid syntax is used in a HTTP Header from ArgumentError to Net::HTTPHeaderSyntaxError.

The reason I am proposing this change is to make it easier for applications and libraries to catch errors that result as part of HTTP response.

For example, if you're calling: Net::HTTP.get("www.example.com", "/") it is confusing to receive an ArgumentError exception as this isn't a problem with the arguments provided to Net::HTTP#get and instead is a lower-level parsing concern of the work done by Net::HTTP#get.

This exception isn't caught by common libraries that wrap Net::HTTP, for example Faraday: https://github.com/lostisland/faraday-net_http/blob/616bd1c35c058da01f91af8c4f504141b4f2b427/lib/faraday/adapter/net_http.rb#L15-L31 and is a somewhat delicate error to catch when making a request as a developer likely wants to distinguish between a genuine ArgumentError to usage of Net::HTTP and what is an error in the syntax in the HTTP request/response.

There was existing precedence for the use of this exception in Net::HTTPHeader, so this seemed like a change that could made and make the class behave consistently.

An example of usage is below.

Before this change:

irb(main):001:0> require 'webmock'; include WebMock::API; WebMock.enable!; stub_request(:any, "www.example.com").to_return
(headers: { "Invalid" => "Item with \r character" })
irb(main):002:0> Net::HTTP.get("www.example.com", "/")
/Users/kevin.dew/dev/net-http/lib/net/http/header.rb:85:in `set_field': header field value cannot include CR/LF (ArgumentError)

After this change:


irb(main):001:0> require 'webmock'; include WebMock::API; WebMock.enable!; stub_request(:any, "www.example.com").to_return
(headers: { "Invalid" => "Item with \r character" })
irb(main):002:0> Net::HTTP.get("www.example.com", "/")
/Users/kevin.dew/dev/net-http/lib/net/http/header.rb:87:in `set_field': header field value cannot include CR/LF (Net::HTTPHeaderSyntaxError)    => "Item with \r character" })

kevindew avatar Sep 27 '22 19:09 kevindew