Support HTTP server and client request streaming (single HTTP::Reques…

…t) (#3406)
crystal-lang · Oct 20, 2016 · 86e80d0 · 86e80d0
1 parent 9cdc89e
commit 86e80d0
Showing 8 changed files with 189 additions and 53 deletions.
diff --git a/spec/std/http/client/client_spec.cr b/spec/std/http/client/client_spec.cr
@@ -47,6 +47,10 @@ module HTTP
     typeof(Client.get(URI.parse("http://www.example.com")))
     typeof(Client.get(URI.parse("http://www.example.com")))
     typeof(Client.get("http://www.example.com"))
+    typeof(Client.post("http://www.example.com", body: MemoryIO.new))
+    typeof(Client.new("host").post("/", body: MemoryIO.new))
+    typeof(Client.post("http://www.example.com", body: Bytes[65]))
+    typeof(Client.new("host").post("/", body: Bytes[65]))
 
     describe "from URI" do
       it "has sane defaults" do

diff --git a/spec/std/http/request_spec.cr b/spec/std/http/request_spec.cr
@@ -74,6 +74,49 @@ module HTTP
       io.to_s.should eq("POST / HTTP/1.1\r\nContent-Length: 13\r\n\r\nthisisthebody")
     end
 
+    it "serialize POST (with bytes body)" do
+      request = Request.new "POST", "/", body: Bytes['a'.ord, 'b'.ord]
+      io = MemoryIO.new
+      request.to_io(io)
+      io.to_s.should eq("POST / HTTP/1.1\r\nContent-Length: 2\r\n\r\nab")
+    end
+
+    it "serialize POST (with io body, without content-length header)" do
+      request = Request.new "POST", "/", body: MemoryIO.new("thisisthebody")
+      io = MemoryIO.new
+      request.to_io(io)
+      io.to_s.should eq("POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\nd\r\nthisisthebody\r\n0\r\n\r\n")
+    end
+
+    it "serialize POST (with io body, with content-length header)" do
+      string = "thisisthebody"
+      request = Request.new "POST", "/", body: MemoryIO.new(string)
+      request.content_length = string.bytesize
+      io = MemoryIO.new
+      request.to_io(io)
+      io.to_s.should eq("POST / HTTP/1.1\r\nContent-Length: 13\r\n\r\nthisisthebody")
+    end
+
+    it "raises if serializing POST body with incorrect content-length (less then real)" do
+      string = "thisisthebody"
+      request = Request.new "POST", "/", body: MemoryIO.new(string)
+      request.content_length = string.bytesize - 1
+      io = MemoryIO.new
+      expect_raises(ArgumentError) do
+        request.to_io(io)
+      end
+    end
+
+    it "raises if serializing POST body with incorrect content-length (more then real)" do
+      string = "thisisthebody"
+      request = Request.new "POST", "/", body: MemoryIO.new(string)
+      request.content_length = string.bytesize + 1
+      io = MemoryIO.new
+      expect_raises(ArgumentError) do
+        request.to_io(io)
+      end
+    end
+
     it "parses GET" do
       request = Request.from_io(MemoryIO.new("GET / HTTP/1.1\r\nHost: host.example.org\r\n\r\n")).as(Request)
       request.method.should eq("GET")
@@ -125,7 +168,7 @@ module HTTP
       request.method.should eq("POST")
       request.path.should eq("/foo")
       request.headers.should eq({"Content-Length" => "13"})
-      request.body.should eq("thisisthebody")
+      request.body.not_nil!.gets_to_end.should eq("thisisthebody")
     end
 
     it "handles malformed request" do

diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr
@@ -245,6 +245,44 @@ module HTTP
       ))
     end
 
+    it "skips body between requests" do
+      processor = HTTP::Server::RequestProcessor.new do |context|
+        context.response.content_type = "text/plain"
+        context.response.puts "Hello world\r"
+      end
+
+      input = MemoryIO.new(requestize(<<-REQUEST
+        POST / HTTP/1.1
+        Content-Length: 7
+
+        hello
+        POST / HTTP/1.1
+        Content-Length: 7
+
+        hello
+        REQUEST
+      ))
+      output = MemoryIO.new
+      processor.process(input, output)
+      output.rewind
+      output.gets_to_end.should eq(requestize(<<-RESPONSE
+        HTTP/1.1 200 OK
+        Connection: keep-alive
+        Content-Type: text/plain
+        Content-Length: 13
+
+        Hello world
+        HTTP/1.1 200 OK
+        Connection: keep-alive
+        Content-Type: text/plain
+        Content-Length: 13
+
+        Hello world
+
+        RESPONSE
+      ))
+    end
+
     it "handles Errno" do
       processor = HTTP::Server::RequestProcessor.new { }
       input = RaiseErrno.new(Errno::ECONNRESET)

