Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First attempt at supporting https connections through proxy #186

Merged
merged 1 commit into from
Apr 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Metrics/BlockNesting:

Metrics/ClassLength:
CountComments: false
Max: 110
Max: 120

Metrics/PerceivedComplexity:
Max: 8
Expand Down
7 changes: 5 additions & 2 deletions lib/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ def make_request(req, options)
@state = :dirty

@connection ||= HTTP::Connection.new(req, options)
@connection.send_request(req)
@connection.read_headers!

unless @connection.failed_proxy_connect?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A failed proxy connection should be an error, rather than soft failing I think. It's the equivalent of a socket error in that you're saying to connect to X and it failed.

If the concern is around accessing the response for further debugging, we can do something like what Net::HTTP does and include response as part of the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me what the correct behavior is here. This case includes the 407 response when proxy authentication fails (which we have a test case for), its also consistent with the current behavior for http proxies.

i.e.
HTTP.get('http://not-real-sldjflkdjlkds.com') => exception
HTTP.via(proxy.addr, proxy.port).get('http://not-real-sldjflkdjlkds.com') => 500 response

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okie-doke. If it's consistent with what we have then 👍

@connection.send_request(req)
@connection.read_headers!
end

res = Response.new(
@connection.status_code,
Expand Down
40 changes: 35 additions & 5 deletions lib/http/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module HTTP
# A connection to the HTTP server
class Connection
extend Forwardable

# Attempt to read this much data
BUFFER_SIZE = 16_384

Expand All @@ -18,16 +19,18 @@ class Connection
# @param [HTTP::Request] req
# @param [HTTP::Options] options
def initialize(req, options)
@persistent = options.persistent?
@keep_alive_timeout = options[:keep_alive_timeout].to_f
@pending_request = false
@pending_response = false
@persistent = options.persistent?
@keep_alive_timeout = options[:keep_alive_timeout].to_f
@pending_request = false
@pending_response = false
@failed_proxy_connect = false

@parser = Response::Parser.new

@socket = options[:timeout_class].new(options[:timeout_options])
@socket.connect(options[:socket_class], req.socket_host, req.socket_port)

send_proxy_connect_request(req)
start_tls(req, options)
reset_timer
end
Expand All @@ -41,6 +44,11 @@ def initialize(req, options)
# @see (HTTP::Response::Parser#headers)
def_delegator :@parser, :headers

# @return [Boolean] whenever proxy connect failed
def failed_proxy_connect?
@failed_proxy_connect
end

# Send a request to the server
#
# @param [Request] Request to send to the server
Expand Down Expand Up @@ -129,7 +137,7 @@ def expired?
# @param (see #initialize)
# @return [void]
def start_tls(req, options)
return unless req.uri.https? && !req.using_proxy?
return unless req.uri.https? && !failed_proxy_connect?

ssl_context = options[:ssl_context]

Expand All @@ -141,6 +149,28 @@ def start_tls(req, options)
@socket.start_tls(req.uri.host, options[:ssl_socket_class], ssl_context)
end

# Open tunnel through proxy
def send_proxy_connect_request(req)
return unless req.uri.https? && req.using_proxy?

@pending_request = true

req.connect_using_proxy @socket

@pending_request = false
@pending_response = true

read_headers!

if @parser.status_code == 200
@parser.reset
@pending_response = false
return
end

@failed_proxy_connect = true
end

# Resets expiration of persistent connection.
# @return [void]
def reset_timer
Expand Down
30 changes: 27 additions & 3 deletions lib/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def redirect(uri, verb = @verb)

# Stream the request to a socket
def stream(socket)
include_proxy_authorization_header if using_authenticated_proxy?
include_proxy_authorization_header if using_authenticated_proxy? && !@uri.https?
Request::Writer.new(socket, body, headers, request_header).stream
end

Expand All @@ -113,15 +113,39 @@ def using_authenticated_proxy?

# Compute and add the Proxy-Authorization header
def include_proxy_authorization_header
digest = Base64.encode64("#{proxy[:proxy_username]}:#{proxy[:proxy_password]}").chomp
headers["Proxy-Authorization"] = "Basic #{digest}"
headers["Proxy-Authorization"] = proxy_authorization_header
end

def proxy_authorization_header
digest = Base64.strict_encode64("#{proxy[:proxy_username]}:#{proxy[:proxy_password]}")
"Basic #{digest}"
end

# Setup tunnel through proxy for SSL request
def connect_using_proxy(socket)
Request::Writer.new(socket, nil, proxy_connect_headers, proxy_connect_header).connect_through_proxy
end

# Compute HTTP request header for direct or proxy request
def request_header
"#{verb.to_s.upcase} #{uri.normalize} HTTP/#{version}"
end

# Compute HTTP request header SSL proxy connection
def proxy_connect_header
"CONNECT #{@uri.host}:#{@uri.port} HTTP/#{version}"
end

# Headers to send with proxy connect request
def proxy_connect_headers
connect_headers = HTTP::Headers.coerce(
"Host" => headers["Host"],
"User-Agent" => headers["User-Agent"]
)
connect_headers["Proxy-Authorization"] = proxy_authorization_header if using_authenticated_proxy?
connect_headers
end

# Host for tcp socket
def socket_host
using_proxy? ? proxy[:proxy_address] : host
Expand Down
9 changes: 7 additions & 2 deletions lib/http/request/writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ def stream
send_request_body
end

# Send headers needed to connect through proxy
def connect_through_proxy
add_headers
@socket << join_headers
end

# Adds the headers to the header array for the given request body we are working
# with
def add_body_type_headers
Expand All @@ -50,9 +56,8 @@ def join_headers
def send_request_header
add_headers
add_body_type_headers
header = join_headers

@socket << header
@socket << join_headers
end

def send_request_body
Expand Down
34 changes: 34 additions & 0 deletions spec/lib/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@

RSpec.describe HTTP do
run_server(:dummy) { DummyServer.new }
run_server(:dummy_ssl) { DummyServer.new(:ssl => true) }

let(:ssl_client) do
HTTP::Client.new :ssl_context => SSLHelper.client_context
end

context "getting resources" do
it "is easy" do
Expand Down Expand Up @@ -63,6 +68,18 @@
response = HTTP.via(proxy.addr, proxy.port, "username", "password").get dummy.endpoint
expect(response.to_s).to match(/<!doctype html>/)
end

context "ssl" do
it "responds with the endpoint's body" do
response = ssl_client.via(proxy.addr, proxy.port).get dummy_ssl.endpoint
expect(response.to_s).to match(/<!doctype html>/)
end

it "ignores credentials" do
response = ssl_client.via(proxy.addr, proxy.port, "username", "password").get dummy_ssl.endpoint
expect(response.to_s).to match(/<!doctype html>/)
end
end
end

context "proxy with authentication" do
Expand All @@ -87,6 +104,23 @@
response = HTTP.via(proxy.addr, proxy.port).get dummy.endpoint
expect(response.status).to eq(407)
end

context "ssl" do
it "responds with the endpoint's body" do
response = ssl_client.via(proxy.addr, proxy.port, "username", "password").get dummy_ssl.endpoint
expect(response.to_s).to match(/<!doctype html>/)
end

it "responds with 407 when wrong credentials given" do
response = ssl_client.via(proxy.addr, proxy.port, "user", "pass").get dummy_ssl.endpoint
expect(response.status).to eq(407)
end

it "responds with 407 if no credentials given" do
response = ssl_client.via(proxy.addr, proxy.port).get dummy_ssl.endpoint
expect(response.status).to eq(407)
end
end
end
end

Expand Down