From 93b4ac85421aaf4de1347c30b138d51adde29a50 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 13:37:17 +0100 Subject: [PATCH 1/6] added tests to faraday adapter, for timeout and proxy based features --- test/adapters/faraday_test.rb | 107 +++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 34 deletions(-) diff --git a/test/adapters/faraday_test.rb b/test/adapters/faraday_test.rb index 46597937..1754e1e5 100644 --- a/test/adapters/faraday_test.rb +++ b/test/adapters/faraday_test.rb @@ -11,12 +11,14 @@ class FaradayTest < Minitest::Test include HTTPHelpers include ProxyHelper - def_delegators :create_connection, :get, :head, :put, :post, :patch, :delete, :run_request + using HTTPX::URIExtensions + + def_delegators :faraday_connection, :get, :head, :put, :post, :patch, :delete, :run_request def test_adapter_in_parallel resp1, resp2 = nil, nil - connection = create_connection + connection = faraday_connection connection.in_parallel do resp1 = connection.get(build_path("/get?a=1")) resp2 = connection.get(build_path("/get?b=2")) @@ -32,7 +34,7 @@ class FaradayTest < Minitest::Test def test_adapter_in_parallel_errors resp1 = nil - connection = create_connection + connection = faraday_connection connection.in_parallel do resp1 = connection.get("http://wfijojsfsoijf") assert connection.in_parallel? @@ -46,7 +48,7 @@ class FaradayTest < Minitest::Test end def test_adapter_in_parallel_no_requests - connection = create_connection + connection = faraday_connection assert_nil(connection.in_parallel {}) end @@ -57,11 +59,16 @@ class FaradayTest < Minitest::Test def test_adapter_get_ssl_fails_with_bad_cert err = assert_raises Faraday::Adapter::HTTPX::SSL_ERROR do - get("https://wrong.host.badssl.com/") + faraday_connection(server_uri: "https://expired.badssl.com/", ssl: { verify: true }).get("/") end assert_includes err.message, "certificate" end + def test_adapter_ssl_verify_none + res = faraday_connection(server_uri: "https://expired.badssl.com/", ssl: { verify: false }).get("/") + assert res.status == 200 + end + def test_adapter_get_send_url_encoded_params assert_equal({ "name" => "zack" }, JSON.parse(get(build_path("/get"), name: "zack").body.to_s)["args"]) end @@ -132,20 +139,59 @@ class FaradayTest < Minitest::Test def test_adapter_get_data streamed = [] get(build_path("/stream/3")) do |req| + assert !req.options.stream_response? req.options.on_data = proc do |chunk, _overall_received_bytes| streamed << chunk end + assert req.options.stream_response? end + assert !streamed.empty? assert streamed.join.lines.size == 3 end if Faraday::VERSION >= "1.0.0" - # def test_adapter_timeout - # conn = create_connection(request: { timeout: 1, open_timeout: 1 }) - # assert_raises Faraday::Error::TimeoutError do - # conn.get(build_path("/delay/5")) - # end - # end + def test_adapter_timeout_open_timeout + server = TCPServer.new("127.0.0.1", CONNECT_TIMEOUT_PORT) + begin + uri = URI(build_uri("/", origin("127.0.0.1:#{CONNECT_TIMEOUT_PORT}"))) + conn = faraday_connection(server_uri: uri.origin, request: { open_timeout: 0.5 }) + assert_raises Faraday::TimeoutError do + conn.get("/") + end + ensure + server.close + end + end + + def test_adapter_timeout_read_timeout + conn = faraday_connection(request: { read_timeout: 0.5 }) + assert_raises Faraday::TimeoutError do + conn.get(build_path("/delay/4")) + end + end + + def test_adapter_timeouts_write_timeout + start_test_servlet(SlowReader) do |server| + uri = URI("#{server.origin}/") + conn = faraday_connection(request: { write_timeout: 0.5 }) + assert_raises Faraday::TimeoutError do + conn.post(uri, StringIO.new("a" * 65_536 * 3 * 5)) + end + end + end + + def test_adapter_bind + start_test_servlet(KeepAliveServer) do |server| + origin = URI(server.origin) + ip = origin.host + port = origin.port + conn = faraday_connection(request: { bind: { host: ip, port: port } }) + response = conn.get("/") + verify_status(response, 200) + body = response.body.to_s + assert body == "{\"counter\": infinity}" + end + end def test_adapter_connection_error assert_raises Faraday::Adapter::HTTPX::CONNECTION_FAILED_ERROR do @@ -153,28 +199,15 @@ class FaradayTest < Minitest::Test end end - # def test_proxy - # proxy_uri = http_proxy.first - # conn = create_connection(proxy: proxy_uri) + def test_adapter_proxy + proxy_uri = http_proxy.first + conn = faraday_connection(proxy: proxy_uri) - # res = conn.get(build_path("/get")) - # assert res.status == 200 + res = conn.get(build_path("/get")) + assert res.status == 200 - # unless self.class.ssl_mode? - # # proxy can't append "Via" header for HTTPS responses - # assert_match(/:#{proxy_uri.port}$/, res["via"]) - # end - # end - - # def test_proxy_auth_fail - # proxy_uri = URI(ENV["LIVE_PROXY"]) - # proxy_uri.password = "WRONG" - # conn = create_connection(proxy: proxy_uri) - - # err = assert_raises Faraday::Error::ConnectionFailed do - # conn.get "/echo" - # end - # end + # TODO: test that request has been proxied + end private @@ -191,7 +224,9 @@ class FaradayTest < Minitest::Test [] end - def create_connection(options = {}, &optional_connection_config_blk) + def faraday_connection(options = {}, &optional_connection_config_blk) + return @faraday_connection if defined?(@faraday_connection) + builder_block = proc do |b| b.request :url_encoded b.adapter :httpx, *adapter_options, &optional_connection_config_blk @@ -200,12 +235,16 @@ class FaradayTest < Minitest::Test options[:ssl] ||= {} options[:ssl][:ca_file] ||= ENV["SSL_FILE"] - server = URI("https://#{httpbin}") + server = options.delete(:server_uri) || URI("https://#{httpbin}") - Faraday::Connection.new(server.to_s, options, &builder_block).tap do |conn| + @faraday_connection = Faraday::Connection.new(server.to_s, options, &builder_block).tap do |conn| conn.headers["X-Faraday-Adapter"] = "httpx" adapter_handler = conn.builder.handlers.last conn.builder.insert_before adapter_handler, Faraday::Response::RaiseError end end + + def teardown + @faraday_connection.close if defined?(@faraday_connection) + end end From b7d421fdcdf0561068a6dac1ff1daec72f3c3d63 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 13:41:32 +0100 Subject: [PATCH 2/6] fix for accessing wrong ivar --- lib/httpx/adapters/faraday.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/httpx/adapters/faraday.rb b/lib/httpx/adapters/faraday.rb index 1b6be6f5..b992bc90 100644 --- a/lib/httpx/adapters/faraday.rb +++ b/lib/httpx/adapters/faraday.rb @@ -55,7 +55,7 @@ module Faraday end def close - @session.close + @connection.close if @connection end private From b7a850f6dac6fbe019b037f14f099204fa68f639 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 13:42:06 +0100 Subject: [PATCH 3/6] turn httpx timeout errors into faraday errors --- lib/httpx/adapters/faraday.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/httpx/adapters/faraday.rb b/lib/httpx/adapters/faraday.rb index b992bc90..ac5fd350 100644 --- a/lib/httpx/adapters/faraday.rb +++ b/lib/httpx/adapters/faraday.rb @@ -73,6 +73,8 @@ module Faraday Errno::EPIPE, ::HTTPX::ConnectionError => e raise CONNECTION_FAILED_ERROR, e + rescue ::HTTPX::TimeoutError => e + raise Faraday::TimeoutError, e end def build_request(env) @@ -232,6 +234,8 @@ module Faraday handler.on_complete.call(handler.env) end end + rescue ::HTTPX::TimeoutError => e + raise Faraday::TimeoutError, e end # from Faraday::Adapter#connection @@ -299,6 +303,8 @@ module Faraday response.raise_for_status unless response.is_a?(::HTTPX::Response) response end + rescue ::HTTPX::TimeoutError => e + raise Faraday::TimeoutError, e end def parallel?(env) From 7be554dc6297013b58d93150f3838a8daa1b1422 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 13:43:22 +0100 Subject: [PATCH 4/6] fix: faraday timeouts not being correctly mapped to httpx timeouts --- lib/httpx/adapters/faraday.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/httpx/adapters/faraday.rb b/lib/httpx/adapters/faraday.rb index ac5fd350..51d53d5e 100644 --- a/lib/httpx/adapters/faraday.rb +++ b/lib/httpx/adapters/faraday.rb @@ -89,15 +89,16 @@ module Faraday def options_from_env(env) timeout_options = {} - if (sec = request_timeout(:read, env)) + req_opts = env.request + if (sec = request_timeout(:read, req_opts)) timeout_options[:operation_timeout] = sec end - if (sec = request_timeout(:write, env)) + if (sec = request_timeout(:write, req_opts)) timeout_options[:operation_timeout] = sec end - if (sec = request_timeout(:open, env)) + if (sec = request_timeout(:open, req_opts)) timeout_options[:connect_timeout] = sec end From 2ef2b5f7976e6150d21815f881e9b43a94eeb2e3 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 13:44:00 +0100 Subject: [PATCH 5/6] fix: set ssl verify to none when verify field is false (and ignore when nil) --- lib/httpx/adapters/faraday.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/httpx/adapters/faraday.rb b/lib/httpx/adapters/faraday.rb index 51d53d5e..37fb9094 100644 --- a/lib/httpx/adapters/faraday.rb +++ b/lib/httpx/adapters/faraday.rb @@ -104,7 +104,10 @@ module Faraday ssl_options = {} - ssl_options[:verify_mode] = OpenSSL::SSL::VERIFY_PEER if env.ssl.verify + unless env.ssl.verify.nil? + ssl_options[:verify_mode] = env.ssl.verify ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE + end + ssl_options[:ca_file] = env.ssl.ca_file if env.ssl.ca_file ssl_options[:ca_path] = env.ssl.ca_path if env.ssl.ca_path ssl_options[:cert_store] = env.ssl.cert_store if env.ssl.cert_store From 1b9422e828848541035b990a5f93d2a86ccd6a71 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 31 Jul 2023 15:21:08 +0100 Subject: [PATCH 6/6] supporting faraday bind request option --- lib/httpx/adapters/faraday.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/httpx/adapters/faraday.rb b/lib/httpx/adapters/faraday.rb index 37fb9094..0af94daf 100644 --- a/lib/httpx/adapters/faraday.rb +++ b/lib/httpx/adapters/faraday.rb @@ -40,7 +40,13 @@ module Faraday @connection = ::HTTPX.plugin(:compression).plugin(:persistent).plugin(ReasonPlugin) @connection = @connection.with(@connection_options) unless @connection_options.empty? - @connection = @connection.with(options_from_env(env)) + connection_opts = options_from_env(env) + + if (bind = env.request.bind) + @bind = TCPSocket.new(bind[:host], bind[:port]) + connection_opts[:io] = @bind + end + @connection = @connection.with(connection_opts) if (proxy = env.request.proxy) proxy_options = { uri: proxy.uri } @@ -56,6 +62,7 @@ module Faraday def close @connection.close if @connection + @bind.close if @bind end private