diff --git a/src/http/client.cr b/src/http/client.cr
@@ -53,6 +53,9 @@
 # of the returned IO (or used for creating a String for the body). Invalid bytes in the given encoding
 # are silently ignored when reading text content.
 class HTTP::Client
+  # The set of possible valid body types
+  alias BodyType = String | Bytes | IO | Nil
+
   # Returns the target host.
   #
   # ```
@@ -300,7 +303,7 @@ class HTTP::Client
     # response = client.{{method.id}}("/", headers: HTTP::Headers{"User-agent" => "AwesomeApp"}, body: "Hello!")
     # response.body #=> "..."
     # ```
-    def {{method.id}}(path, headers : HTTP::Headers? = nil, body : String? = nil) : HTTP::Client::Response
+    def {{method.id}}(path, headers : HTTP::Headers? = nil, body : BodyType = nil) : HTTP::Client::Response
       exec {{method.upcase}}, path, headers, body
     end
 
@@ -313,7 +316,7 @@ class HTTP::Client
     #   response.body_io.gets #=> "..."
     # end
     # ```
-    def {{method.id}}(path, headers : HTTP::Headers? = nil, body : String? = nil)
+    def {{method.id}}(path, headers : HTTP::Headers? = nil, body : BodyType = nil)
       exec {{method.upcase}}, path, headers, body do |response|
         yield response
       end
@@ -326,7 +329,7 @@ class HTTP::Client
     # response = HTTP::Client.{{method.id}}("/", headers: HTTP::Headers{"User-agent" => "AwesomeApp"}, body: "Hello!")
     # response.body #=> "..."
     # ```
-    def self.{{method.id}}(url : String | URI, headers : HTTP::Headers? = nil, body : String? = nil, tls = nil) : HTTP::Client::Response
+    def self.{{method.id}}(url : String | URI, headers : HTTP::Headers? = nil, body : BodyType = nil, tls = nil) : HTTP::Client::Response
       exec {{method.upcase}}, url, headers, body, tls
     end
 
@@ -338,7 +341,7 @@ class HTTP::Client
     #   response.body_io.gets #=> "..."
     # end
     # ```
-    def self.{{method.id}}(url : String | URI, headers : HTTP::Headers? = nil, body : String? = nil, tls = nil)
+    def self.{{method.id}}(url : String | URI, headers : HTTP::Headers? = nil, body : BodyType = nil, tls = nil)
       exec {{method.upcase}}, url, headers, body, tls do |response|
         yield response
       end
@@ -352,7 +355,7 @@ class HTTP::Client
   # client = HTTP::Client.new "www.example.com"
   # response = client.post_form "/", "foo=bar"
   # ```
-  def post_form(path, form : String, headers : HTTP::Headers? = nil) : HTTP::Client::Response
+  def post_form(path, form : String | IO, headers : HTTP::Headers? = nil) : HTTP::Client::Response
     request = new_request("POST", path, headers, form)
     request.headers["Content-type"] = "application/x-www-form-urlencoded"
     exec request
@@ -368,7 +371,7 @@ class HTTP::Client
   #   response.body_io.gets
   # end
   # ```
-  def post_form(path, form : String, headers : HTTP::Headers? = nil)
+  def post_form(path, form : String | IO, headers : HTTP::Headers? = nil)
     request = new_request("POST", path, headers, form)
     request.headers["Content-type"] = "application/x-www-form-urlencoded"
     exec(request) do |response|
@@ -411,7 +414,7 @@ class HTTP::Client
   # ```
   # response = HTTP::Client.post_form "http://www.example.com", "foo=bar"
   # ```
-  def self.post_form(url, form : String | Hash, headers : HTTP::Headers? = nil, tls = nil) : HTTP::Client::Response
+  def self.post_form(url, form : String | IO | Hash, headers : HTTP::Headers? = nil, tls = nil) : HTTP::Client::Response
     exec(url, tls) do |client, path|
       client.post_form(path, form, headers)
     end
@@ -426,7 +429,7 @@ class HTTP::Client
   #   response.body_io.gets
   # end
   # ```
