Fix thread safety issue by avoiding mutation of proxy options hash

[QWYNG]

Hi! Thank you for maintaining this great gem.
This pull request includes both a bug report and a fix.

Description

Currently, the proxy option in Faraday mutates the provided hash by adding keys to it.
This causes issues in multi-threaded environments, potentially raising a RuntimeError.
Here is a minimal example to reproduce the issue (note: due to the nature of thread safety issues, this may not reproduce consistently):

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'faraday'
end

require 'faraday'

proxy_options = { uri: 'http://localhost:8080' }

t = Thread.new do
  loop do
    Faraday.new(proxy: proxy_options)
  end
end

sleep 1

loop do
  Faraday.new(proxy: proxy_options)
end

t.join

# => 
#<Thread:0x000000010295ddc0 faraday_proxy_multi.rb:14 run> terminated with exception (report_on_exception is true):
# ###/installs/ruby/3.4.1/lib/ruby/gems/3.4.0/gems/faraday-2.12.2/lib/faraday/options/proxy_options.rb:26:in 'Faraday::ProxyOptions.from': can't add a new key into hash during iteration (RuntimeError)

This PR avoids modifying the original hash and instead works with a duplicate one.
It also adds a test to ensure that the original proxy option remains unchanged.

[QWYNG]

I didn’t notice the difference with Struct's keyword_init in the spec and was calling Options.new directly.
I’ve updated it to use Options.from instead.
Force-pushed to keep the commit history clean.

[ioquatix]

I've been asked to comment on this issue.

I believe the best strategy is to call dup if you intend to mutate the hash, or call Utils.URI at the point where you need a URI instance. Either option is okay, but I'd avoid assuming the user provided hash is mutable.

More specifically, we are assuming mutation is safe. It may not be, and it may even be a global instance, e.g.

OPTIONS = {uri: ...}.freeze

# ... and later:
Faraday::ProxyOptions.from(OPTIONS)

[QWYNG]

@ioquatix
Thank you for your comment! You're absolutely right. We've used dup to ensure that the user-provided arguments aren't modified directly.

[iMacTia]

Thank you for fixing this @QWYNG, I absolutely agree we shouldn't mutate the hash provided as parameter. I checked the file history and this has been here for years, it probably just went unnoticed until now and I simply don't see any reason to keep it as is.