13 files changed, 726 insertions, 116 deletions

diff --git a/spec/unit/network/authentication_spec.rb b/spec/unit/network/authentication_spec.rb
index 8f3653cad..5e2f2de87 100755
--- a/spec/unit/network/authentication_spec.rb
+++ b/spec/unit/network/authentication_spec.rb
@@ -81,6 +81,10 @@ describe Puppet::Network::Authentication do
         cert.stubs(:unmunged_name).returns('foo')
       end
 
+      after(:all) do
+        reload_module
+      end
+
       it "should log a warning if a certificate's expiration is near" do
         logger.expects(:warning)
         subject.warn_if_near_expiration(cert)
diff --git a/spec/unit/network/http/api/v2/environments_spec.rb b/spec/unit/network/http/api/v2/environments_spec.rb
index 6c6d7a581..993e55011 100644
--- a/spec/unit/network/http/api/v2/environments_spec.rb
+++ b/spec/unit/network/http/api/v2/environments_spec.rb
@@ -23,20 +23,41 @@ describe Puppet::Network::HTTP::API::V2::Environments do
         "production" => {
           "settings" => {
             "modulepath" => [File.expand_path("/first"), File.expand_path("/second")],
-            "manifest" => File.expand_path("/manifests")
+            "manifest" => File.expand_path("/manifests"),
+            "environment_timeout" => 0,
+            "config_version" => ""
           }
         }
       }
     })
   end
 
-  it "the response conforms to the environments schema" do
+  it "the response conforms to the environments schema for unlimited timeout" do
+    conf_stub = stub 'conf_stub'
+    conf_stub.expects(:environment_timeout).returns(1.0 / 0.0)
     environment = Puppet::Node::Environment.create(:production, [])
-    handler = Puppet::Network::HTTP::API::V2::Environments.new(Puppet::Environments::Static.new(environment))
+    env_loader = Puppet::Environments::Static.new(environment)
+    env_loader.expects(:get_conf).with(:production).returns(conf_stub)
+    handler = Puppet::Network::HTTP::API::V2::Environments.new(env_loader)
     response = Puppet::Network::HTTP::MemoryResponse.new
 
     handler.call(Puppet::Network::HTTP::Request.from_hash(:headers => { 'accept' => 'application/json' }), response)
 
     expect(response.body).to validate_against('api/schemas/environments.json')
   end
+
+  it "the response conforms to the environments schema for integer timeout" do
+    conf_stub = stub 'conf_stub'
+    conf_stub.expects(:environment_timeout).returns(1)
+    environment = Puppet::Node::Environment.create(:production, [])
+    env_loader = Puppet::Environments::Static.new(environment)
+    env_loader.expects(:get_conf).with(:production).returns(conf_stub)
+    handler = Puppet::Network::HTTP::API::V2::Environments.new(env_loader)
+    response = Puppet::Network::HTTP::MemoryResponse.new
+
+    handler.call(Puppet::Network::HTTP::Request.from_hash(:headers => { 'accept' => 'application/json' }), response)
+
+    expect(response.body).to validate_against('api/schemas/environments.json')
+  end
+
 end
diff --git a/spec/unit/network/http/connection_spec.rb b/spec/unit/network/http/connection_spec.rb
index a5e6f64ae..ef6ca65d6 100644..100755
--- a/spec/unit/network/http/connection_spec.rb
+++ b/spec/unit/network/http/connection_spec.rb
@@ -11,50 +11,30 @@ describe Puppet::Network::HTTP::Connection do
   let (:httpok) { Net::HTTPOK.new('1.1', 200, '') }
 
   context "when providing HTTP connections" do
-    after do
-      Puppet::Network::HTTP::Connection.instance_variable_set("@ssl_host", nil)
-    end
-
     context "when initializing http instances" do
-      before :each do
-        # All of the cert stuff is tested elsewhere
-        Puppet::Network::HTTP::Connection.stubs(:cert_setup)
-      end
-
       it "should return an http instance created with the passed host and port" do
-        http = subject.send(:connection)
-        http.should be_an_instance_of Net::HTTP
-        http.address.should == host
-        http.port.should    == port
+        conn = Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator)
+
+        expect(conn.address).to eq(host)
+        expect(conn.port).to eq(port)
       end
 
       it "should enable ssl on the http instance by default" do
-        http = subject.send(:connection)
-        http.should be_use_ssl
-      end
+        conn = Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator)
 
-      it "can set ssl using an option" do
-        Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator).send(:connection).should_not be_use_ssl
-        Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => true, :verify => Puppet::SSL::Validator.no_validator).send(:connection).should be_use_ssl
+        expect(conn).to be_use_ssl
       end
 
