Merge pull request #72 from launchdarkly/eb/ch19217/proxy

fix proxy implementation so it actually works

Author: eli-darkly

README.md

@@ -83,7 +83,7 @@ Note that this gem will automatically switch to using the Rails logger it is det
 
 HTTPS proxy
 ------------
-The Ruby SDK uses Faraday to handle all of its network traffic. Faraday provides built-in support for the use of an  HTTPS proxy. If the HTTPS_PROXY environment variable is present then the SDK will proxy all network requests through the URL provided.
+The Ruby SDK uses Faraday and Socketry to handle its network traffic. Both of these provide built-in support for the use of an  HTTPS proxy. If the HTTPS_PROXY environment variable is present then the SDK will proxy all network requests through the URL provided.
 
 How to set the HTTPS_PROXY environment variable on Mac/Linux systems:
 ```

ldclient-rb.gemspec

@@ -26,23 +26,13 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency "codeclimate-test-reporter", "~> 0"
   spec.add_development_dependency "redis", "~> 3.3.5"
   spec.add_development_dependency "connection_pool", ">= 2.1.2"
-  if RUBY_VERSION >= "2.0.0"
-    spec.add_development_dependency "rake", "~> 10.0"
-    spec.add_development_dependency "rspec_junit_formatter", "~> 0.3.0"
-  else
-    spec.add_development_dependency "rake", "12.1.0"
-    # higher versions of rake fail to install in JRuby 1.7
-  end
+  spec.add_development_dependency "rake", "~> 10.0"
+  spec.add_development_dependency "rspec_junit_formatter", "~> 0.3.0"
   spec.add_development_dependency "timecop", "~> 0.9.1"
 
   spec.add_runtime_dependency "json", [">= 1.8", "< 3"]
-  if RUBY_VERSION >= "2.1.0"
-    spec.add_runtime_dependency "faraday", [">= 0.9", "< 2"]
-    spec.add_runtime_dependency "faraday-http-cache", [">= 1.3.0", "< 3"]
-  else
-    spec.add_runtime_dependency "faraday", [">= 0.9", "< 0.14.0"]
-    spec.add_runtime_dependency "faraday-http-cache", [">= 1.3.0", "< 2"]
-  end
+  spec.add_runtime_dependency "faraday", [">= 0.9", "< 2"]
+  spec.add_runtime_dependency "faraday-http-cache", [">= 1.3.0", "< 3"]
   spec.add_runtime_dependency "semantic", "~> 1.6.0"
   spec.add_runtime_dependency "thread_safe", "~> 0.3"
   spec.add_runtime_dependency "net-http-persistent", "~> 2.9"

lib/sse_client/sse_client.rb

@@ -24,10 +24,11 @@ def initialize(uri, options = {})
       @read_timeout = options[:read_timeout] || DEFAULT_READ_TIMEOUT
       @logger = options[:logger] || default_logger
 
-      proxy = ENV['HTTP_PROXY'] || ENV['http_proxy'] || options[:proxy]
-      if proxy
-        proxyUri = URI(proxy)
-        if proxyUri.scheme == 'http' || proxyUri.scheme == 'https'
+      if options[:proxy]
+        @proxy = options[:proxy]
+      else
+        proxyUri = @uri.find_proxy
+        if !proxyUri.nil? && (proxyUri.scheme == 'http' || proxyUri.scheme == 'https')
           @proxy = proxyUri
         end
       end
@@ -152,8 +153,7 @@ def dispatch_event(event)
     def build_headers
       h = {
         'Accept' => 'text/event-stream',
-        'Cache-Control' => 'no-cache',
-        'Host' => @uri.host
+        'Cache-Control' => 'no-cache'
       }
       h['Last-Event-Id'] = @last_id if !@last_id.nil?
       h.merge(@headers)

lib/sse_client/streaming_http.rb

@@ -7,32 +7,21 @@ module SSE
   # The socket is created and managed by Socketry, which we use so that we can have a read timeout.
   #
   class StreamingHTTPConnection
-    def initialize(uri, proxy, headers, connect_timeout, read_timeout)
-      if proxy
-        @socket = open_socket(proxy, connect_timeout)
-        @socket.write(build_proxy_request(uri, proxy))
-      else
-        @socket = open_socket(uri, connect_timeout)
-      end
+    attr_reader :status, :headers
 
+    def initialize(uri, proxy, headers, connect_timeout, read_timeout)
+      @socket = HTTPConnectionFactory.connect(uri, proxy, connect_timeout, read_timeout)
       @socket.write(build_request(uri, headers))
-
       @reader = HTTPResponseReader.new(@socket, read_timeout)
+      @status = @reader.status
+      @headers = @reader.headers
     end
 
     def close
       @socket.close if @socket
       @socket = nil
     end
-
-    def status
-      @reader.status
-    end
-
-    def headers
-      @reader.headers
-    end
-
+    
     # Generator that returns one line of the response body at a time (delimited by \r, \n,
     # or \r\n) until the response is fully consumed or the socket is closed.
     def read_lines
@@ -46,25 +35,55 @@ def read_all
 
     private
 
-    def open_socket(uri, connect_timeout)
-      if uri.scheme == 'https'
-        Socketry::SSL::Socket.connect(uri.host, uri.port, timeout: connect_timeout)
-      else
-        Socketry::TCP::Socket.connect(uri.host, uri.port, timeout: connect_timeout)
-      end
-    end
-
     # Build an HTTP request line and headers.
     def build_request(uri, headers)
       ret = "GET #{uri.request_uri} HTTP/1.1\r\n"
+      ret << "Host: #{uri.host}\r\n"
       headers.each { |k, v|
         ret << "#{k}: #{v}\r\n"
       }
       ret + "\r\n"
     end
+  end
+
+  #
+  # Used internally to send the HTTP request, including the proxy dialogue if necessary.
+  #
+  class HTTPConnectionFactory
+    def self.connect(uri, proxy, connect_timeout, read_timeout)
+      if !proxy
+        return open_socket(uri, connect_timeout)
+      end
+
+      socket = open_socket(proxy, connect_timeout)
+      socket.write(build_proxy_request(uri, proxy))
+
+      # temporarily create a reader just for the proxy connect response
+      proxy_reader = HTTPResponseReader.new(socket, read_timeout)
+      if proxy_reader.status != 200
+        raise ProxyError, "proxy connection refused, status #{proxy_reader.status}"
+      end
+
+      # start using TLS at this point if appropriate
+      if uri.scheme.downcase == 'https'
+        wrap_socket_in_ssl_socket(socket)
+      else
+        socket
+      end
+    end
+
+    private
+
+    def self.open_socket(uri, connect_timeout)
+      if uri.scheme.downcase == 'https'
+        Socketry::SSL::Socket.connect(uri.host, uri.port, timeout: connect_timeout)
+      else
+        Socketry::TCP::Socket.connect(uri.host, uri.port, timeout: connect_timeout)
+      end
+    end
 
     # Build a proxy connection header.
-    def build_proxy_request(uri, proxy)
+    def self.build_proxy_request(uri, proxy)
       ret = "CONNECT #{uri.host}:#{uri.port} HTTP/1.1\r\n"
       ret << "Host: #{uri.host}:#{uri.port}\r\n"
       if proxy.user || proxy.password
@@ -74,6 +93,19 @@ def build_proxy_request(uri, proxy)
       ret << "\r\n"
       ret
     end
+
+    def self.wrap_socket_in_ssl_socket(socket)
+      io = IO.try_convert(socket)
+      ssl_sock = OpenSSL::SSL::SSLSocket.new(io, OpenSSL::SSL::SSLContext.new)
+      ssl_sock.connect
+      Socketry::SSL::Socket.new.from_socket(ssl_sock)
+    end
+  end
+
+  class ProxyError < StandardError
+    def initialize(message)
+      super
+    end
   end
 
   #

spec/sse_client/sse_shared.rb

@@ -1,22 +1,28 @@
 require "spec_helper"
 require "webrick"
+require "webrick/httpproxy"
+require "webrick/https"
 
 class StubHTTPServer
   def initialize
     @port = 50000
     begin
-      @server = WEBrick::HTTPServer.new(
-        BindAddress: '127.0.0.1',
-        Port: @port,
-        AccessLog: [],
-        Logger: NullLogger.new
-      )
+      @server = create_server(@port)
     rescue Errno::EADDRINUSE
       @port += 1
       retry
     end
   end
 
+  def create_server(port)
+    WEBrick::HTTPServer.new(
+      BindAddress: '127.0.0.1',
+      Port: port,
+      AccessLog: [],
+      Logger: NullLogger.new
+    )
+  end
+
   def start
     Thread.new { @server.start }
   end
@@ -34,14 +40,39 @@ def setup_response(uri_path, &action)
   end
 end
 
+class StubProxyServer < StubHTTPServer
+  attr_reader :request_count
+  attr_accessor :connect_status
+
+  def initialize
+    super
+    @request_count = 0
+  end
+
+  def create_server(port)
+    WEBrick::HTTPProxyServer.new(
+      BindAddress: '127.0.0.1',
+      Port: port,
+      AccessLog: [],
+      Logger: NullLogger.new,
+      ProxyContentHandler: proc do |req,res|
+        if !@connect_status.nil?
+          res.status = @connect_status
+        end
+        @request_count += 1
+      end
+    )
+  end
+end
+
 class NullLogger
   def method_missing(*)
     self
   end
 end
 
-def with_server
-  server = StubHTTPServer.new
+def with_server(server = nil)
+  server = StubHTTPServer.new if server.nil?
   begin
     server.start
     yield server

spec/sse_client/streaming_http_spec.rb

@@ -29,7 +29,10 @@ def with_connection(cxn)
       with_connection(subject.new(server.base_uri.merge("/foo?bar"), nil, headers, 30, 30)) do
         received_req = requests.pop
         expect(received_req.unparsed_uri).to eq("/foo?bar")
-        expect(received_req.header).to eq({ "accept" => ["text/plain"] })
+        expect(received_req.header).to eq({
+          "accept" => ["text/plain"],
+          "host" => [server.base_uri.host]
+        })
       end
     end
   end
@@ -98,6 +101,54 @@ def with_connection(cxn)
       expect { subject.new(server.base_uri, nil, {}, 30, 0.25) }.to raise_error(Socketry::TimeoutError)
     end
   end
+
+  it "connects to HTTP server through proxy" do
+    body = "hi"
+    with_server do |server|
+      server.setup_response("/") do |req,res|
+        res.body = body
+      end
+      with_server(StubProxyServer.new) do |proxy|
+        with_connection(subject.new(server.base_uri, proxy.base_uri, {}, 30, 30)) do |cxn|
+          read_body = cxn.read_all
+          expect(read_body).to eq("hi")
+          expect(proxy.request_count).to eq(1)
+        end
+      end
+    end
+  end
+
+  it "throws error if proxy responds with error status" do
+    with_server do |server|
+      server.setup_response("/") do |req,res|
+        res.body = body
+      end
+      with_server(StubProxyServer.new) do |proxy|
+        proxy.connect_status = 403
+        expect { subject.new(server.base_uri, proxy.base_uri, {}, 30, 30) }.to raise_error(SSE::ProxyError)
+      end
+    end
+  end
+
+  # The following 2 tests were originally written to connect to an embedded HTTPS server made with
+  # WEBrick. Unfortunately, some unknown problem prevents WEBrick's self-signed certificate feature
+  # from working in JRuby 9.1 (but not in any other Ruby version). Therefore these tests currently
+  # hit an external URL.
+
+  it "connects to HTTPS server" do
+    with_connection(subject.new(URI("https://app.launchdarkly.com"), nil, {}, 30, 30)) do |cxn|
+      expect(cxn.status).to eq 200
+    end
+  end
+  
+  it "connects to HTTPS server through proxy" do
+    with_server(StubProxyServer.new) do |proxy|
+      with_connection(subject.new(URI("https://app.launchdarkly.com"), proxy.base_uri, {}, 30, 30)) do |cxn|
+        expect(cxn.status).to eq 200
+        expect(proxy.request_count).to eq(1)
+      end
+    end
+  end
 end
 
 #