-  def self.post_form(url, form : String | Hash, headers : HTTP::Headers? = nil, tls = nil)
+  def self.post_form(url, form : String | IO | Hash, headers : HTTP::Headers? = nil, tls = nil)
     exec(url, tls) do |client, path|
       client.post_form(path, form, headers) do |response|
         yield response
@@ -506,7 +509,7 @@ class HTTP::Client
   # response = client.exec "GET", "/"
   # response.body # => "..."
   # ```
-  def exec(method : String, path, headers : HTTP::Headers? = nil, body : String? = nil) : HTTP::Client::Response
+  def exec(method : String, path, headers : HTTP::Headers? = nil, body : BodyType = nil) : HTTP::Client::Response
     exec new_request method, path, headers, body
   end
 
@@ -519,7 +522,7 @@ class HTTP::Client
   #   response.body_io.gets # => "..."
   # end
   # ```
-  def exec(method : String, path, headers : HTTP::Headers? = nil, body : String? = nil)
+  def exec(method : String, path, headers : HTTP::Headers? = nil, body : BodyType = nil)
     exec(new_request(method, path, headers, body)) do |response|
       yield response
     end
@@ -532,7 +535,7 @@ class HTTP::Client
   # response = HTTP::Client.exec "GET", "http://www.example.com"
   # response.body # => "..."
   # ```
-  def self.exec(method, url : String | URI, headers : HTTP::Headers? = nil, body : String? = nil, tls = nil) : HTTP::Client::Response
+  def self.exec(method, url : String | URI, headers : HTTP::Headers? = nil, body : BodyType = nil, tls = nil) : HTTP::Client::Response
     exec(url, tls) do |client, path|
       client.exec method, path, headers, body
     end
@@ -546,7 +549,7 @@ class HTTP::Client
   #   response.body_io.gets # => "..."
   # end
   # ```
-  def self.exec(method, url : String | URI, headers : HTTP::Headers? = nil, body : String? = nil, tls = nil)
+  def self.exec(method, url : String | URI, headers : HTTP::Headers? = nil, body : BodyType = nil, tls = nil)
     exec(url, tls) do |client, path|
       client.exec(method, path, headers, body) do |response|
         yield response
@@ -560,7 +563,7 @@ class HTTP::Client
     @socket = nil
   end
 
-  private def new_request(method, path, headers, body)
+  private def new_request(method, path, headers, body : BodyType)
     HTTP::Request.new(method, path, headers, body).tap do |request|
       request.headers["Host"] ||= host_header
     end

diff --git a/src/http/common.cr b/src/http/common.cr
@@ -107,46 +107,58 @@ module HTTP
 
   # :nodoc:
   def self.serialize_headers_and_body(io, headers, body, body_io, version)
-    # prepare either chunked response headers if protocol supports it
-    # or consume the io to get the Content-Length header
-    unless body
-      if body_io
-        if Client::Response.supports_chunked?(version)
-          headers["Transfer-Encoding"] = "chunked"
-          body = nil
-        else
-          body = body_io.gets_to_end
-          body_io = nil
+    if body
+      serialize_headers_and_string_body(io, headers, body)
+    elsif body_io
+      content_length = content_length(headers)
+      if content_length
+        serialize_headers(io, headers)
+        copied = IO.copy(body_io, io)
+        if copied != content_length
+          raise ArgumentError.new("Content-Length header is #{content_length} but body had #{copied} bytes")
         end
+      elsif Client::Response.supports_chunked?(version)
+        headers["Transfer-Encoding"] = "chunked"
+        serialize_headers(io, headers)
+        serialize_chunked_body(io, body_io)
+      else
+        body = body_io.gets_to_end
+        serialize_headers_and_string_body(io, headers, body)
       end
+    else
+      serialize_headers(io, headers)
     end
+  end
 
-    if body
-      headers["Content-Length"] = body.bytesize.to_s
-    end
+  def self.serialize_headers_and_string_body(io, headers, body)
+    headers["Content-Length"] = body.bytesize.to_s
+    serialize_headers(io, headers)
+    io << body
+  end
 
+  def self.serialize_headers(io, headers)
     headers.each do |name, values|
       values.each do |value|
         io << name << ": " << value << "\r\n"
       end
     end
-
     io << "\r\n"
+  end
 
