From a60a1bbb3e877d76c8b93217b5cd3d6cf7a1c8ba Mon Sep 17 00:00:00 2001 From: Anton Date: Sun, 15 Dec 2019 15:34:13 +0100 Subject: [PATCH] Duplicate options Hash in .timeout call 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. --- lib/http/chainable.rb | 2 +- spec/lib/http_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb index 582e65cc..70a32bd4 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -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)`." diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 5e188e2b..56b228d6 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -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 }