lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.72k stars 972 forks source link

Make `Headers#[]=` set a value as String #1456

Closed yykamei closed 1 year ago

yykamei commented 1 year ago

Description

Faraday::Utils::Headers#[]= has converted a passed value into a String separated with ", " if the value is an array. On the other hand, #[]= had stored the value as is if the value is not an array.

This is the previous behavior:

h = Faraday::Utils::Headers.new
h[:length] = 123
h[:length] # => 123

h[:length] = [123, 234]
h[:length] # => "123, 234"

I think this is a bit confusing, and it should consistently store a value as string.

With this patch, we will get the headers with all stored values as string.

h = Faraday::Utils::Headers.new
h[:length] = 123
h[:length] # => "123"

h[:length] = [123, 234]
h[:length] # => "123, 234"

It might be a breaking change, but I think it's worth discussing.

What do you think?

Todos

List any remaining work that needs to be done, i.e:

yykamei commented 1 year ago

Thank you for your review. Yes, you're absolutely right; this breaks existing code as you know the CIs fail. I gave up the effect of this change for all middlewares and adapters because of my lack of time.

I found out #add_parsed assumes the existing value is an instance of String, but it ends up raising an error like this, so I will update #add_parsed to use #to_s before calling #<<.

irb(main):001:0> h = Faraday::Utils::Headers.new
=> {}
irb(main):002:0> h[:foo] = 123
=> 123
irb(main):003:0> h.parse 'foo: 89'
/Users/yutaka.kamei/git/Fork/faraday/lib/faraday/utils/headers.rb:135:in `<<': no implicit conversion of String into Integer (TypeError)
    from /Users/yutaka.kamei/git/Fork/faraday/lib/faraday/utils/headers.rb:135:in `add_parsed'
    from /Users/yutaka.kamei/git/Fork/faraday/lib/faraday/utils/headers.rb:124:in `block in parse'
    from /Users/yutaka.kamei/git/Fork/faraday/lib/faraday/utils/headers.rb:124:in `each'
    from /Users/yutaka.kamei/git/Fork/faraday/lib/faraday/utils/headers.rb:124:in `parse'
    from (irb):3:in `<main>'
    from ./bin/console:15:in `<main>'

Now, I close this PR. Thanks for reviewing 👍