-    if body
-      io << body
+  def self.serialize_chunked_body(io, body)
+    buf = uninitialized UInt8[8192]
+    while (buf_length = body.read(buf.to_slice)) > 0
+      buf_length.to_s(16, io)
+      io << "\r\n"
+      io.write(buf.to_slice[0, buf_length])
+      io << "\r\n"
     end
+    io << "0\r\n\r\n"
+  end
 
-    if body_io
-      buf = uninitialized UInt8[8192]
-      while (buf_length = body_io.read(buf.to_slice)) > 0
-        buf_length.to_s(16, io)
-        io << "\r\n"
-        io.write(buf.to_slice[0, buf_length])
-        io << "\r\n"
-      end
-      io << "0\r\n\r\n"
-    end
+  # :nodoc
+  def self.content_length(headers)
+    headers["Content-Length"]?.try &.to_u64?
   end
 
   # :nodoc:

diff --git a/src/http/request.cr b/src/http/request.cr
@@ -2,22 +2,27 @@ require "./common"
 require "uri"
 require "http/params"
 
+# An HTTP request.
+#
+# It serves both to perform requests by an `HTTP::Client` and to
+# represent requests received by an `HTTP::Server`.
+#
+# A request always holds an IO as a body.
+# When creating a request with a `String` or `Bytes` its body
+# will be a `MemoryIO` wrapping these, and the Content-Length
+# header will be set appropriately.
 class HTTP::Request
-  getter method : String
-  getter headers : Headers
-  getter body : String?
-  getter version : String
+  property method : String
+  property headers : Headers
+  getter body : IO?
+  property version : String
   @cookies : Cookies?
   @query_params : Params?
   @uri : URI?
 
-  def initialize(@method : String, @resource : String, headers : Headers? = nil, @body = nil, @version = "HTTP/1.1")
+  def initialize(@method : String, @resource : String, headers : Headers? = nil, body : String | Bytes | IO | Nil = nil, @version = "HTTP/1.1")
     @headers = headers.try(&.dup) || Headers.new
-    if body = @body
-      @headers["Content-Length"] = body.bytesize.to_s
-    elsif @method == "POST" || @method == "PUT"
-      @headers["Content-Length"] = "0"
-    end
+    self.body = body
   end
 
   # Returns a convenience wrapper around querying and setting cookie related
@@ -45,11 +50,36 @@ class HTTP::Request
     @method == "HEAD"
   end
 
+  def content_length=(length : Int)
+    headers["Content-Length"] = length.to_s
+  end
+
+  def content_length
+    HTTP.content_length(headers)
+  end
+
+  def body=(body : String)
+    @body = MemoryIO.new(body)
+    self.content_length = body.bytesize
+  end
+
+  def body=(body : Bytes)
+    @body = MemoryIO.new(body)
+    self.content_length = body.size
+  end
+
+  def body=(@body : IO)
+  end
+
+  def body=(@body : Nil)
+    @headers["Content-Length"] = "0" if @method == "POST" || @method == "PUT"
+  end
+
   def to_io(io)
     io << @method << " " << resource << " " << @version << "\r\n"
     cookies = @cookies
     headers = cookies ? cookies.add_request_headers(@headers) : @headers
-    HTTP.serialize_headers_and_body(io, headers, @body, nil, @version)
+    HTTP.serialize_headers_and_body(io, headers, nil, @body, @version)
   end
 
   # :nodoc:
@@ -68,7 +98,7 @@ class HTTP::Request
 
     method, resource, http_version = parts
     HTTP.parse_headers_and_body(io) do |headers, body|
-      return new method, resource, headers, body.try &.gets_to_end, http_version
+      return new method, resource, headers, body, http_version
     end
 
     # Unexpected end of http request

diff --git a/src/http/server/request_processor.cr b/src/http/server/request_processor.cr
@@ -54,6 +54,10 @@ class HTTP::Server::RequestProcessor
         output.flush
 
         break unless request.keep_alive?
+
+        # Skip request body in case the handler
+        # didn't read it all, for the next request
+        request.body.try &.close
       end
     rescue ex : Errno
       # IO-related error, nothing to do

diff --git a/src/oauth/signature.cr b/src/oauth/signature.cr
@@ -80,7 +80,9 @@ struct OAuth::Signature
     body = request.body
     content_type = request.headers["Content-type"]?
     if body && content_type == "application/x-www-form-urlencoded"
-      params.add_query body
+      form = body.gets_to_end
+      params.add_query form
+      request.body = form
     end
 
     params