Merge pull request #7197 from straight-shoota/jm/fix/http-server-specs

Fix leaks in HTTP::Server specs
crystal-lang · Apr 1, 2019 · b006e84 · b006e84
2 parents 05e2a10 + 2f07b56
commit b006e84
Show file tree

Hide file tree

Showing 6 changed files with 181 additions and 162 deletions.
diff --git a/spec/std/http/client/client_spec.cr b/spec/std/http/client/client_spec.cr
@@ -1,4 +1,4 @@
-require "spec"
+require "../spec_helper"
 require "openssl"
 require "http/client"
 require "http/server"
@@ -132,37 +132,34 @@ module HTTP
         context.response.print context.request.headers["Host"]
       end
       address = server.bind_unused_port "::1"
-      spawn { server.listen }
 
-      HTTP::Client.get("http://[::1]:#{address.port}/").body.should eq("[::1]:#{address.port}")
-
-      server.close
+      run_server(server) do
+        HTTP::Client.get("http://[::1]:#{address.port}/").body.should eq("[::1]:#{address.port}")
+      end
     end
 
     it "sends a 'connection: close' header on one-shot request" do
       server = HTTP::Server.new do |context|
         context.response.print context.request.headers["connection"]
       end
       address = server.bind_unused_port "::1"
-      spawn { server.listen }
-
-      HTTP::Client.get("http://[::1]:#{address.port}/").body.should eq("close")
 
-      server.close
+      run_server(server) do
+        HTTP::Client.get("http://[::1]:#{address.port}/").body.should eq("close")
+      end
     end
 
     it "sends a 'connection: close' header on one-shot request with block" do
       server = HTTP::Server.new do |context|
         context.response.print context.request.headers["connection"]
       end
       address = server.bind_unused_port "::1"
-      spawn { server.listen }
-
-      HTTP::Client.get("http://[::1]:#{address.port}/") do |response|
-        response.body_io.gets_to_end
-      end.should eq("close")
 
-      server.close
+      run_server(server) do
+        HTTP::Client.get("http://[::1]:#{address.port}/") do |response|
+          response.body_io.gets_to_end
+        end.should eq("close")
+      end
     end
 
     it "doesn't read the body if request was HEAD" do

diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr
@@ -1,20 +1,8 @@
-require "../../spec_helper"
+require "../spec_helper"
 require "http/server"
 require "http/client/response"
 require "../../../support/ssl"
 
-private def wait_for(timeout = 5.seconds)
-  now = Time.monotonic
-
-  until yield
-    Fiber.yield
-
-    if (Time.monotonic - now) > timeout
-      raise "server failed to start within 5 seconds"
-    end
-  end
-end
-
 private class RaiseErrno < IO
   def initialize(@value : Int32)
   end
@@ -237,9 +225,13 @@ module HTTP
       address = server.bind_unused_port
       address.port.should_not eq(0)
 
+      server.close
+
       server = Server.new { |ctx| }
       port = server.bind_tcp(0).port
       port.should_not eq(0)
+    ensure
+      server.close if server
     end
 
     it "doesn't raise on accept after close #2692" do
@@ -278,11 +270,11 @@ module HTTP
 
       address1.should_not eq address2
 
-      spawn { server.listen }
-
-      HTTP::Client.get("http://#{address2}/").body.should eq "Test Server (#{address2})"
-      HTTP::Client.get("http://#{address1}/").body.should eq "Test Server (#{address1})"
-      HTTP::Client.get("http://#{address1}/").body.should eq "Test Server (#{address1})"
+      run_server(server) do
+        HTTP::Client.get("http://#{address2}/").body.should eq "Test Server (#{address2})"
+        HTTP::Client.get("http://#{address1}/").body.should eq "Test Server (#{address1})"
+        HTTP::Client.get("http://#{address1}/").body.should eq "Test Server (#{address1})"
+      end
     end
 
     it "handles Expect: 100-continue correctly when body is read" do
@@ -291,30 +283,29 @@ module HTTP
       end
 
       address = server.bind_unused_port