-      context "proxy and timeout settings should propagate" do
-        subject { Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator).send(:connection) }
-        before :each do
-          Puppet[:http_proxy_host] = "myhost"
-          Puppet[:http_proxy_port] = 432
-          Puppet[:configtimeout]   = 120
-        end
+      it "can disable ssl using an option" do
+        conn = Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator)
 
-        its(:open_timeout)  { should == Puppet[:configtimeout] }
-        its(:read_timeout)  { should == Puppet[:configtimeout] }
-        its(:proxy_address) { should == Puppet[:http_proxy_host] }
-        its(:proxy_port)    { should == Puppet[:http_proxy_port] }
+        expect(conn).to_not be_use_ssl
       end
 
-      it "should not set a proxy if the value is 'none'" do
-        Puppet[:http_proxy_host] = 'none'
-        subject.send(:connection).proxy_address.should be_nil
+      it "can enable ssl using an option" do
+        conn = Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => true, :verify => Puppet::SSL::Validator.no_validator)
+
+        expect(conn).to be_use_ssl
       end
 
       it "should raise Puppet::Error when invalid options are specified" do
@@ -96,7 +76,44 @@ describe Puppet::Network::HTTP::Connection do
     end
   end
 
-  context "when validating HTTPS requests" do
+  class ConstantErrorValidator
+    def initialize(args)
+      @fails_with = args[:fails_with]
+      @error_string = args[:error_string] || ""
+      @peer_certs = args[:peer_certs] || []
+    end
+
+    def setup_connection(connection)
+      connection.stubs(:start).raises(OpenSSL::SSL::SSLError.new(@fails_with))
+    end
+
+    def peer_certs
+      @peer_certs
+    end
+
+    def verify_errors
+      [@error_string]
+    end
+  end
+
+  class NoProblemsValidator
+    def initialize(cert)
+      @cert = cert
+    end
+
+    def setup_connection(connection)
+    end
+
+    def peer_certs
+      [@cert]
+    end
+
+    def verify_errors
+      []
+    end
+  end
+
+  shared_examples_for 'ssl verifier' do
     include PuppetSpec::Files
 
     let (:host) { "my_server" }
@@ -117,11 +134,11 @@ describe Puppet::Network::HTTP::Connection do
       Puppet[:confdir] = tmpdir('conf')
 
       connection = Puppet::Network::HTTP::Connection.new(
-        host, port,
-        :verify => ConstantErrorValidator.new(
-          :fails_with => 'hostname was not match with server certificate',
-          :peer_certs => [Puppet::SSL::CertificateAuthority.new.generate(
-            'not_my_server', :dns_alt_names => 'foo,bar,baz')]))
+      host, port,
+      :verify => ConstantErrorValidator.new(
+        :fails_with => 'hostname was not match with server certificate',
+        :peer_certs => [Puppet::SSL::CertificateAuthority.new.generate(
+          'not_my_server', :dns_alt_names => 'foo,bar,baz')]))
 
       expect do
         connection.get('request')
@@ -150,86 +167,104 @@ describe Puppet::Network::HTTP::Connection do
         host, port,
         :verify => NoProblemsValidator.new(cert))
 
+      Net::HTTP.any_instance.stubs(:start)
       Net::HTTP.any_instance.stubs(:request).returns(httpok)
 
       connection.expects(:warn_if_near_expiration).with(cert)
 
       connection.get('request')
     end
+  end
 
-    class ConstantErrorValidator
-      def initialize(args)
-        @fails_with = args[:fails_with]
-        @error_string = args[:error_string] || ""
-        @peer_certs = args[:peer_certs] || []
-      end
+  context "when using single use HTTPS connections" do
+    it_behaves_like 'ssl verifier' do
+    end
+  end
 
-      def setup_connection(connection)
-        connection.stubs(:request).with do
-          true
-        end.raises(OpenSSL::SSL::SSLError.new(@fails_with))
+  context "when using persistent HTTPS connections" do
+    around :each do |example|
+      pool = Puppet::Network::HTTP::Pool.new
+      Puppet.override(:http_pool => pool) do
+        example.run
       end
+      pool.close
+    end
 
-      def peer_certs
-        @peer_certs
-      end
+    it_behaves_like 'ssl verifier' do
+    end
+  end
 
