Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the content-length header more RFC compliant #10353

Merged
merged 2 commits into from
Feb 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion spec/std/http/server/handlers/compress_handler_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions spec/std/http/server/response_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion src/http/server/response.cr
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,12 @@ 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.
status = response.status
set_content_length = !(status.not_modified? || status.no_content? || status.informational?)

if !response.wrote_headers? && !response.headers.has_key?("Content-Length") && set_content_length
response.content_length = @out_count
end

Expand Down