-      spawn server.listen
 
-      wait_for { server.listening? }
+      run_server(server) do
+        TCPSocket.open(address.address, address.port) do |socket|
+          socket << requestize(<<-REQUEST
+            POST / HTTP/1.1
+            Expect: 100-continue
+            Content-Length: 5
 
-      TCPSocket.open(address.address, address.port) do |socket|
-        socket << requestize(<<-REQUEST
-          POST / HTTP/1.1
-          Expect: 100-continue
-          Content-Length: 5
+            REQUEST
+          )
+          socket << "\r\n"
+          socket.flush
 
-          REQUEST
-        )
-        socket << "\r\n"
-        socket.flush
+          response = Client::Response.from_io(socket)
+          response.status_code.should eq(100)
 
-        response = Client::Response.from_io(socket)
-        response.status_code.should eq(100)
+          socket << "hello"
+          socket.flush
 
-        socket << "hello"
-        socket.flush
-
-        response = Client::Response.from_io(socket)
-        response.status_code.should eq(200)
-        response.body.should eq("hello")
+          response = Client::Response.from_io(socket)
+          response.status_code.should eq(200)
+          response.body.should eq("hello")
+        end
       end
     end
 
@@ -324,24 +315,23 @@ module HTTP
       end
 
       address = server.bind_unused_port
-      spawn server.listen
-
-      wait_for { server.listening? }
 
-      TCPSocket.open(address.address, address.port) do |socket|
-        socket << requestize(<<-REQUEST
-          POST / HTTP/1.1
-          Expect: 100-continue
-          Content-Length: 5
-
-          REQUEST
-        )
-        socket << "\r\n"
-        socket.flush
-
-        response = Client::Response.from_io(socket)
-        response.status_code.should eq(400)
-        response.body.should eq("400 I don't want your body\n")
+      run_server(server) do
+        TCPSocket.open(address.address, address.port) do |socket|
+          socket << requestize(<<-REQUEST
+            POST / HTTP/1.1
+            Expect: 100-continue
+            Content-Length: 5
+
+            REQUEST
+          )
+          socket << "\r\n"
+          socket.flush
+
+          response = Client::Response.from_io(socket)
+          response.status_code.should eq(400)
+          response.body.should eq("400 I don't want your body\n")
+        end
       end
     end
 
@@ -353,30 +343,33 @@ module HTTP
       server.bind tcp_server
 
       server.addresses.should eq addresses
+    ensure
+      server.try &.close
     end
 
     describe "#bind" do
       it "fails after listen" do
         server = Server.new { }
         server.bind_unused_port
-        spawn { server.listen }
-        wait_for { server.listening? }
-        expect_raises(Exception, "Can't add socket to running server") do
-          server.bind_unused_port
+
+        run_server(server) do
+          expect_raises(Exception, "Can't add socket to running server") do
+            server.bind_unused_port
+          end
         end
-        server.close
       end
 
       it "fails after close" do
         server = Server.new { }
         server.bind_unused_port
-        spawn { server.listen }
-        wait_for { server.listening? }
-        server.close
-        expect_raises(Exception, "Can't add socket to closed server") do
-          server.bind_unused_port
+
+        run_server(server) do
+          server.close
+
+          expect_raises(Exception, "Can't add socket to closed server") do
+            server.bind_unused_port
+          end
         end
-        server.close unless server.closed?
       end
 
       describe "with URI" do
@@ -482,36 +475,36 @@ module HTTP
         ip_address1 = server.bind_tls "127.0.0.1", 0, server_context
         ip_address2 = socket.local_address
 
-        spawn server.listen
-
-        HTTP::Client.get("https://#{ip_address1}", tls: client_context).body.should eq "Test Server (#{ip_address1})\n"
-        HTTP::Client.get("https://#{ip_address2}", tls: client_context).body.should eq "Test Server (#{ip_address2})\n"
-
-        server.close
+        run_server(server) do
+          HTTP::Client.get("https://#{ip_address1}", tls: client_context).body.should eq "Test Server (#{ip_address1})\n"
+          HTTP::Client.get("https://#{ip_address2}", tls: client_context).body.should eq "Test Server (#{ip_address2})\n"
+        end
       end
     end
 
     describe "#listen" do
       it "fails after listen" do
         server = Server.new { }
         server.bind_unused_port
