From 4a09e0f90a426a729ea59dfac71028bbe05e87c6 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Wed, 14 Apr 2021 11:12:55 -0700 Subject: [PATCH 1/8] Support additional headers for TLS proxies --- lib/excon/connection.rb | 6 ++++++ lib/excon/constants.rb | 1 + lib/excon/ssl_socket.rb | 6 ++++++ tests/proxy_tests.rb | 6 +++++- 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/excon/connection.rb b/lib/excon/connection.rb index 72e34a94..6ce1578d 100644 --- a/lib/excon/connection.rb +++ b/lib/excon/connection.rb @@ -567,6 +567,12 @@ def setup_proxy :port => uri.port, :scheme => uri.scheme, } + if @data[:ssl_proxy_headers] + if !@data[:ssl_uri_schemes].include?(@data[:scheme]) + raise ArgumentError, "The `:ssl_proxy_headers` parameter should only be used with HTTPS requests." + end + @data[:proxy][:headers] = @data.delete(:ssl_proxy_headers) + end if uri.password @data[:proxy][:password] = uri.password end diff --git a/lib/excon/constants.rb b/lib/excon/constants.rb index 1dd37cd8..899ebce5 100644 --- a/lib/excon/constants.rb +++ b/lib/excon/constants.rb @@ -99,6 +99,7 @@ module Excon :ssl_version, :ssl_min_version, :ssl_max_version, + :ssl_proxy_headers, :ssl_uri_schemes, :tcp_nodelay, :thread_safe_sockets, diff --git a/lib/excon/ssl_socket.rb b/lib/excon/ssl_socket.rb index 36090bc4..443acd4f 100644 --- a/lib/excon/ssl_socket.rb +++ b/lib/excon/ssl_socket.rb @@ -104,6 +104,12 @@ def initialize(data = {}) request += "Proxy-Connection: Keep-Alive#{Excon::CR_NL}" + if @data[:proxy][:headers] + @data[:proxy][:headers].each do |key, value| + request << key.to_s << ': ' << value.to_s << Excon::CR_NL + end + end + request += Excon::CR_NL # write out the proxy setup request diff --git a/tests/proxy_tests.rb b/tests/proxy_tests.rb index 462824e5..f4ef9acb 100644 --- a/tests/proxy_tests.rb +++ b/tests/proxy_tests.rb @@ -21,7 +21,7 @@ connection = nil tests('connection.data[:proxy][:host]').returns('myproxy.net') do - connection = Excon.new('http://foo.com', :proxy => 'https://myproxy.net:8080') + connection = Excon.new('http://foo.com', :proxy => 'https://myproxy.net:8080', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) connection.data[:proxy][:host] end @@ -32,6 +32,10 @@ tests('connection.data[:proxy][:scheme]').returns('https') do connection.data[:proxy][:scheme] end + + tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do + connection.data[:proxy][:headers] + end end tests('with fully-specified Unix socket proxy: unix:///') do From 7e660c7c5f7dc05184807e7fb93ca0d089442288 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Wed, 14 Apr 2021 16:36:41 -0700 Subject: [PATCH 2/8] Move test --- tests/proxy_tests.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/proxy_tests.rb b/tests/proxy_tests.rb index f4ef9acb..157d22ed 100644 --- a/tests/proxy_tests.rb +++ b/tests/proxy_tests.rb @@ -21,7 +21,7 @@ connection = nil tests('connection.data[:proxy][:host]').returns('myproxy.net') do - connection = Excon.new('http://foo.com', :proxy => 'https://myproxy.net:8080', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) + connection = Excon.new('http://foo.com', :proxy => 'https://myproxy.net:8080') connection.data[:proxy][:host] end @@ -32,10 +32,6 @@ tests('connection.data[:proxy][:scheme]').returns('https') do connection.data[:proxy][:scheme] end - - tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do - connection.data[:proxy][:headers] - end end tests('with fully-specified Unix socket proxy: unix:///') do @@ -92,7 +88,7 @@ def env_proxy_tests(env) connection = nil tests('connection.data[:proxy][:host]').returns('mysecureproxy') do - connection = Excon.new('https://secret.com') + connection = Excon.new('https://secret.com', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) connection.data[:proxy][:host] end @@ -104,6 +100,10 @@ def env_proxy_tests(env) connection.data[:proxy][:scheme] end + tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do + connection.data[:proxy][:headers] + end + tests('with disable_proxy set') do connection = nil From 654a6a36b67d6f6b440ff070daf935df77452e3d Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Thu, 15 Apr 2021 12:38:35 -0700 Subject: [PATCH 3/8] use :ssl_proxy_headers directly --- lib/excon/connection.rb | 9 +++------ lib/excon/ssl_socket.rb | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/excon/connection.rb b/lib/excon/connection.rb index 6ce1578d..e4223773 100644 --- a/lib/excon/connection.rb +++ b/lib/excon/connection.rb @@ -567,18 +567,15 @@ def setup_proxy :port => uri.port, :scheme => uri.scheme, } - if @data[:ssl_proxy_headers] - if !@data[:ssl_uri_schemes].include?(@data[:scheme]) - raise ArgumentError, "The `:ssl_proxy_headers` parameter should only be used with HTTPS requests." - end - @data[:proxy][:headers] = @data.delete(:ssl_proxy_headers) - end if uri.password @data[:proxy][:password] = uri.password end if uri.user @data[:proxy][:user] = uri.user end + if @data[:ssl_proxy_headers] && !@data[:ssl_uri_schemes].include?(@data[:scheme]) + raise ArgumentError, "The `:ssl_proxy_headers` parameter should only be used with HTTPS requests." + end if @data[:proxy][:scheme] == UNIX if @data[:proxy][:host] raise ArgumentError, "The `:host` parameter should not be set for `unix://` proxies.\n" + diff --git a/lib/excon/ssl_socket.rb b/lib/excon/ssl_socket.rb index 443acd4f..3861bae4 100644 --- a/lib/excon/ssl_socket.rb +++ b/lib/excon/ssl_socket.rb @@ -104,8 +104,8 @@ def initialize(data = {}) request += "Proxy-Connection: Keep-Alive#{Excon::CR_NL}" - if @data[:proxy][:headers] - @data[:proxy][:headers].each do |key, value| + if @data[:ssl_proxy_headers] + @data[:ssl_proxy_headers].each do |key, value| request << key.to_s << ': ' << value.to_s << Excon::CR_NL end end From 69d61677b8dc364896cc45b46b6ce7df1b7cd489 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Fri, 16 Apr 2021 17:07:55 -0700 Subject: [PATCH 4/8] use helper method to convert headers --- lib/excon/connection.rb | 12 +----------- lib/excon/ssl_socket.rb | 4 +--- lib/excon/utils.rb | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/excon/connection.rb b/lib/excon/connection.rb index e4223773..26bbdb7d 100644 --- a/lib/excon/connection.rb +++ b/lib/excon/connection.rb @@ -144,17 +144,7 @@ def request_call(datum) end # add headers to request - datum[:headers].each do |key, values| - if key.to_s.match(/[\r\n]/) - raise Excon::Errors::InvalidHeaderKey.new(key.to_s.inspect + ' contains forbidden "\r" or "\n"') - end - [values].flatten.each do |value| - if value.to_s.match(/[\r\n]/) - raise Excon::Errors::InvalidHeaderValue.new(value.to_s.inspect + ' contains forbidden "\r" or "\n"') - end - request << key.to_s << ': ' << value.to_s << CR_NL - end - end + request << Utils.headers_hash_to_s(datum[:headers]) # add additional "\r\n" to indicate end of headers request << CR_NL diff --git a/lib/excon/ssl_socket.rb b/lib/excon/ssl_socket.rb index 3861bae4..dceb8cc4 100644 --- a/lib/excon/ssl_socket.rb +++ b/lib/excon/ssl_socket.rb @@ -105,9 +105,7 @@ def initialize(data = {}) request += "Proxy-Connection: Keep-Alive#{Excon::CR_NL}" if @data[:ssl_proxy_headers] - @data[:ssl_proxy_headers].each do |key, value| - request << key.to_s << ': ' << value.to_s << Excon::CR_NL - end + request << Utils.headers_hash_to_s(@data[:ssl_proxy_headers]) end request += Excon::CR_NL diff --git a/lib/excon/utils.rb b/lib/excon/utils.rb index 5130839b..cd991abf 100644 --- a/lib/excon/utils.rb +++ b/lib/excon/utils.rb @@ -121,5 +121,22 @@ def unescape_form(str) str.gsub!(/\+/, ' ') str.gsub(ESCAPED) { $1.hex.chr } end + + # Performs validation on the passed header hash and returns a string representation of the headers + def headers_hash_to_s(headers) + headers_str = String.new + headers.each do |key, values| + if key.to_s.match(/[\r\n]/) + raise Excon::Errors::InvalidHeaderKey.new(key.to_s.inspect + ' contains forbidden "\r" or "\n"') + end + [values].flatten.each do |value| + if value.to_s.match(/[\r\n]/) + raise Excon::Errors::InvalidHeaderValue.new(value.to_s.inspect + ' contains forbidden "\r" or "\n"') + end + headers_str << key.to_s << ': ' << value.to_s << CR_NL + end + end + headers_str + end end end From 3a9dba4c4c357e1d18cbc330143d31e16cd6a8f7 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Fri, 16 Apr 2021 17:08:00 -0700 Subject: [PATCH 5/8] wip test updates --- tests/proxy_tests.rb | 37 ++++++++++++++++++++++++++++++++----- tests/rackups/proxy.ru | 22 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/tests/proxy_tests.rb b/tests/proxy_tests.rb index 157d22ed..92655746 100644 --- a/tests/proxy_tests.rb +++ b/tests/proxy_tests.rb @@ -88,7 +88,7 @@ def env_proxy_tests(env) connection = nil tests('connection.data[:proxy][:host]').returns('mysecureproxy') do - connection = Excon.new('https://secret.com', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) + connection = Excon.new('https://secret.com') connection.data[:proxy][:host] end @@ -100,10 +100,6 @@ def env_proxy_tests(env) connection.data[:proxy][:scheme] end - tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do - connection.data[:proxy][:headers] - end - tests('with disable_proxy set') do connection = nil @@ -277,6 +273,37 @@ def env_proxy_tests(env) end end + tests('https proxying: https://foo.com:8080') do + response = nil + + tests('response.status').returns(200) do + connection = Excon.new('https://foo.com:8080', :proxy => 'http://127.0.0.1:9292', :ssl_proxy_headers => {'X-Proxy-Id': 'abc123'}) + response = connection.request(:method => :get, :path => '/bar', :query => {:alpha => 'kappa'}) + + response.status + end + + # must be absolute form for proxy requests + tests('sent Request URI').returns('http://foo.com:8080/bar?alpha=kappa') do + response.headers['Sent-Request-Uri'] + end + + tests('sent Sent-Host header').returns('foo.com:8080') do + response.headers['Sent-Host'] + end + + tests('sent Proxy-Connection header').returns('Keep-Alive') do + response.headers['Sent-Proxy-Connection'] + end + + tests('response.body (proxied content)').returns('proxied content') do + response.body + end + + tests('sent ssl proxy header').returns('abc123') do + response.headers['X-Proxy-Id'] + end + end end with_unicorn('proxy.ru', 'unix:///tmp/myproxy.sock') do diff --git a/tests/rackups/proxy.ru b/tests/rackups/proxy.ru index e6d7b659..cfa74d32 100644 --- a/tests/rackups/proxy.ru +++ b/tests/rackups/proxy.ru @@ -5,6 +5,28 @@ class App < Sinatra::Base set :environment, :production enable :dump_errors + # a bit of a hack to handle the proxy CONNECT request since + # Sinatra doesn't support it by default + configure do + class << Sinatra::Base + def connect(path, opts={}, &block) + route 'CONNECT', path, opts, &block + end + end + Sinatra::Delegator.delegate :options + end + + connect('*') do + headers( + "Sent-Request-Uri" => request.env['REQUEST_URI'].to_s, + "Sent-Host" => request.env['HTTP_HOST'].to_s, + "Sent-Proxy-Connection" => request.env['HTTP_PROXY_CONNECTION'].to_s, + "X-Proxy-Id" => request.env['X_PROXY_ID'].to_s + ) + + halt 200, 'proxy connect successful' + end + get('*') do headers( "Sent-Request-Uri" => request.env['REQUEST_URI'].to_s, From 1dd844bc90e010b297fb06172e7ea2e7981fc615 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Thu, 22 Apr 2021 12:20:25 -0700 Subject: [PATCH 6/8] Revert "wip test updates" This reverts commit 3a9dba4c4c357e1d18cbc330143d31e16cd6a8f7. --- tests/proxy_tests.rb | 37 +++++-------------------------------- tests/rackups/proxy.ru | 22 ---------------------- 2 files changed, 5 insertions(+), 54 deletions(-) diff --git a/tests/proxy_tests.rb b/tests/proxy_tests.rb index 92655746..157d22ed 100644 --- a/tests/proxy_tests.rb +++ b/tests/proxy_tests.rb @@ -88,7 +88,7 @@ def env_proxy_tests(env) connection = nil tests('connection.data[:proxy][:host]').returns('mysecureproxy') do - connection = Excon.new('https://secret.com') + connection = Excon.new('https://secret.com', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) connection.data[:proxy][:host] end @@ -100,6 +100,10 @@ def env_proxy_tests(env) connection.data[:proxy][:scheme] end + tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do + connection.data[:proxy][:headers] + end + tests('with disable_proxy set') do connection = nil @@ -273,37 +277,6 @@ def env_proxy_tests(env) end end - tests('https proxying: https://foo.com:8080') do - response = nil - - tests('response.status').returns(200) do - connection = Excon.new('https://foo.com:8080', :proxy => 'http://127.0.0.1:9292', :ssl_proxy_headers => {'X-Proxy-Id': 'abc123'}) - response = connection.request(:method => :get, :path => '/bar', :query => {:alpha => 'kappa'}) - - response.status - end - - # must be absolute form for proxy requests - tests('sent Request URI').returns('http://foo.com:8080/bar?alpha=kappa') do - response.headers['Sent-Request-Uri'] - end - - tests('sent Sent-Host header').returns('foo.com:8080') do - response.headers['Sent-Host'] - end - - tests('sent Proxy-Connection header').returns('Keep-Alive') do - response.headers['Sent-Proxy-Connection'] - end - - tests('response.body (proxied content)').returns('proxied content') do - response.body - end - - tests('sent ssl proxy header').returns('abc123') do - response.headers['X-Proxy-Id'] - end - end end with_unicorn('proxy.ru', 'unix:///tmp/myproxy.sock') do diff --git a/tests/rackups/proxy.ru b/tests/rackups/proxy.ru index cfa74d32..e6d7b659 100644 --- a/tests/rackups/proxy.ru +++ b/tests/rackups/proxy.ru @@ -5,28 +5,6 @@ class App < Sinatra::Base set :environment, :production enable :dump_errors - # a bit of a hack to handle the proxy CONNECT request since - # Sinatra doesn't support it by default - configure do - class << Sinatra::Base - def connect(path, opts={}, &block) - route 'CONNECT', path, opts, &block - end - end - Sinatra::Delegator.delegate :options - end - - connect('*') do - headers( - "Sent-Request-Uri" => request.env['REQUEST_URI'].to_s, - "Sent-Host" => request.env['HTTP_HOST'].to_s, - "Sent-Proxy-Connection" => request.env['HTTP_PROXY_CONNECTION'].to_s, - "X-Proxy-Id" => request.env['X_PROXY_ID'].to_s - ) - - halt 200, 'proxy connect successful' - end - get('*') do headers( "Sent-Request-Uri" => request.env['REQUEST_URI'].to_s, From 1e5a5eb912db732e1d545b7b17f4cf621b95a27a Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Thu, 22 Apr 2021 12:23:51 -0700 Subject: [PATCH 7/8] remove test --- tests/proxy_tests.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/proxy_tests.rb b/tests/proxy_tests.rb index 157d22ed..462824e5 100644 --- a/tests/proxy_tests.rb +++ b/tests/proxy_tests.rb @@ -88,7 +88,7 @@ def env_proxy_tests(env) connection = nil tests('connection.data[:proxy][:host]').returns('mysecureproxy') do - connection = Excon.new('https://secret.com', :ssl_proxy_headers => {'x-proxy-id': 'abc123' }) + connection = Excon.new('https://secret.com') connection.data[:proxy][:host] end @@ -100,10 +100,6 @@ def env_proxy_tests(env) connection.data[:proxy][:scheme] end - tests('connection.data[:proxy][:headers]').returns({ 'x-proxy-id': 'abc123' }) do - connection.data[:proxy][:headers] - end - tests('with disable_proxy set') do connection = nil From b415556aec406ac1d92cfc430cecb8f36b3033c7 Mon Sep 17 00:00:00 2001 From: Jessica Jiang Date: Fri, 23 Apr 2021 10:27:25 -0700 Subject: [PATCH 8/8] pass datum to socket --- lib/excon/connection.rb | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/excon/connection.rb b/lib/excon/connection.rb index 26bbdb7d..7ca064d8 100644 --- a/lib/excon/connection.rb +++ b/lib/excon/connection.rb @@ -115,7 +115,7 @@ def request_call(datum) # we already have data from a middleware, so bail return datum else - socket.data = datum + socket(datum) # start with "METHOD /path" request = datum[:method].to_s.upcase + ' ' if datum[:proxy] && datum[:scheme] != HTTPS @@ -150,19 +150,19 @@ def request_call(datum) request << CR_NL if datum.has_key?(:request_block) - socket.write(request) # write out request + headers + socket(datum).write(request) # write out request + headers while true # write out body with chunked encoding chunk = datum[:request_block].call chunk = binary_encode(chunk) if chunk.length > 0 - socket.write(chunk.length.to_s(16) << CR_NL << chunk << CR_NL) + socket(datum).write(chunk.length.to_s(16) << CR_NL << chunk << CR_NL) else - socket.write(String.new("0#{CR_NL}#{CR_NL}")) + socket(datum).write(String.new("0#{CR_NL}#{CR_NL}")) break end end elsif body.nil? - socket.write(request) # write out request + headers + socket(datum).write(request) # write out request + headers else # write out body if body.respond_to?(:binmode) && !body.is_a?(StringIO) body.binmode @@ -176,13 +176,13 @@ def request_call(datum) chunk = body.read([datum[:chunk_size] - request.length, 0].max) if chunk chunk = binary_encode(chunk) - socket.write(request << chunk) + socket(datum).write(request << chunk) else - socket.write(request) # write out request + headers + socket(datum).write(request) # write out request + headers end while (chunk = body.read(datum[:chunk_size])) - socket.write(chunk) + socket(datum).write(chunk) end end end @@ -453,14 +453,14 @@ def response(datum={}) end end - def socket - unix_proxy = @data[:proxy] ? @data[:proxy][:scheme] == UNIX : false - sockets[@socket_key] ||= if @data[:scheme] == UNIX || unix_proxy - Excon::UnixSocket.new(@data) - elsif @data[:ssl_uri_schemes].include?(@data[:scheme]) - Excon::SSLSocket.new(@data) + def socket(datum = @data) + unix_proxy = datum[:proxy] ? datum[:proxy][:scheme] == UNIX : false + sockets[@socket_key] ||= if datum[:scheme] == UNIX || unix_proxy + Excon::UnixSocket.new(datum) + elsif datum[:ssl_uri_schemes].include?(datum[:scheme]) + Excon::SSLSocket.new(datum) else - Excon::Socket.new(@data) + Excon::Socket.new(datum) end end