-      def verify_errors
-        [@error_string]
-      end
+  context "when response is a redirect" do
+    let (:site)       { Puppet::Network::HTTP::Site.new('http', 'my_server', 8140) }
+    let (:other_site) { Puppet::Network::HTTP::Site.new('http', 'redirected', 9292) }
+    let (:other_path) { "other-path" }
+    let (:verify) { Puppet::SSL::Validator.no_validator }
+    let (:subject) { Puppet::Network::HTTP::Connection.new(site.host, site.port, :use_ssl => false, :verify => verify) }
+    let (:httpredirection) do
+      response = Net::HTTPFound.new('1.1', 302, 'Moved Temporarily')
+      response['location'] = "#{other_site.addr}/#{other_path}"
+      response.stubs(:read_body).returns("This resource has moved")
+      response
     end
 
-    class NoProblemsValidator
-      def initialize(cert)
-        @cert = cert
-      end
+    def create_connection(site, options)
+      options[:use_ssl] = site.use_ssl?
+      Puppet::Network::HTTP::Connection.new(site.host, site.port, options)
+    end
 
-      def setup_connection(connection)
-      end
+    it "should redirect to the final resource location" do
+      http = stub('http')
+      http.stubs(:request).returns(httpredirection).then.returns(httpok)
 
-      def peer_certs
-        [@cert]
-      end
+      seq = sequence('redirection')
+      pool = Puppet.lookup(:http_pool)
+      pool.expects(:with_connection).with(site, anything).yields(http).in_sequence(seq)
+      pool.expects(:with_connection).with(other_site, anything).yields(http).in_sequence(seq)
 
-      def verify_errors
-        []
-      end
+      conn = create_connection(site, :verify => verify)
+      conn.get('/foo')
     end
-  end
 
-  context "when response is a redirect" do
-    let (:other_host) { "redirected" }
-    let (:other_port) { 9292 }
-    let (:other_path) { "other-path" }
-    let (:subject) { Puppet::Network::HTTP::Connection.new("my_server", 8140, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator) }
-    let (:httpredirection) { Net::HTTPFound.new('1.1', 302, 'Moved Temporarily') }
+    def expects_redirection(conn, &block)
+      http = stub('http')
+      http.stubs(:request).returns(httpredirection)
 
-    before :each do
-      httpredirection['location'] = "http://#{other_host}:#{other_port}/#{other_path}"
-      httpredirection.stubs(:read_body).returns("This resource has moved")
+      pool = Puppet.lookup(:http_pool)
+      pool.expects(:with_connection).with(site, anything).yields(http)
+      pool
+    end
 
-      socket = stub_everything("socket")
-      TCPSocket.stubs(:open).returns(socket)
+    def expects_limit_exceeded(conn)
+      expect {
+        conn.get('/')
+      }.to raise_error(Puppet::Network::HTTP::RedirectionLimitExceededException)
+    end
 
-      Net::HTTP::Get.any_instance.stubs(:exec).returns("")
-      Net::HTTP::Post.any_instance.stubs(:exec).returns("")
+    it "should not redirect when the limit is 0" do
+      conn = create_connection(site, :verify => verify, :redirect_limit => 0)
+
+      pool = expects_redirection(conn)
+      pool.expects(:with_connection).with(other_site, anything).never
+
+      expects_limit_exceeded(conn)
     end
 
-    it "should redirect to the final resource location" do
-      httpok.stubs(:read_body).returns(:body)
-      Net::HTTPResponse.stubs(:read_new).returns(httpredirection).then.returns(httpok)
+    it "should redirect only once" do
+      conn = create_connection(site, :verify => verify, :redirect_limit => 1)
+
+      pool = expects_redirection(conn)
+      pool.expects(:with_connection).with(other_site, anything).once
 
-      subject.get("/foo").body.should == :body
-      subject.port.should == other_port
-      subject.address.should == other_host
+      expects_limit_exceeded(conn)
     end
 
-    it "should raise an error after too many redirections" do
-      Net::HTTPResponse.stubs(:read_new).returns(httpredirection)
+    it "should raise an exception when the redirect limit is exceeded" do
+      conn = create_connection(site, :verify => verify, :redirect_limit => 3)
 
-      expect {
-        subject.get("/foo")
-      }.to raise_error(Puppet::Network::HTTP::RedirectionLimitExceededException)
+      pool = expects_redirection(conn)
+      pool.expects(:with_connection).with(other_site, anything).times(3)
+
+      expects_limit_exceeded(conn)
     end
   end
 