-        spawn { server.listen }
-        wait_for { server.listening? }
-        expect_raises(Exception, "Can't start running server") do
-          server.listen
+
+        run_server(server) do
+          expect_raises(Exception, "Can't start running server") do
+            server.listen
+          end
         end
-        server.close
       end
 
       it "fails after close" do
         server = Server.new { }
         server.bind_unused_port
-        spawn { server.listen }
-        wait_for { server.listening? }
-        server.close
-        server.listening?.should be_false
-        expect_raises(Exception, "Can't re-start closed server") do
-          server.listen
+
+        run_server(server) do
+          server.close
+          server.listening?.should be_false
+
+          expect_raises(Exception, "Can't re-start closed server") do
+            server.listen
+          end
         end
       end
     end
@@ -533,13 +526,10 @@ module HTTP
             server.bind socket1
             socket2 = server.bind_unix path2
 
-            spawn server.listen
-            wait_for { server.listening? }
-
-            unix_request(path1).should eq "Test Server (#{path1})"
-            unix_request(path2).should eq "Test Server (#{path2})"
-
-            server.close
+            run_server(server) do
+              unix_request(path1).should eq "Test Server (#{path1})"
+              unix_request(path2).should eq "Test Server (#{path2})"
+            end
 
             File.exists?(path1).should be_false
             File.exists?(path2).should be_false
@@ -560,27 +550,25 @@ module HTTP
       server_context, client_context = ssl_context_pair
       address = server.bind_tls "localhost", server_context
 
-      server_done = false
-      spawn do
-        server.listen
-      ensure
-        server_done = true
-      end
-
-      3.times do
-        # Perform multiple wrong calls together and check
-        # that the server is still able to respond.
+      run_server(server) do |server_done|
         3.times do
-          empty_context = OpenSSL::SSL::Context::Client.new
-          socket = TCPSocket.new(address.address, address.port)
-          expect_raises(OpenSSL::SSL::Error) do
-            OpenSSL::SSL::Socket::Client.new(socket, empty_context)
+          # Perform multiple wrong calls together and check
+          # that the server is still able to respond.
+          3.times do
+            empty_context = OpenSSL::SSL::Context::Client.new
+            TCPSocket.open(address.address, address.port) do |socket|
+              expect_raises(OpenSSL::SSL::Error) do
+                OpenSSL::SSL::Socket::Client.new(socket, empty_context)
+              end
+            end
           end
+
+          HTTP::Client.get("https://#{address}/", tls: client_context).body.should eq "ok"
         end
-        HTTP::Client.get("https://#{address}/", tls: client_context).body.should eq "ok"
-      end
 
-      server_done.should be_false
+        server.closed?.should be_false
+        server_done.empty?.should be_true
+      end
     end
 
     describe "#close" do
@@ -594,24 +582,25 @@ module HTTP
         end
 
         address = server.bind_unused_port
-        spawn server.listen
 
-        TCPSocket.open(address.address, address.port) do |socket|
-          socket << "GET / HTTP/1.1\r\n\r\n"
+        run_server(server) do
+          TCPSocket.open(address.address, address.port) do |socket|
+            socket << "GET / HTTP/1.1\r\n\r\n"
 
-          while true
-            line = socket.gets || break
-            break if line.empty?
-          end
+            while true
+              line = socket.gets || break
+              break if line.empty?
+            end
 
-          socket = HTTP::ChunkedContent.new(socket)
+            socket = HTTP::ChunkedContent.new(socket)
 
-          socket.gets.should eq "foo"
+            socket.gets.should eq "foo"
 
-          server.close
+            server.close
 
-          socket.closed?.should be_false
-          socket.gets.should eq "bar"
+            socket.closed?.should be_false
+            socket.gets.should eq "bar"
+          end
         end
       end
     end