From 71b9ac4e60a28b8682e778e19d1ce29263fa3ae8 Mon Sep 17 00:00:00 2001 From: Benny Bach Date: Sun, 17 Apr 2016 19:18:22 +0200 Subject: [PATCH 1/5] Add cache control headers to http static file handler + a few more mime types --- .../handlers/static_file_handler_spec.cr | 20 +++++++++++++++++-- .../server/handlers/static_file_handler.cr | 12 +++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/spec/std/http/server/handlers/static_file_handler_spec.cr b/spec/std/http/server/handlers/static_file_handler_spec.cr index f86fe5f56c49..0582bbfefad0 100644 --- a/spec/std/http/server/handlers/static_file_handler_spec.cr +++ b/spec/std/http/server/handlers/static_file_handler_spec.cr @@ -1,7 +1,7 @@ require "spec" require "http/server" -private def handle(request, fallthrough = true, directory_listing = true) +private def handle(request, fallthrough = true, directory_listing = true, ignore_body = false) io = IO::Memory.new response = HTTP::Server::Response.new(io) context = HTTP::Server::Context.new(request, response) @@ -9,7 +9,7 @@ private def handle(request, fallthrough = true, directory_listing = true) handler.call context response.close io.rewind - HTTP::Client::Response.from_io(io) + HTTP::Client::Response.from_io(io, ignore_body) end describe HTTP::StaticFileHandler do @@ -21,6 +21,22 @@ describe HTTP::StaticFileHandler do response.body.should eq(File.read("#{__DIR__}/static/test.txt")) end + it "should return 304 (not modified) if file is not modified" do + headers = HTTP::Headers.new + headers["If-Modified-Since"] = File.stat("#{__DIR__}/static/test.txt").mtime.to_s("%a, %-d %h %C%y %T GMT") + response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true + response.status_code.should eq(304) + end + + it "should serve file if it is modified" do + headers = HTTP::Headers.new + headers["If-Modified-Since"] = Time.epoch(0).to_s("%a, %-d %h %C%y %T GMT") + response = handle HTTP::Request.new("GET", "/test.txt") + response.status_code.should eq(200) + response.headers["Last-Modified"].should eq(File.stat("#{__DIR__}/static/test.txt").mtime.to_s("%a, %-d %h %C%y %T GMT")) + response.body.should eq(File.read("#{__DIR__}/static/test.txt")) + end + it "should list directory's entries" do response = handle HTTP::Request.new("GET", "/") response.status_code.should eq(200) diff --git a/src/http/server/handlers/static_file_handler.cr b/src/http/server/handlers/static_file_handler.cr index 9472e89e7566..c70bfac049a0 100644 --- a/src/http/server/handlers/static_file_handler.cr +++ b/src/http/server/handlers/static_file_handler.cr @@ -64,6 +64,15 @@ class HTTP::StaticFileHandler context.response.content_type = "text/html" directory_listing(context.response, request_path, file_path) elsif is_file + last_modified = File.stat(file_path).mtime.to_s("%a, %-d %h %C%y %T GMT") + if if_modified_since = context.request.headers.fetch("If-Modified-Since", nil) + if last_modified == if_modified_since + context.response.status_code = 304 + return + end + end + context.response.headers["Cache-Control"] = "public" + context.response.headers["Last-Modified"] = last_modified context.response.content_type = mime_type(file_path) context.response.content_length = File.size(file_path) File.open(file_path) do |file| @@ -93,6 +102,9 @@ class HTTP::StaticFileHandler when ".htm", ".html" then "text/html" when ".css" then "text/css" when ".js" then "application/javascript" + when ".jpg" then "image/jpeg" + when ".png" then "image/png" + when ".svg" then "image/svg+xml" else "application/octet-stream" end end From fb5b8f207e9c056594a825e1a7cf35dbe1383a44 Mon Sep 17 00:00:00 2001 From: Benny Bach Date: Mon, 31 Jul 2017 12:18:09 +0200 Subject: [PATCH 2/5] Remove Cache-Control header from static file handler --- src/http/server/handlers/static_file_handler.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/http/server/handlers/static_file_handler.cr b/src/http/server/handlers/static_file_handler.cr index c70bfac049a0..dfef02794728 100644 --- a/src/http/server/handlers/static_file_handler.cr +++ b/src/http/server/handlers/static_file_handler.cr @@ -71,7 +71,6 @@ class HTTP::StaticFileHandler return end end - context.response.headers["Cache-Control"] = "public" context.response.headers["Last-Modified"] = last_modified context.response.content_type = mime_type(file_path) context.response.content_length = File.size(file_path) From 131cccb75070b2161884d8d60ecf9a49e7aef4a1 Mon Sep 17 00:00:00 2001 From: Benny Bach Date: Mon, 31 Jul 2017 12:19:25 +0200 Subject: [PATCH 3/5] Undo extra mime types in static file handler --- src/http/server/handlers/static_file_handler.cr | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/http/server/handlers/static_file_handler.cr b/src/http/server/handlers/static_file_handler.cr index dfef02794728..e5bf5d49c7cc 100644 --- a/src/http/server/handlers/static_file_handler.cr +++ b/src/http/server/handlers/static_file_handler.cr @@ -101,9 +101,6 @@ class HTTP::StaticFileHandler when ".htm", ".html" then "text/html" when ".css" then "text/css" when ".js" then "application/javascript" - when ".jpg" then "image/jpeg" - when ".png" then "image/png" - when ".svg" then "image/svg+xml" else "application/octet-stream" end end From 73c4289d9f477a1b4f8f439ff49a76f5a77d0fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Mon, 31 Jul 2017 17:35:56 +0200 Subject: [PATCH 4/5] Fix code review issues: * use HTTP.rfc3339_date formatter * parse time value from If-Modified-Since header * compare header and mtime as older or equals --- .../handlers/static_file_handler_spec.cr | 37 ++++++++++++------- .../server/handlers/static_file_handler.cr | 15 ++++++-- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/spec/std/http/server/handlers/static_file_handler_spec.cr b/spec/std/http/server/handlers/static_file_handler_spec.cr index 0582bbfefad0..f4de1cf6ba58 100644 --- a/spec/std/http/server/handlers/static_file_handler_spec.cr +++ b/spec/std/http/server/handlers/static_file_handler_spec.cr @@ -21,20 +21,31 @@ describe HTTP::StaticFileHandler do response.body.should eq(File.read("#{__DIR__}/static/test.txt")) end - it "should return 304 (not modified) if file is not modified" do - headers = HTTP::Headers.new - headers["If-Modified-Since"] = File.stat("#{__DIR__}/static/test.txt").mtime.to_s("%a, %-d %h %C%y %T GMT") - response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true - response.status_code.should eq(304) - end + context "with header If-Modified-Since" do + it "should return 304 Not Modified if file mtime is equal" do + headers = HTTP::Headers.new + headers["If-Modified-Since"] = HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime) + response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true + response.status_code.should eq(304) + response.headers["Last-Modified"].should eq(HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime)) + end - it "should serve file if it is modified" do - headers = HTTP::Headers.new - headers["If-Modified-Since"] = Time.epoch(0).to_s("%a, %-d %h %C%y %T GMT") - response = handle HTTP::Request.new("GET", "/test.txt") - response.status_code.should eq(200) - response.headers["Last-Modified"].should eq(File.stat("#{__DIR__}/static/test.txt").mtime.to_s("%a, %-d %h %C%y %T GMT")) - response.body.should eq(File.read("#{__DIR__}/static/test.txt")) + it "should return 304 Not Modified if file mtime is older" do + headers = HTTP::Headers.new + headers["If-Modified-Since"] = HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime + 1.hour) + response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true + response.status_code.should eq(304) + response.headers["Last-Modified"].should eq(HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime)) + end + + it "should serve file if file mtime is younger" do + headers = HTTP::Headers.new + headers["If-Modified-Since"] = HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime - 1.hour) + response = handle HTTP::Request.new("GET", "/test.txt") + response.status_code.should eq(200) + response.headers["Last-Modified"].should eq(HTTP.rfc1123_date(File.stat("#{__DIR__}/static/test.txt").mtime)) + response.body.should eq(File.read("#{__DIR__}/static/test.txt")) + end end it "should list directory's entries" do diff --git a/src/http/server/handlers/static_file_handler.cr b/src/http/server/handlers/static_file_handler.cr index e5bf5d49c7cc..b5dbd32dc80b 100644 --- a/src/http/server/handlers/static_file_handler.cr +++ b/src/http/server/handlers/static_file_handler.cr @@ -64,14 +64,23 @@ class HTTP::StaticFileHandler context.response.content_type = "text/html" directory_listing(context.response, request_path, file_path) elsif is_file - last_modified = File.stat(file_path).mtime.to_s("%a, %-d %h %C%y %T GMT") + last_modified = File.stat(file_path).mtime + context.response.headers["Last-Modified"] = HTTP.rfc1123_date(last_modified) + if if_modified_since = context.request.headers.fetch("If-Modified-Since", nil) - if last_modified == if_modified_since + # TODO: Use a more generalized time format parser for better compatibility to RFC 7232 + header_time = Time.parse(if_modified_since, "%a, %d %b %Y %H:%M:%S GMT") + + # File mtime probably has a higher resolution than the header value. + # An exact comparison might be slightly off, so we add 1s padding. + # Static files should generally not be modified in subsecond intervals, so this is perfectly safe. + # This might replaced by a more sophisticated time comparison when it becomes available. + if last_modified <= header_time + 1.second context.response.status_code = 304 return end end - context.response.headers["Last-Modified"] = last_modified + context.response.content_type = mime_type(file_path) context.response.content_length = File.size(file_path) File.open(file_path) do |file| From 2ff58b83f0e729610ae9109e12f05ab8b39be053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 26 Sep 2017 18:54:39 +0200 Subject: [PATCH 5/5] use `headers["If-Modified-Since"]?` --- src/http/server/handlers/static_file_handler.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http/server/handlers/static_file_handler.cr b/src/http/server/handlers/static_file_handler.cr index b5dbd32dc80b..6c875efdde07 100644 --- a/src/http/server/handlers/static_file_handler.cr +++ b/src/http/server/handlers/static_file_handler.cr @@ -67,7 +67,7 @@ class HTTP::StaticFileHandler last_modified = File.stat(file_path).mtime context.response.headers["Last-Modified"] = HTTP.rfc1123_date(last_modified) - if if_modified_since = context.request.headers.fetch("If-Modified-Since", nil) + if if_modified_since = context.request.headers["If-Modified-Since"]? # TODO: Use a more generalized time format parser for better compatibility to RFC 7232 header_time = Time.parse(if_modified_since, "%a, %d %b %Y %H:%M:%S GMT")