Add cache control headers to http static file handler + a few more mi… (

#2470) * Add cache control headers to http static file handler + a few more mime types * Remove Cache-Control header from static file handler * Undo extra mime types in static file handler * 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 * use `headers["If-Modified-Since"]?`
crystal-lang · Jan 18, 2018 · 1210596 · 1210596
1 parent 3b50388
commit 1210596
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 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
@@ -1,15 +1,15 @@
 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)
   handler = HTTP::StaticFileHandler.new "#{__DIR__}/static", fallthrough, directory_listing
   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,33 @@ describe HTTP::StaticFileHandler do
     response.body.should eq(File.read("#{__DIR__}/static/test.txt"))
   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 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
     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
@@ -64,6 +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
+      context.response.headers["Last-Modified"] = HTTP.rfc1123_date(last_modified)
+
+      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")
+
+        # 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.content_type = mime_type(file_path)
       context.response.content_length = File.size(file_path)
       File.open(file_path) do |file|