Skip to content

Commit

Permalink
Duplicate options Hash in .timeout call
Browse files Browse the repository at this point in the history
When `Hash` is passed to HTTP.timeout, it's been mutated. So when you pass
a frozen `Hash` it raises an exception.

Here is an example of the code:
```
timeout_settings = { connect: 5, write: 2, read: 10 }.freeze
HTTP.timeout(timeout_settings).get('http://example.com')
```

In this change, I have added a call to .dup on options Hash to create a
copy and avoid mutation of original Hash.
  • Loading branch information
antonvolkoff authored and ixti committed Dec 16, 2019
1 parent 427525c commit 7ce201e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/http/chainable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def build_request(*args)
def timeout(options)
klass, options = case options
when Numeric then [HTTP::Timeout::Global, {:global => options}]
when Hash then [HTTP::Timeout::PerOperation, options]
when Hash then [HTTP::Timeout::PerOperation, options.dup]
when :null then [HTTP::Timeout::Null, {}]
else raise ArgumentError, "Use `.timeout(global_timeout_in_seconds)` or `.timeout(connect: x, write: y, read: z)`."

Expand Down
9 changes: 9 additions & 0 deletions spec/lib/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,15 @@
end
end

context "specifying per operation timeouts as frozen hash" do
let(:frozen_options) { {:read => 123}.freeze }
subject(:client) { HTTP.timeout(frozen_options) }

it "does not raise an error" do
expect { client }.not_to raise_error
end
end

context "specifying a global timeout" do
subject(:client) { HTTP.timeout 123 }

Expand Down

0 comments on commit 7ce201e

Please sign in to comment.