diff --git a/spec/unit/network/http/factory_spec.rb b/spec/unit/network/http/factory_spec.rb
new file mode 100755
index 000000000..107ededcd
--- /dev/null
+++ b/spec/unit/network/http/factory_spec.rb
@@ -0,0 +1,82 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+require 'puppet/network/http'
+
+describe Puppet::Network::HTTP::Factory do
+  before :each do
+    Puppet::SSL::Key.indirection.terminus_class = :memory
+    Puppet::SSL::CertificateRequest.indirection.terminus_class = :memory
+  end
+
+  let(:site) { Puppet::Network::HTTP::Site.new('https', 'www.example.com', 443) }
+  def create_connection(site)
+    factory = Puppet::Network::HTTP::Factory.new
+
+    factory.create_connection(site)
+  end
+
+  it 'creates a connection for the site' do
+    conn = create_connection(site)
+
+    expect(conn.use_ssl?).to be_true
+    expect(conn.address).to eq(site.host)
+    expect(conn.port).to eq(site.port)
+  end
+
+  it 'creates a connection that has not yet been started' do
+    conn = create_connection(site)
+
+    expect(conn).to_not be_started
+  end
+
+  it 'creates a connection supporting at least HTTP 1.1' do
+    conn = create_connection(site)
+
+    expect(any_of(conn.class.version_1_1?, conn.class.version_1_1?)).to be_true
+  end
+
+  context "proxy settings" do
+    let(:proxy_host) { 'myhost' }
+    let(:proxy_port) { 432 }
+
+    it "should not set a proxy if the value is 'none'" do
+      Puppet[:http_proxy_host] = 'none'
+      conn = create_connection(site)
+
+      expect(conn.proxy_address).to be_nil
+    end
+
+    it 'sets proxy_address' do
+      Puppet[:http_proxy_host] = proxy_host
+      conn = create_connection(site)
+
+      expect(conn.proxy_address).to eq(proxy_host)
+    end
+
+    it 'sets proxy address and port' do
+      Puppet[:http_proxy_host] = proxy_host
+      Puppet[:http_proxy_port] = proxy_port
+      conn = create_connection(site)
+
+      expect(conn.proxy_port).to eq(proxy_port)
+    end
+
+    context 'socket timeouts' do
+      let(:timeout) { 5 }
+
+      it 'sets open timeout' do
+        Puppet[:configtimeout] = timeout
+        conn = create_connection(site)
+
+        expect(conn.open_timeout).to eq(timeout)
+      end
+
+      it 'sets read timeout' do
+        Puppet[:configtimeout] = timeout
+        conn = create_connection(site)
+
+        expect(conn.read_timeout).to eq(timeout)
+      end
+    end
+  end
+end
diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
index 345818b4a..25df9d270 100755
--- a/spec/unit/network/http/handler_spec.rb
+++ b/spec/unit/network/http/handler_spec.rb
@@ -103,31 +103,30 @@ describe Puppet::Network::HTTP::Handler do
       handler.stubs(:warn_if_near_expiration)
     end
 
-    it "should check the client certificate for upcoming expiration" do
-      request = a_request
-      cert = mock 'cert'
-      handler.expects(:client_cert).returns(cert).with(request)
-      handler.expects(:warn_if_near_expiration).with(cert)
-
-      handler.process(request, response)
-    end
-
     it "should setup a profiler when the puppet-profiling header exists" do
       request = a_request
       request[:headers][Puppet::Network::HTTP::HEADER_ENABLE_PROFILING.downcase] = "true"
 
-      handler.process(request, response)
+      p = HandlerTestProfiler.new
+
+      Puppet::Util::Profiler.expects(:add_profiler).with { |profiler|
+        profiler.is_a? Puppet::Util::Profiler::WallClock
+      }.returns(p)
 
-      Puppet::Util::Profiler.current.should be_kind_of(Puppet::Util::Profiler::WallClock)
+      Puppet::Util::Profiler.expects(:remove_profiler).with { |profiler|
+        profiler == p
+      }
+
+      handler.process(request, response)
     end
 
     it "should not setup profiler when the profile parameter is missing" do
       request = a_request
       request[:params] = { }
 
-      handler.process(request, response)
+      Puppet::Util::Profiler.expects(:add_profiler).never
 
-      Puppet::Util::Profiler.current.should == Puppet::Util::Profiler::NONE
+      handler.process(request, response)
     end
 
     it "should raise an error if the request is formatted in an unknown format" do
@@ -219,4 +218,15 @@ describe Puppet::Network::HTTP::Handler do
       request[:headers] || {}
     end
   end
+
+  class HandlerTestProfiler
+    def start(metric, description)
+    end
+
+    def finish(context, metric, description)
+    end
+
+    def shutdown()
+    end
+  end
 end
