Fix static conditional requests.

Assisted-By: devx/b3414b50-d642-461b-95a8-8f773c4d087b

Author: samuel-williams-shopify

lib/utopia/static/local_file.rb

@@ -14,28 +14,21 @@
 module Utopia
 	# A middleware which serves static files from the specified root directory.
 	module Static
-		# Represents a local file on disk which can be served directly, or passed upstream to sendfile.
+		# Represents a local static resource and constructs responses for it.
 		class LocalFile
 			# Initialize metadata for a file beneath a static root.
 			# @parameter root [String] The root directory.
 			# @parameter path [Utopia::Path | String] The path.
 			def initialize(root, path)
 				@root = root
 				@path = path
-				@etag = Digest::SHA1.hexdigest("#{File.size(full_path)}#{mtime_date}")
-				
-				@range = nil
+				fingerprint = Digest::SHA1.hexdigest("#{File.size(full_path)}#{mtime_date}")
+				@etag = %Q{"#{fingerprint}"}
 			end
 
 			attr :root
 			attr :path
 			attr :etag
-			attr :range
-			
-			# Expose the filesystem path for upstream sendfile support.
-			def to_path
-				full_path
-			end
 
 			# Resolve this file beneath its configured root.
 			# @returns [String] The full filesystem path.
@@ -55,41 +48,16 @@ def bytesize
 				File.size(full_path)
 			end
 
-			# This reflects whether calling each would yield anything.
-			def empty?
-				bytesize == 0
-			end
-			
-			alias size bytesize
-			
-			# Enumerate the contained values.
-			# @returns [Enumerator] An enumerator over the resulting values.
-			def each
-				File.open(full_path, "rb") do |file|
-					file.seek(@range.begin)
-					remaining = @range.end - @range.begin+1
-					
-					while remaining > 0
-						break unless part = file.read([8192, remaining].min)
-						
-						remaining -= part.length
-						
-						yield part
-					end
-				end
-			end
-			
 			# Check whether the file has changed since the request validators.
 			# @parameter request [Utopia::Request] The request.
 			# @returns [Boolean] Whether the file is newer than the request validators.
 			def modified?(request)
-				if modified_since = request.headers["if-modified-since"]
-					return false if File.mtime(full_path) <= Time.parse(modified_since)
+				if etags = request.headers["if-none-match"]
+					return !etags.weak_match?(@etag)
 				end
 
-				if etags = request.headers["if-none-match"]
-					etags = etags.split(/\s*,\s*/)
-					return false if etags.include?(etag) || etags.include?("*")
+				if modified_since = request.headers["if-modified-since"]
+					return File.mtime(full_path).to_i > modified_since.to_time.to_i
 				end
 
 				return true
@@ -104,6 +72,7 @@ def modified?(request)
 			# @returns [Protocol::HTTP::Response] The response.
 			def serve(request, response_headers)
 				ranges = byte_ranges(request.headers["range"])
+				size = bytesize
 
 				# puts "Requesting ranges: #{ranges.inspect} (#{size})"
 
@@ -112,18 +81,18 @@ def serve(request, response_headers)
 					# TODO: Support multiple byte-ranges, for now just send entire file:
 					status = 200
 					response_headers[CONTENT_LENGTH] = size.to_s
-					@range = 0...size
+					range = nil
 				else
 					# Partial content:
-					@range = ranges[0]
-					partial_size = @range.size
+					range = ranges[0]
+					partial_size = range.size
 
 					status = 206
 					response_headers[CONTENT_LENGTH] = partial_size.to_s
-					response_headers[CONTENT_RANGE] = "bytes #{@range.min}-#{@range.max}/#{size}"
+					response_headers[CONTENT_RANGE] = "bytes #{range.min}-#{range.max}/#{size}"
 				end
 
-				body = Protocol::HTTP::Body::File.open(full_path, status == 206 ? @range : nil, size: size)
+				body = Protocol::HTTP::Body::File.open(full_path, range, size: size)
 
 				return Response[status, response_headers, body]
 			end
@@ -134,7 +103,7 @@ def serve(request, response_headers)
 			def byte_ranges(range)
 				return nil unless range&.bytes?
 
-				return range.resolve(size)
+				return range.resolve(bytesize)
 			end
 		end
 	end

lib/utopia/static/middleware.rb

@@ -47,7 +47,6 @@ def freeze
 			# @parameter path [Utopia::Path | String] The path.
 			# @returns [LocalFile | Nil] The local file, or `nil` when it does not exist.
 			def fetch_file(path)
-				# We need file_path to be an absolute path for X-Sendfile to work correctly.
 				file_path = File.join(@root, path.components)
 
 				if File.exist?(file_path)

test/utopia/static.rb

@@ -24,6 +24,50 @@
 		expect(last_response.body).to be_a(Protocol::HTTP::Body::File)
 	end
 
+	it "returns not modified for matching entity tags" do
+		get "/test.txt"
+		etag = last_response.headers["etag"]
+		
+		expect(etag).to be(:start_with?, '"')
+		expect(etag).to be(:end_with?, '"')
+		
+		get "/test.txt", {"if-none-match" => etag}
+		expect(last_response.status).to be == 304
+		
+		get "/test.txt", {"if-none-match" => "W/#{etag}"}
+		expect(last_response.status).to be == 304
+		
+		get "/test.txt", {"if-none-match" => "*"}
+		expect(last_response.status).to be == 304
+	end
+	
+	it "gives entity tags precedence over modification dates" do
+		get "/test.txt", {
+			"if-none-match" => '"different"',
+			"if-modified-since" => (Time.now + 3600).httpdate,
+		}
+		
+		expect(last_response.status).to be == 200
+	end
+	
+	it "returns not modified when the modification time matches" do
+		get "/test.txt"
+		last_modified = last_response.headers["last-modified"]
+		
+		get "/test.txt", {"if-modified-since" => last_modified}
+		
+		expect(last_response.status).to be == 304
+	end
+	
+	it "returns modified when the modification time is newer" do
+		get "/test.txt"
+		last_modified = last_response.headers["last-modified"].to_time
+		
+		get "/test.txt", {"if-modified-since" => (last_modified - 1).httpdate}
+		
+		expect(last_response.status).to be == 200
+	end
+	
 	it "should return partial content" do
 		get "/test.txt", {"range" => "bytes=1-4"}