Add handling for invalid responses to Response.from_io (#5630)

crystal-lang · Jun 6, 2018 · 9ddaf28 · 9ddaf28
1 parent a64e603
commit 9ddaf28
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 1 deletion.
diff --git a/spec/std/http/client/response_spec.cr b/spec/std/http/client/response_spec.cr
@@ -133,6 +133,41 @@ class HTTP::Client
       request.should eq(nil)
     end
 
+    describe "handle invalid IO" do
+      it "missing HTTP header" do
+        expect_raises(Exception, "Invalid HTTP response") do
+          Response.from_io?(IO::Memory.new("<html>\n</html>"))
+        end
+      end
+
+      it "unsupported version" do
+        expect_raises(Exception, "Unsupported HTTP version: HTML/1.0") do
+          Response.from_io?(IO::Memory.new("HTML/1.0 200 OK\n\nNot an HTTP response"))
+        end
+      end
+
+      it "missing status" do
+        expect_raises(Exception, "Invalid HTTP response") do
+          Response.from_io?(IO::Memory.new("HTTTP/1.0\n\nNot an HTTP response"))
+        end
+      end
+
+      it "invalid status" do
+        expect_raises(Exception, "Invalid HTTP status code: OK") do
+          Response.from_io?(IO::Memory.new("HTTP/1.0 OK\n\nNot an HTTP response"))
+        end
+        expect_raises(Exception, "Invalid HTTP status code: 1000") do
+          Response.from_io?(IO::Memory.new("HTTP/1.0 1000\n\nNot an HTTP response"))
+        end
+        expect_raises(Exception, "Invalid HTTP status code: -5") do
+          Response.from_io?(IO::Memory.new("HTTP/1.0 -5\n\nNot an HTTP response"))
+        end
+        expect_raises(Exception, "Invalid HTTP status code: 42") do
+          Response.from_io?(IO::Memory.new("HTTP/1.0 42\n\nNot an HTTP response"))
+        end
+      end
+    end
+
     it "doesn't sets content length for 1xx, 204 or 304" do
       [100, 101, 204, 304].each do |status|
         response = Response.new(status)

diff --git a/spec/std/http/request_spec.cr b/spec/std/http/request_spec.cr
@@ -178,6 +178,11 @@ module HTTP
       request.should be_a(Request::BadRequest)
     end
 
+    it "handles unsupported HTTP version" do
+      request = Request.from_io(IO::Memory.new("GET / HTTP/1.2\r\nContent-Length: 0\r\n\r\n"))
+      request.should be_a(Request::BadRequest)
+    end
+
     it "handles long request lines" do
       request = Request.from_io(IO::Memory.new("GET /#{"a" * 4096} HTTP/1.1\r\n\r\n"))
       request.should be_a(Request::BadRequest)

diff --git a/src/http/client/response.cr b/src/http/client/response.cr
@@ -120,8 +120,17 @@ class HTTP::Client::Response
     return yield nil unless line
 
     pieces = line.split(3)
+    raise "Invalid HTTP response" if pieces.size < 2
+
     http_version = pieces[0]
-    status_code = pieces[1].to_i
+    raise "Unsupported HTTP version: #{http_version}" unless HTTP::SUPPORTED_VERSIONS.includes?(http_version)
+
+    status_code = pieces[1].to_i?
+
+    unless status_code && 100 <= status_code < 1000
+      raise "Invalid HTTP status code: #{pieces[1]}"
+    end
+
     status_message = pieces[2]? ? pieces[2].chomp : ""
 
     body_type = HTTP::BodyType::OnDemand

diff --git a/src/http/common.cr b/src/http/common.cr
@@ -16,6 +16,8 @@ module HTTP
     Mandatory
   end
 
+  SUPPORTED_VERSIONS = {"HTTP/1.0", "HTTP/1.1"}
+
   # :nodoc:
   def self.parse_headers_and_body(io, body_type : BodyType = BodyType::OnDemand, decompress = true)
     headers = Headers.new

diff --git a/src/http/request.cr b/src/http/request.cr
@@ -95,6 +95,9 @@ class HTTP::Request
     return BadRequest.new unless parts.size == 3
 
     method, resource, http_version = parts
+
+    return BadRequest.new unless HTTP::SUPPORTED_VERSIONS.includes?(http_version)
+
     HTTP.parse_headers_and_body(io) do |headers, body|
       return new method, resource, headers, body, http_version
     end