Add Etag support in StaticFileHandler

spec/std/http/server/handlers/static_file_handler_spec.cr

@@ -21,29 +21,94 @@ describe HTTP::StaticFileHandler do
     response.body.should eq(File.read("#{__DIR__}/static/test.txt"))
   end
 
-  context "with header If-Modified-Since" do
+  it "should add Etag header" do
+    response = handle HTTP::Request.new("GET", "/test.txt")
+    response.headers["Etag"].should match(/W\/"\d+"$/)
+  end
+
+  it "should add Last-Modified header" do
+    response = handle HTTP::Request.new("GET", "/test.txt")
+    response.headers["Last-Modified"].should eq(HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time))
+  end
+
+  context "with If-Modified-Since header" do
     it "should return 304 Not Modified if file mtime is equal" do
+      initial_response = handle HTTP::Request.new("GET", "/test.txt")
+
       headers = HTTP::Headers.new
-      headers["If-Modified-Since"] = HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time)
+      headers["If-Modified-Since"] = initial_response.headers["Last-Modified"]
+
       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.info("#{__DIR__}/static/test.txt").modification_time))
+    end
+
+    it "should not return Content-Type if file mtime is equal" do
+      initial_response = handle HTTP::Request.new("GET", "/test.txt")
+
+      headers = HTTP::Headers.new
+      headers["If-Modified-Since"] = initial_response.headers["Last-Modified"]
+
+      response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true
+      response.headers["Content-Type"]?.should eq(nil)
     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.info("#{__DIR__}/static/test.txt").modification_time + 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.info("#{__DIR__}/static/test.txt").modification_time))
     end
 
     it "should serve file if file mtime is younger" do
       headers = HTTP::Headers.new
       headers["If-Modified-Since"] = HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time - 1.hour)
-      response = handle HTTP::Request.new("GET", "/test.txt")
+      response = handle HTTP::Request.new("GET", "/test.txt", headers)
+
+      response.status_code.should eq(200)
+      response.body.should eq(File.read("#{__DIR__}/static/test.txt"))
+    end
+  end
+
+  context "with If-None-Match header" do
+    it "should return 304 Not Modified if header matches etag" do
+      initial_response = handle HTTP::Request.new("GET", "/test.txt")
+
+      headers = HTTP::Headers.new
+      headers["If-None-Match"] = initial_response.headers["Etag"]
+      response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true
+      response.status_code.should eq(304)
+    end
+
+    it "should serve file if header does not match etag" do
+      headers = HTTP::Headers.new
+      headers["If-None-Match"] = "some random etag"
+
+      response = handle HTTP::Request.new("GET", "/test.txt", headers)
+      response.status_code.should eq(200)
+      response.body.should eq(File.read("#{__DIR__}/static/test.txt"))
+    end
+  end
+
+  context "with both If-None-Match and If-Modified-Since headers" do
+    it "ignores If-Modified-Since as specified in RFC 7232" do
+      initial_response = handle HTTP::Request.new("GET", "/test.txt")
+
+      headers = HTTP::Headers.new
+      headers["If-Modified-Since"] = HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time - 1.hour)
+      headers["If-None-Match"] = initial_response.headers["Etag"]
+      response = handle HTTP::Request.new("GET", "/test.txt", headers), ignore_body: true
+
+      response.status_code.should eq(304)
+    end
+
+    it "should serve a file if header does not match etag" do
+      headers = HTTP::Headers.new
+      headers["If-Modified-Since"] = HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time)
+      headers["If-None-Match"] = "some random etag"
+      response = handle HTTP::Request.new("GET", "/test.txt", headers)
+
       response.status_code.should eq(200)
-      response.headers["Last-Modified"].should eq(HTTP.rfc1123_date(File.info("#{__DIR__}/static/test.txt").modification_time))
       response.body.should eq(File.read("#{__DIR__}/static/test.txt"))
     end
   end

src/http/server/handlers/static_file_handler.cr

@@ -6,6 +6,12 @@ require "uri"
 class HTTP::StaticFileHandler
   include HTTP::Handler
 
+  ALLOWED_METHODS        = {"GET", "HEAD"}
+  HTTP_IF_MODIFIED_SINCE = "If-Modified-Since"
+  HTTP_IF_NONE_MATCH     = "If-None-Match"
+  HTTP_ETAG              = "Etag"
+  HTTP_LAST_MODIFIED     = "Last-Modified"
+
   @public_dir : String
 
   # Creates a handler that will serve files in the given *public_dir*, after
@@ -24,7 +30,7 @@ class HTTP::StaticFileHandler
   end
 
   def call(context)
-    unless context.request.method == "GET" || context.request.method == "HEAD"
+    unless ALLOWED_METHODS.includes?(context.request.method)
       if @fallthrough
         call_next(context)
       else
@@ -64,20 +70,11 @@ class HTTP::StaticFileHandler
       context.response.content_type = "text/html"
       directory_listing(context.response, request_path, file_path)
     elsif is_file
-      last_modified = File.info(file_path).modification_time
-      context.response.headers["Last-Modified"] = HTTP.rfc1123_date(last_modified)
-
-      if if_modified_since = context.request.headers["If-Modified-Since"]?
-        header_time = HTTP.parse_time(if_modified_since)
-
-        # 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 be replaced by a more sophisticated time comparison when it becomes available.
-        if header_time && last_modified <= header_time + 1.second
-          context.response.status_code = 304
-          return
-        end
+      add_cache_headers(context.response.headers, file_path)
+
+      if cache_request?(context, file_path)
+        context.response.status_code = 304
+        return
       end
 
       context.response.content_type = mime_type(file_path)
@@ -103,6 +100,39 @@ class HTTP::StaticFileHandler
     context.response.headers.add "Location", url
   end
 
+  private def add_cache_headers(response_headers : HTTP::Headers, file_path : String) : Nil
+    response_headers[HTTP_ETAG] = etag(file_path)
+
+    last_modified = modification_time(file_path)
+    response_headers[HTTP_LAST_MODIFIED] = HTTP.rfc1123_date(last_modified)
+  end
+
+  private def cache_request?(context : HTTP::Server::Context, file_path : String) : Bool
+    # According to RFC 7232:
+    # A recipient must ignore If-Modified-Since if the request contains an If-None-Match header field
+    if if_none_match = context.request.headers[HTTP_IF_NONE_MATCH]?
+      if_none_match == context.response.headers[HTTP_ETAG]
+    elsif if_modified_since = context.request.headers[HTTP_IF_MODIFIED_SINCE]?
+      header_time = HTTP.parse_time(if_modified_since)
+      # 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 be replaced by a more sophisticated time comparison when it becomes available.
+      last_modified = modification_time(file_path)
+      !!(header_time && last_modified <= header_time + 1.second)
+    else
+      false
+    end
+  end
+
+  private def etag(file_path)
+    %{W/"#{modification_time(file_path).epoch}"}
+  end
+
+  private def modification_time(file_path)
+    File.info(file_path).modification_time
+  end
+
   private def mime_type(path)
     case File.extname(path)
     when ".txt"          then "text/plain"