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

Convert `self[key]` to a String with `#<<` on `#add_parsed` #1459

Closed yykamei closed 1 year ago

yykamei commented 1 year ago

Description

I want to handle this problem.

As I said in #1456, an instance of Faraday::Utils::Headers may have non-string values. So, an error might happen when the instance calls #parse with such existing values. For example, this code will raise an error before this PR is merged.

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>'

Todos

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

yykamei commented 1 year ago

1456 might be irrelevant. I just found a TypeError when dealing with #1456.

Yes, you're right. This case doesn't happen in usual use cases. However, Headers#[]= could be called anywhere (e.g., the setup code for testing), so I don't think it's always safe to call #parse without type conversion.

It's OK to close this PR if we don't have to consider the case I found 😇

yykamei commented 1 year ago

Thank you!