diff --git a/spec/unit/network/http/nocache_pool_spec.rb b/spec/unit/network/http/nocache_pool_spec.rb
new file mode 100755
index 000000000..69e2d2e9a
--- /dev/null
+++ b/spec/unit/network/http/nocache_pool_spec.rb
@@ -0,0 +1,43 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+require 'puppet/network/http'
+require 'puppet/network/http/connection'
+
+describe Puppet::Network::HTTP::NoCachePool do
+  let(:site) { Puppet::Network::HTTP::Site.new('https', 'rubygems.org', 443) }
+  let(:verify) { stub('verify', :setup_connection => nil) }
+
+  it 'yields a connection' do
+    http  = stub('http')
+
+    factory = Puppet::Network::HTTP::Factory.new
+    factory.stubs(:create_connection).returns(http)
+    pool = Puppet::Network::HTTP::NoCachePool.new(factory)
+
+    expect { |b|
+      pool.with_connection(site, verify, &b)
+    }.to yield_with_args(http)
+  end
+
+  it 'yields a new connection each time' do
+    http1  = stub('http1')
+    http2  = stub('http2')
+
+    factory = Puppet::Network::HTTP::Factory.new
+    factory.stubs(:create_connection).returns(http1).then.returns(http2)
+    pool = Puppet::Network::HTTP::NoCachePool.new(factory)
+
+    expect { |b|
+      pool.with_connection(site, verify, &b)
+    }.to yield_with_args(http1)
+
+    expect { |b|
+      pool.with_connection(site, verify, &b)
+    }.to yield_with_args(http2)
+  end
+
+  it 'has a close method' do
+    Puppet::Network::HTTP::NoCachePool.new.close
+  end
+end
diff --git a/spec/unit/network/http/pool_spec.rb b/spec/unit/network/http/pool_spec.rb
new file mode 100755
index 000000000..aef100953
--- /dev/null
+++ b/spec/unit/network/http/pool_spec.rb
@@ -0,0 +1,269 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+require 'openssl'
+require 'puppet/network/http'
+require 'puppet/network/http_pool'
+
+describe Puppet::Network::HTTP::Pool do
+  before :each do
+    Puppet::SSL::Key.indirection.terminus_class = :memory
+    Puppet::SSL::CertificateRequest.indirection.terminus_class = :memory
+  end
+
+  let(:site) do
+    Puppet::Network::HTTP::Site.new('https', 'rubygems.org', 443)
+  end
+
+  let(:different_site) do
+    Puppet::Network::HTTP::Site.new('https', 'github.com', 443)
+  end
+
+  let(:verify) do
+    stub('verify', :setup_connection => nil)
+  end
+
+  def create_pool
+    Puppet::Network::HTTP::Pool.new
+  end
+
+  def create_pool_with_connections(site, *connections)
+    pool = Puppet::Network::HTTP::Pool.new
+    connections.each do |conn|
+      pool.release(site, conn)
+    end
+    pool
+  end
+
+  def create_pool_with_expired_connections(site, *connections)
+    # setting keepalive timeout to -1 ensures any newly added
+    # connections have already expired
+    pool = Puppet::Network::HTTP::Pool.new(-1)
+    connections.each do |conn|
+      pool.release(site, conn)
+    end
+    pool
+  end
+
+  def create_connection(site)
+    stub(site.addr, :started? => false, :start => nil, :finish => nil, :use_ssl? => true, :verify_mode => OpenSSL::SSL::VERIFY_PEER)
+  end
+
+  context 'when yielding a connection' do
+    it 'yields a connection' do
+      conn = create_connection(site)
+      pool = create_pool_with_connections(site, conn)
+
+      expect { |b|
+        pool.with_connection(site, verify, &b)
+      }.to yield_with_args(conn)
+    end
+
+    it 'returns the connection to the pool' do
+      conn = create_connection(site)
+      pool = create_pool
+      pool.release(site, conn)
+
+      pool.with_connection(site, verify) { |c| }
+
+      expect(pool.pool[site].first.connection).to eq(conn)
+    end
+
+    it 'can yield multiple connections to the same site' do
+      lru_conn = create_connection(site)
+      mru_conn = create_connection(site)
+      pool = create_pool_with_connections(site, lru_conn, mru_conn)
+
+      pool.with_connection(site, verify) do |a|
+        expect(a).to eq(mru_conn)
+
+        pool.with_connection(site, verify) do |b|
+          expect(b).to eq(lru_conn)
+        end
+      end
+    end
+
+    it 'propagates exceptions' do
+      conn = create_connection(site)
+      pool = create_pool
+      pool.release(site, conn)
+
+      expect {
+        pool.with_connection(site, verify) do |c|
+          raise IOError, 'connection reset'
+        end
+      }.to raise_error(IOError, 'connection reset')
+    end
+
+    it 'does not re-cache connections when an error occurs' do
+      # we're not distinguishing between network errors that would
+      # suggest we close the socket, and other errors
+      conn = create_connection(site)
+      pool = create_pool
+      pool.release(site, conn)
+
+      pool.expects(:release).with(site, conn).never
+
+      pool.with_connection(site, verify) do |c|
+        raise IOError, 'connection reset'
+      end rescue nil
+    end
+
+    context 'when releasing connections' do
+      it 'releases HTTP connections' do
+        conn = create_connection(site)
+        conn.expects(:use_ssl?).returns(false)
+
+        pool = create_pool_with_connections(site, conn)
+        pool.expects(:release).with(site, conn)
+
+        pool.with_connection(site, verify) {|c| }
+      end
+
+      it 'releases secure HTTPS connections' do
+        conn = create_connection(site)
+        conn.expects(:use_ssl?).returns(true)
+        conn.expects(:verify_mode).returns(OpenSSL::SSL::VERIFY_PEER)
+
+        pool = create_pool_with_connections(site, conn)
+        pool.expects(:release).with(site, conn)
+
+        pool.with_connection(site, verify) {|c| }
+      end
+
+      it 'closes insecure HTTPS connections' do
+        conn = create_connection(site)
+        conn.expects(:use_ssl?).returns(true)
+        conn.expects(:verify_mode).returns(OpenSSL::SSL::VERIFY_NONE)
+
+        pool = create_pool_with_connections(site, conn)
+
+        pool.expects(:release).with(site, conn).never
+
+        pool.with_connection(site, verify) {|c| }
+      end
+    end
+  end
+
+  context 'when borrowing' do
+    it 'returns a new connection if the pool is empty' do
+      conn = create_connection(site)
+      pool = create_pool
+      pool.factory.expects(:create_connection).with(site).returns(conn)
+
+      expect(pool.borrow(site, verify)).to eq(conn)
+    end
+
+    it 'returns a matching connection' do
+      conn = create_connection(site)
+      pool = create_pool_with_connections(site, conn)
+
+      pool.factory.expects(:create_connection).never
+
+      expect(pool.borrow(site, verify)).to eq(conn)
+    end
+
+    it 'returns a new connection if there are no matching sites' do
+      different_conn = create_connection(different_site)
+      pool = create_pool_with_connections(different_site, different_conn)
+
+      conn = create_connection(site)
+      pool.factory.expects(:create_connection).with(site).returns(conn)
+
+      expect(pool.borrow(site, verify)).to eq(conn)
+    end
+
+    it 'returns started connections' do
+      conn = create_connection(site)
+      conn.expects(:start)
+
+      pool = create_pool
+      pool.factory.expects(:create_connection).with(site).returns(conn)
+
+      expect(pool.borrow(site, verify)).to eq(conn)
+    end
+
+    it "doesn't start a cached connection" do
+      conn = create_connection(site)
+      conn.expects(:start).never
+
+      pool = create_pool_with_connections(site, conn)
+      pool.borrow(site, verify)
+    end
+
+    it 'returns the most recently used connection from the pool' do
+      least_recently_used = create_connection(site)
+      most_recently_used = create_connection(site)
+
+      pool = create_pool_with_connections(site, least_recently_used, most_recently_used)
+      expect(pool.borrow(site, verify)).to eq(most_recently_used)
+    end
+
+    it 'finishes expired connections' do
+      conn = create_connection(site)
+      conn.expects(:finish)
+
+      pool = create_pool_with_expired_connections(site, conn)
+      pool.factory.expects(:create_connection => stub('conn', :start => nil))
+
+      pool.borrow(site, verify)
+    end
+
+    it 'logs an exception if it fails to close an expired connection' do
+      Puppet.expects(:log_exception).with(is_a(IOError), "Failed to close connection for #{site}: read timeout")
+
+      conn = create_connection(site)
+      conn.expects(:finish).raises(IOError, 'read timeout')
+
+      pool = create_pool_with_expired_connections(site, conn)
+      pool.factory.expects(:create_connection => stub('open_conn', :start => nil))
+
+      pool.borrow(site, verify)
+    end
+  end
+
+  context 'when releasing a connection' do
+    it 'adds the connection to an empty pool' do
+      conn = create_connection(site)
+
+      pool = create_pool
+      pool.release(site, conn)
+
+      expect(pool.pool[site].first.connection).to eq(conn)
+    end
+
+    it 'adds the connection to a pool with a connection for the same site' do
+      pool = create_pool
+      pool.release(site, create_connection(site))
+      pool.release(site, create_connection(site))
+
+      expect(pool.pool[site].count).to eq(2)
+    end
+
+    it 'adds the connection to a pool with a connection for a different site' do
+      pool = create_pool
+      pool.release(site, create_connection(site))
+      pool.release(different_site, create_connection(different_site))
+
+      expect(pool.pool[site].count).to eq(1)
+      expect(pool.pool[different_site].count).to eq(1)
+    end
+  end
+
+  context 'when closing' do
+    it 'clears the pool' do
+      pool = create_pool
+      pool.close
+
+      expect(pool.pool).to be_empty
+    end
+
+    it 'closes all cached connections' do
+      conn = create_connection(site)
+      conn.expects(:finish)
+
+      pool = create_pool_with_connections(site, conn)
+      pool.close
+    end
+  end
+end
diff --git a/spec/unit/network/http/rack/rest_spec.rb b/spec/unit/network/http/rack/rest_spec.rb
index 165b6ceb9..c35b789a2 100755
--- a/spec/unit/network/http/rack/rest_spec.rb
+++ b/spec/unit/network/http/rack/rest_spec.rb
@@ -12,11 +12,11 @@ describe "Puppet::Network::HTTP::RackREST", :if => Puppet.features.rack? do
     before :all do
       @model_class = stub('indirected model class')
       Puppet::Indirector::Indirection.stubs(:model).with(:foo).returns(@model_class)
