Skip to content

Commit

Permalink
Adds default Content-Type header: text/html (#949)
Browse files Browse the repository at this point in the history
* [Response] Adds Content-Type headers when missing

Issue: #947

> Amber don't return any Content-Type (for the default example while using
amber new myapp). Currently when rendering a view the Content-Type header
is not set.

- Ensures a content type is set based on the requested format or
defaults to `text/html`.

Alls responses should contain a content type based on the format
requested, if no format is requested it will default to `text/html`

Alternate Solution:

A more long term solution is to have `render` implicitly parse the the content-type
for the response.

`render("some_view.html.slang") == Content-Type: text/html`
`render("some_view.txt.slang") == Content-Type: text/plain`
`render("some_view.json.slang") == Content-Type: application/plain`

* fixup! [Response] Adds Content-Type headers when missing
  • Loading branch information
eliasjpr authored and robacarp committed Sep 7, 2018
1 parent c0df539 commit a6b2f74
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 6 deletions.
7 changes: 7 additions & 0 deletions spec/amber/pipes/pipeline_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,43 +35,50 @@ module Amber
request = HTTP::Request.new("GET", "/index/elias")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Hello World!"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to GET request" do
request = HTTP::Request.new("GET", "/hello")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Index"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to PUT request" do
request = HTTP::Request.new("PUT", "/hello/1")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Update"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to PATCH request" do
request = HTTP::Request.new("PATCH", "/hello/1")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Update"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to POST request" do
request = HTTP::Request.new("POST", "/hello")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Create"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to DELETE request" do
request = HTTP::Request.new("DELETE", "/hello/1")
response = create_request_and_return_io(pipeline, request)
response.body.should eq "Destroy"
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end

it "responds to HEAD request" do
request = HTTP::Request.new("HEAD", "/hello/1")
response = create_request_and_return_io(pipeline, request)
response.headers["Content-Length"].should eq "0"
response.body.empty?.should be_true
response.headers[HTTP::Server::Context::CONTENT_TYPE].should eq "text/html"
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/amber/router/context_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe HTTP::Server::Context do
end
end

describe "port" do
describe "#port" do
it "gets the port from the requested URL" do
url = "http://localhost:9450"
request = HTTP::Request.new("GET", url)
Expand Down
2 changes: 1 addition & 1 deletion src/amber/pipes/pipeline.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module Amber
context.process_websocket_request
elsif @drain[context.valve]
@drain[context.valve].call(context)
context.finalize_response
context.finalize_response!
end
rescue e : Amber::Exceptions::Base
Amber::Pipe::Error.new.call(context)
Expand Down
10 changes: 7 additions & 3 deletions src/amber/router/context.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ require "./session"
require "./flash"

class HTTP::Server::Context
METHODS = %i(get post put patch delete head)
METHODS = %i(get post put patch delete head)
CONTENT_TYPE = "Content-Type"

include Amber::Router::Session
include Amber::Router::Flash
Expand Down Expand Up @@ -61,7 +62,7 @@ class HTTP::Server::Context
end

def halt!(status_code : Int32 = 200, @content = "")
response.headers["Content-Type"] = "text/plain"
response.headers[CONTENT_TYPE] = "text/plain"
response.status_code = status_code
end

Expand All @@ -77,9 +78,12 @@ class HTTP::Server::Context
request.route.call(self)
end

protected def finalize_response
protected def finalize_response!
response.headers["Connection"] = "Keep-Alive"
response.headers.add("Keep-Alive", "timeout=5, max=10000")
unless response.headers[CONTENT_TYPE]?
response.headers[CONTENT_TYPE] = Amber::Support::MimeTypes.mime_type(format, "text/html")
end
response.print(@content) unless request.method == "HEAD"
end
end
6 changes: 5 additions & 1 deletion src/amber/support/mime_types.cr
Original file line number Diff line number Diff line change
Expand Up @@ -624,11 +624,15 @@ module Amber
# Amber::Support::Mime.mime_type("unknown") # => "application/octet-stream"
# Amber::Support::Mime.mime_type("unknown", "text/plain") # => "text/plain"
# ```
def self.mime_type(format, fallback = DEFAULT_MIME_TYPE)
def self.mime_type(format : String, fallback = DEFAULT_MIME_TYPE)
format = format[1..-1] if format.starts_with?('.')
MIME_TYPES.fetch(format, fallback)
end

def self.mime_type(format : Nil, fallback = DEFAULT_MIME_TYPE)
fallback
end

def self.zip_types(path)
ZIP_FILE_EXTENSIONS.includes? File.extname(path)
end
Expand Down

0 comments on commit a6b2f74

Please sign in to comment.