From 90fa2f78c0b2003158fe322841582b26f66cb03b Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 1 Feb 2021 20:42:16 -0500 Subject: [PATCH 1/2] Better handle RFC and `content-length` header --- .../server/handlers/compress_handler_spec.cr | 2 +- spec/std/http/server/response_spec.cr | 34 +++++++++++++++++++ src/http/server/response.cr | 9 ++++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/spec/std/http/server/handlers/compress_handler_spec.cr b/spec/std/http/server/handlers/compress_handler_spec.cr index f6c0b5ab653d..cb4badc994d1 100644 --- a/spec/std/http/server/handlers/compress_handler_spec.cr +++ b/spec/std/http/server/handlers/compress_handler_spec.cr @@ -127,7 +127,7 @@ describe HTTP::CompressHandler do response.close io.rewind - io.to_s.should eq("HTTP/1.1 304 Not Modified\r\nContent-Length: 0\r\n\r\n") + io.to_s.should eq("HTTP/1.1 304 Not Modified\r\n\r\n") end it "don't try to compress upgraded response" do diff --git a/spec/std/http/server/response_spec.cr b/spec/std/http/server/response_spec.cr index 0042f40c008f..f22aed60b294 100644 --- a/spec/std/http/server/response_spec.cr +++ b/spec/std/http/server/response_spec.cr @@ -42,6 +42,40 @@ describe HTTP::Server::Response do io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") end + it "does not automatically add the `content-length` header if the response is a 304" do + io = IO::Memory.new + response = Response.new(io) + response.status = :not_modified + response.close + io.to_s.should eq("HTTP/1.1 304 Not Modified\r\n\r\n") + end + + it "does not automatically add the `content-length` header if the response is a 204" do + io = IO::Memory.new + response = Response.new(io) + response.status = :no_content + response.close + io.to_s.should eq("HTTP/1.1 204 No Content\r\n\r\n") + end + + it "does not automatically add the `content-length` header if the response is informational" do + io = IO::Memory.new + response = Response.new(io) + response.status = :processing + response.close + io.to_s.should eq("HTTP/1.1 102 Processing\r\n\r\n") + end + + # Case where the content-length represents the size of the data that would have been returned. + it "allows specifying the content-length header explicitly" do + io = IO::Memory.new + response = Response.new(io) + response.status = :not_modified + response.headers["Content-Length"] = "5" + response.close + io.to_s.should eq("HTTP/1.1 304 Not Modified\r\nContent-Length: 5\r\n\r\n") + end + it "prints less then buffer's size" do io = IO::Memory.new response = Response.new(io) diff --git a/src/http/server/response.cr b/src/http/server/response.cr index a2651339fcca..d2002f9c0321 100644 --- a/src/http/server/response.cr +++ b/src/http/server/response.cr @@ -223,7 +223,14 @@ class HTTP::Server def close return if closed? - if !response.wrote_headers? && !response.headers.has_key?("Content-Length") + # Conditionally determine based on status if the `content-length` header should be added automatically. + # See https://tools.ietf.org/html/rfc7230#section-3.3.2. + include_content_length_header = case response.status + when .not_modified?, .no_content?, .informational? then false + else true + end + + if !response.wrote_headers? && !response.headers.has_key?("Content-Length") && include_content_length_header response.content_length = @out_count end From 53544e6afb433ca473354d287ac884093dea3d3a Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Tue, 2 Feb 2021 19:01:19 -0500 Subject: [PATCH 2/2] Replace case with || --- src/http/server/response.cr | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/http/server/response.cr b/src/http/server/response.cr index d2002f9c0321..8b6a5fab736a 100644 --- a/src/http/server/response.cr +++ b/src/http/server/response.cr @@ -225,12 +225,10 @@ class HTTP::Server # Conditionally determine based on status if the `content-length` header should be added automatically. # See https://tools.ietf.org/html/rfc7230#section-3.3.2. - include_content_length_header = case response.status - when .not_modified?, .no_content?, .informational? then false - else true - end + status = response.status + set_content_length = !(status.not_modified? || status.no_content? || status.informational?) - if !response.wrote_headers? && !response.headers.has_key?("Content-Length") && include_content_length_header + if !response.wrote_headers? && !response.headers.has_key?("Content-Length") && set_content_length response.content_length = @out_count end