-      @handler = Puppet::Network::HTTP::RackREST.new(:handler => :foo)
     end
 
     before :each do
       @response = Rack::Response.new
+      @handler = Puppet::Network::HTTP::RackREST.new(:handler => :foo)
     end
 
     def mk_req(uri, opts = {})
diff --git a/spec/unit/network/http/session_spec.rb b/spec/unit/network/http/session_spec.rb
new file mode 100755
index 000000000..4eba67d7d
--- /dev/null
+++ b/spec/unit/network/http/session_spec.rb
@@ -0,0 +1,43 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+require 'puppet/network/http'
+
+describe Puppet::Network::HTTP::Session do
+  let(:connection) { stub('connection') }
+
+  def create_session(connection, expiration_time = nil)
+    expiration_time ||= Time.now + 60 * 60
+
+    Puppet::Network::HTTP::Session.new(connection, expiration_time)
+  end
+
+  it 'provides access to its connection' do
+    session = create_session(connection)
+
+    session.connection.should == connection
+  end
+
+  it 'expires a connection whose expiration time is in the past' do
+    now = Time.now
+    past = now - 1
+
+    session = create_session(connection, past)
+    session.expired?(now).should be_true
+  end
+
+  it 'expires a connection whose expiration time is now' do
+    now = Time.now
+
+    session = create_session(connection, now)
+    session.expired?(now).should be_true
+  end
+
+  it 'does not expire a connection whose expiration time is in the future' do
+    now = Time.now
+    future = now + 1
+
+    session = create_session(connection, future)
+    session.expired?(now).should be_false
+  end
+end
diff --git a/spec/unit/network/http/site_spec.rb b/spec/unit/network/http/site_spec.rb
new file mode 100755
index 000000000..06fcbf83d
--- /dev/null
+++ b/spec/unit/network/http/site_spec.rb
@@ -0,0 +1,90 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+
+require 'puppet/network/http'
+
+describe Puppet::Network::HTTP::Site do
+  let(:scheme)      { 'https' }
+  let(:host)        { 'rubygems.org' }
+  let(:port)        { 443 }
+
+  def create_site(scheme, host, port)
+    Puppet::Network::HTTP::Site.new(scheme, host, port)
+  end
+
+  it 'accepts scheme, host, and port' do
+    site = create_site(scheme, host, port)
+
+    expect(site.scheme).to eq(scheme)
+    expect(site.host).to eq(host)
+    expect(site.port).to eq(port)
+  end
+
+  it 'generates an external URI string' do
+    site = create_site(scheme, host, port)
+
+    expect(site.addr).to eq("https://rubygems.org:443")
+  end
+
+  it 'considers sites to be different when the scheme is different' do
+    https_site = create_site('https', host, port)
+    http_site = create_site('http', host, port)
+
+    expect(https_site).to_not eq(http_site)
+  end
+
+  it 'considers sites to be different when the host is different' do
+    rubygems_site = create_site(scheme, 'rubygems.org', port)
+    github_site = create_site(scheme, 'github.com', port)
+
+    expect(rubygems_site).to_not eq(github_site)
+  end
+
+  it 'considers sites to be different when the port is different' do
+    site_443 = create_site(scheme, host, 443)
+    site_80 = create_site(scheme, host, 80)
+
+    expect(site_443).to_not eq(site_80)
+  end
+
+  it 'compares values when determining equality' do
+    site = create_site(scheme, host, port)
+
+    sites = {}
+    sites[site] = site
+
+    another_site = create_site(scheme, host, port)
+
+    expect(sites.include?(another_site)).to be_true
+  end
+
+  it 'computes the same hash code for equivalent objects' do
+    site = create_site(scheme, host, port)
+    same_site = create_site(scheme, host, port)
+
+    expect(site.hash).to eq(same_site.hash)
+  end
+
+  it 'uses ssl with https' do
+    site = create_site('https', host, port)
+
+    expect(site).to be_use_ssl
+  end
+
+  it 'does not use ssl with http' do
+    site = create_site('http', host, port)
+
+    expect(site).to_not be_use_ssl
+  end
+
+  it 'moves to a new URI location' do
+    site = create_site('http', 'host1', 80)
+
+    uri = URI.parse('https://host2:443/some/where/else')
+    new_site = site.move_to(uri)
+
+    expect(new_site.scheme).to eq('https')
+    expect(new_site.host).to eq('host2')
+    expect(new_site.port).to eq(443)
+  end
+end
diff --git a/spec/unit/network/http/webrick_spec.rb b/spec/unit/network/http/webrick_spec.rb
index 17f61e339..edeb439a9 100755
--- a/spec/unit/network/http/webrick_spec.rb
+++ b/spec/unit/network/http/webrick_spec.rb
@@ -127,7 +127,7 @@ describe Puppet::Network::HTTP::WEBrick do
       server.setup_logger
     end
 
-    it "should use the masterlog if the run_mode is master" do
+    it "should use the masterhttplog if the run_mode is master" do
       Puppet.run_mode.stubs(:master?).returns(true)
       log = make_absolute("/master/log")
       Puppet[:masterhttplog] = log
diff --git a/spec/unit/network/http_pool_spec.rb b/spec/unit/network/http_pool_spec.rb
index d8b84232e..a9c5783f2 100755
--- a/spec/unit/network/http_pool_spec.rb
+++ b/spec/unit/network/http_pool_spec.rb
@@ -9,7 +9,6 @@ describe Puppet::Network::HttpPool do
   end
 
   describe "when managing http instances" do
-
     it "should return an http instance created with the passed host and port" do
       http = Puppet::Network::HttpPool.http_instance("me", 54321)
       http.should be_an_instance_of Puppet::Network::HTTP::Connection
@@ -47,7 +46,6 @@ describe Puppet::Network::HttpPool do
       Puppet::Network::HttpPool.http_instance("me", 54321, true).should be_use_ssl
     end
 
-
     describe 'peer verification' do
       def setup_standard_ssl_configuration
         ca_cert_file = File.expand_path('/path/to/ssl/certs/ca_cert.pem')
@@ -77,12 +75,18 @@ describe Puppet::Network::HttpPool do
         setup_standard_ssl_host
       end
 
-      it 'can enable peer verification' do
-        Puppet::Network::HttpPool.http_instance("me", 54321, true, true).send(:connection).verify_mode.should == OpenSSL::SSL::VERIFY_PEER
+      it 'enables peer verification by default' do
+        response = Net::HTTPOK.new('1.1', 200, 'body')
+        conn = Puppet::Network::HttpPool.http_instance("me", 54321, true)
+        conn.expects(:execute_request).with { |http, request| expect(http.verify_mode).to eq(OpenSSL::SSL::VERIFY_PEER) }.returns(response)
+        conn.get('/')
       end
 
       it 'can disable peer verification' do
-        Puppet::Network::HttpPool.http_instance("me", 54321, true, false).send(:connection).verify_mode.should == OpenSSL::SSL::VERIFY_NONE
+        response = Net::HTTPOK.new('1.1', 200, 'body')
+        conn = Puppet::Network::HttpPool.http_instance("me", 54321, true, false)
+        conn.expects(:execute_request).with { |http, request| expect(http.verify_mode).to eq(OpenSSL::SSL::VERIFY_NONE) }.returns(response)
+        conn.get('/')
       end
     end
 
@@ -91,5 +95,4 @@ describe Puppet::Network::HttpPool do
         should_not equal(Puppet::Network::HttpPool.http_instance("me", 54321))
     end
   end
-
 end
diff --git a/spec/unit/network/http_spec.rb b/spec/unit/network/http_spec.rb
new file mode 100755
index 000000000..4a149d3a8
--- /dev/null
+++ b/spec/unit/network/http_spec.rb
@@ -0,0 +1,10 @@
+#! /usr/bin/env ruby
+require 'spec_helper'
+require 'puppet/network/http'
+
+describe Puppet::Network::HTTP do
+  it 'defines an http_pool context' do
+    pool = Puppet.lookup(:http_pool)
+    expect(pool).to be_a(Puppet::Network::HTTP::NoCachePool)
+  end
+end