From 6392916f90cae739d9620596653d1c1bc5357881 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Thu, 4 May 2023 23:44:42 +0200 Subject: [PATCH 01/13] Update FileIO.swift sha-256 digest set when file is written to system --- Sources/Vapor/Utilities/FileIO.swift | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index 981de83fc9..dad3c10224 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -3,6 +3,7 @@ import NIOCore import NIOHTTP1 import NIOPosix import Logging +import Crypto extension Request { public var fileio: FileIO { @@ -265,6 +266,16 @@ public struct FileIO { done.whenComplete { _ in try? fd.close() } + + let digest = SHA256.hash(data: buffer.readableBytesView) + let filePath = URL(string: path) + + filePath?.withUnsafeFileSystemRepresentation({ pointer in + let _ = digest.withUnsafeBytes { bufferPointer in + setxattr(pointer, XAttributes.sha256Digest.getUnsafeMutablePointer(), bufferPointer.baseAddress, bufferPointer.count, 0, 0) + } + }) + return done } catch { return self.request.eventLoop.makeFailedFuture(error) @@ -272,6 +283,18 @@ public struct FileIO { } } +fileprivate extension FileIO { + /// Custom file attributes (_xattrs_) used by vapor when interacting with files. + enum XAttributes: String { + /// The key in a file attribute dictionary whose value represents a pre-computed SHA-256 digest of the file's content. + case sha256Digest = "x-vapor-sha256-digest" + + func getUnsafeMutablePointer() -> UnsafeMutablePointer? { + return UnsafeMutablePointer(mutating: (self.rawValue as NSString).utf8String) + } + } +} + extension HTTPHeaders.Range.Value { fileprivate func asByteBufferBounds(withMaxSize size: Int, logger: Logger) throws -> (offset: Int64, byteCount: Int) { From baa7fc435ac405a9e10bfadde807d7de9589265e Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sun, 14 May 2023 20:43:14 +0200 Subject: [PATCH 02/13] Introduced new streamFile method Introduced new streamFile method which allows for advancedETag comparison. Deprecated the old one. --- Sources/Vapor/Middleware/FileMiddleware.swift | 21 ++- Sources/Vapor/Utilities/FileIO.swift | 168 ++++++++++++++++-- 2 files changed, 165 insertions(+), 24 deletions(-) diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift index 242703777b..fe61bf2564 100644 --- a/Sources/Vapor/Middleware/FileMiddleware.swift +++ b/Sources/Vapor/Middleware/FileMiddleware.swift @@ -1,5 +1,6 @@ import Foundation import NIOCore +import CryptoKit /// Serves static files from a public directory. /// @@ -9,7 +10,8 @@ public final class FileMiddleware: Middleware { private let publicDirectory: String private let defaultFile: String? private let directoryAction: DirectoryAction - + private let advancedETagComparison: Bool + public struct BundleSetupError: Equatable, Error { /// The description of this error. @@ -22,6 +24,15 @@ public final class FileMiddleware: Middleware { public static let publicDirectoryIsNotAFolder: Self = .init(description: "Cannot find any actual folder for the given Public Directory") } + public struct ETagHashes: StorageKey { + public typealias Value = [String: FileHash] + + public struct FileHash { + var lastModified: Date + var digest: SHA256Digest + } + } + /// Creates a new `FileMiddleware`. /// /// - parameters: @@ -29,10 +40,11 @@ public final class FileMiddleware: Middleware { /// - defaultFile: The name of the default file to look for and serve if a request hits any public directory. Starting with `/` implies /// an absolute path from the public directory root. If `nil`, no default files are served. /// - directoryAction: Determines the action to take when the request doesn't have a trailing slash but matches a directory. - public init(publicDirectory: String, defaultFile: String? = nil, directoryAction: DirectoryAction = .none) { + public init(publicDirectory: String, defaultFile: String? = nil, directoryAction: DirectoryAction = .none, advancedETagComparison: Bool = true) { self.publicDirectory = publicDirectory.addTrailingSlash() self.defaultFile = defaultFile self.directoryAction = directoryAction + self.advancedETagComparison = advancedETagComparison } public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture { @@ -86,10 +98,9 @@ public final class FileMiddleware: Middleware { return next.respond(to: request) } } - + // stream the file - let res = request.fileio.streamFile(at: absPath) - return request.eventLoop.makeSucceededFuture(res) + return request.fileio.streamFile(at: absPath, advancedETagComparison: advancedETagComparison) } /// Creates a new `FileMiddleware` for a server contained in an Xcode Project. diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index dad3c10224..3e6322b975 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -4,6 +4,7 @@ import NIOHTTP1 import NIOPosix import Logging import Crypto +import NIOConcurrencyHelpers extension Request { public var fileio: FileIO { @@ -122,6 +123,7 @@ public struct FileIO { /// - mediaType: HTTPMediaType, if not specified, will be created from file extension. /// - onCompleted: Closure to be run on completion of stream. /// - returns: A `200 OK` response containing the file stream and appropriate headers. + @available(*, deprecated, message: "Use the new `streamFile` method which returns EventLoopFuture") public func streamFile( at path: String, chunkSize: Int = NonBlockingFileIO.defaultChunkSize, @@ -157,7 +159,7 @@ public struct FileIO { // Respond with lastModified header headers.lastModified = HTTPHeaders.LastModified(value: modifiedAt) - + // Generate ETag value, "HEX value of last modified date" + "-" + "file size" let fileETag = "\"\(modifiedAt.timeIntervalSince1970)-\(fileSize)\"" headers.replaceOrAdd(name: .eTag, value: fileETag) @@ -216,10 +218,134 @@ public struct FileIO { onCompleted(result) } }, count: byteCount, byteBufferAllocator: request.byteBufferAllocator) - + return response } + /// Generates a chunked `Response` for the specified file. This method respects values in + /// the `"ETag"` header and is capable of responding `304 Not Modified` if the file in question + /// has not been modified since last served. This method will also set the `"Content-Type"` header + /// automatically if an appropriate `MediaType` can be found for the file's suffix. + /// + /// router.get("file-stream") { req in + /// return req.fileio.streamFile(at: "/path/to/file.txt") + /// } + /// + /// - parameters: + /// - path: Path to file on the disk. + /// - chunkSize: Maximum size for the file data chunks. + /// - mediaType: HTTPMediaType, if not specified, will be created from file extension. + /// - advancedETagComparison: The method used when ETags are generated. If true, a byte-by-byte hash is created, otherwise a simple comparison based on the file's last modified date and size. + /// - onCompleted: Closure to be run on completion of stream. + /// - returns: A `200 OK` response containing the file stream and appropriate headers. + public func streamFile( + at path: String, + chunkSize: Int = NonBlockingFileIO.defaultChunkSize, + mediaType: HTTPMediaType? = nil, + advancedETagComparison: Bool, + onCompleted: @escaping (Result) -> () = { _ in } + ) -> EventLoopFuture { + // Get file attributes for this file. + guard + let attributes = try? FileManager.default.attributesOfItem(atPath: path), + let modifiedAt = attributes[.modificationDate] as? Date, + let fileSize = (attributes[.size] as? NSNumber)?.intValue + else { + return request.eventLoop.makeSucceededFuture(Response(status: .internalServerError)) + } + + let contentRange: HTTPHeaders.Range? + if let rangeFromHeaders = request.headers.range { + if rangeFromHeaders.unit == .bytes && rangeFromHeaders.ranges.count == 1 { + contentRange = rangeFromHeaders + } else { + contentRange = nil + } + } else if request.headers.contains(name: .range) { + // Range header was supplied but could not be parsed i.e. it was invalid + request.logger.debug("Range header was provided in request but was invalid") + let response = Response(status: .badRequest) + return request.eventLoop.makeSucceededFuture(response) + } else { + contentRange = nil + } + // Create empty headers array. + var headers: HTTPHeaders = [:] + + // Respond with lastModified header + headers.lastModified = HTTPHeaders.LastModified(value: modifiedAt) + + var eTagFuture: EventLoopFuture + + if advancedETagComparison { + eTagFuture = generateETagHash(path: path, lastModified: modifiedAt) + } else { + // Generate ETag value, "HEX value of last modified date" + "-" + "file size" + eTagFuture = request.eventLoop.makeSucceededFuture("\"\(modifiedAt.timeIntervalSince1970)-\(fileSize)\"") + } + + return eTagFuture.map { fileETag in + headers.replaceOrAdd(name: .eTag, value: fileETag) + + // Check if file has been cached already and return NotModified response if the etags match + if fileETag == request.headers.first(name: .ifNoneMatch) { + // Per RFC 9110 here: https://www.rfc-editor.org/rfc/rfc9110.html#status.304 + // and here: https://www.rfc-editor.org/rfc/rfc9110.html#name-content-encoding + // A 304 response MUST include the ETag header and a Content-Length header matching what the original resource's content length would have been were this a 200 response. + headers.replaceOrAdd(name: .contentLength, value: fileSize.description) + return Response(status: .notModified, version: .http1_1, headersNoUpdate: headers, body: .empty) + } + + // Create the HTTP response. + let response = Response(status: .ok, headers: headers) + let offset: Int64 + let byteCount: Int + if let contentRange = contentRange { + response.status = .partialContent + response.headers.add(name: .accept, value: contentRange.unit.serialize()) + if let firstRange = contentRange.ranges.first { + do { + let range = try firstRange.asResponseContentRange(limit: fileSize) + response.headers.contentRange = HTTPHeaders.ContentRange(unit: contentRange.unit, range: range) + (offset, byteCount) = try firstRange.asByteBufferBounds(withMaxSize: fileSize, logger: request.logger) + } catch { + let response = Response(status: .badRequest) + return response + } + } else { + offset = 0 + byteCount = fileSize + } + } else { + offset = 0 + byteCount = fileSize + } + // Set Content-Type header based on the media type + // Only set Content-Type if file not modified and returned above. + if + let fileExtension = path.components(separatedBy: ".").last, + let type = mediaType ?? HTTPMediaType.fileExtension(fileExtension) + { + response.headers.contentType = type + } + response.body = .init(stream: { stream in + self.read(path: path, fromOffset: offset, byteCount: byteCount, chunkSize: chunkSize) { chunk in + return stream.write(.buffer(chunk)) + }.whenComplete { result in + switch result { + case .failure(let error): + stream.write(.error(error), promise: nil) + case .success: + stream.write(.end, promise: nil) + } + onCompleted(result) + } + }, count: byteCount, byteBufferAllocator: request.byteBufferAllocator) + + return response + } + } + /// Private read method. `onRead` closure uses ByteBuffer and expects future return. /// There may be use in publicizing this in the future for reads that must be async. private func read( @@ -267,30 +393,34 @@ public struct FileIO { try? fd.close() } - let digest = SHA256.hash(data: buffer.readableBytesView) - let filePath = URL(string: path) - - filePath?.withUnsafeFileSystemRepresentation({ pointer in - let _ = digest.withUnsafeBytes { bufferPointer in - setxattr(pointer, XAttributes.sha256Digest.getUnsafeMutablePointer(), bufferPointer.baseAddress, bufferPointer.count, 0, 0) - } - }) - return done } catch { return self.request.eventLoop.makeFailedFuture(error) } } -} -fileprivate extension FileIO { - /// Custom file attributes (_xattrs_) used by vapor when interacting with files. - enum XAttributes: String { - /// The key in a file attribute dictionary whose value represents a pre-computed SHA-256 digest of the file's content. - case sha256Digest = "x-vapor-sha256-digest" + /// Generates a fresh ETag for a file or returns its currently cached one. + /// - Parameters: + /// - path: The file's path. + /// - lastModified: When the file was last modified. + /// - Returns: An `EventLoopFuture` which holds the ETag. + private func generateETagHash(path: String, lastModified: Date) -> EventLoopFuture { + return NIOLock().withLock { + var hashingDictionary = request.application.storage[FileMiddleware.ETagHashes.self] ?? [:] - func getUnsafeMutablePointer() -> UnsafeMutablePointer? { - return UnsafeMutablePointer(mutating: (self.rawValue as NSString).utf8String) + if let hash = hashingDictionary[path], hash.lastModified == lastModified { + return request.eventLoop.makeSucceededFuture(hash.digest.hex) + } else { + return collectFile(at: path).map { buffer in + let digest = SHA256.hash(data: buffer.readableBytesView) + + // update hash in dictionary + hashingDictionary[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) + request.application.storage[FileMiddleware.ETagHashes.self] = hashingDictionary + + return digest.hex + } + } } } } From 19b942e92a57b6ac2589e5dc3c5700a7dcee39b2 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sun, 14 May 2023 20:51:52 +0200 Subject: [PATCH 03/13] Updated Unit Tests Updates to remove deprecated warnings by using the new streamFile() method. Also removed some other deprecation warnings. --- Tests/VaporTests/FileTests.swift | 22 +++++++++++----------- Tests/VaporTests/RequestTests.swift | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift index d01bbce14f..42fe7ef717 100644 --- a/Tests/VaporTests/FileTests.swift +++ b/Tests/VaporTests/FileTests.swift @@ -10,7 +10,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -31,7 +31,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) + return req.fileio.streamFile(at: #file, advancedETagComparison: true) } var headers = HTTPHeaders() @@ -47,13 +47,13 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req -> Response in + app.get("file-stream") { req in var tmpPath: String repeat { tmpPath = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path } while (FileManager.default.fileExists(atPath: tmpPath)) - return req.fileio.streamFile(at: tmpPath) { result in + return req.fileio.streamFile(at: tmpPath, advancedETagComparison: true) { result in do { try result.get() XCTFail("File Stream should have failed") @@ -72,7 +72,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -104,7 +104,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -136,7 +136,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -168,7 +168,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -194,7 +194,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -220,7 +220,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { @@ -375,7 +375,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) + return req.fileio.streamFile(at: #file, advancedETagComparison: true) } var headers = HTTPHeaders() diff --git a/Tests/VaporTests/RequestTests.swift b/Tests/VaporTests/RequestTests.swift index 4cf5f847d3..de9a6a3782 100644 --- a/Tests/VaporTests/RequestTests.swift +++ b/Tests/VaporTests/RequestTests.swift @@ -261,13 +261,13 @@ final class RequestTests: XCTestCase { app.http.client.configuration.redirectConfiguration = .disallow app.get("redirect_normal") { - $0.redirect(to: "foo", type: .normal) + $0.redirect(to: "foo", redirectType: .normal) } app.get("redirect_permanent") { - $0.redirect(to: "foo", type: .permanent) + $0.redirect(to: "foo", redirectType: .permanent) } app.post("redirect_temporary") { - $0.redirect(to: "foo", type: .temporary) + $0.redirect(to: "foo", redirectType: .temporary) } try app.server.start(address: .hostname("localhost", port: 8080)) From 7933b9939e1b7d6a7f65cf6b3c5d3fb665e8b02c Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sun, 14 May 2023 21:32:56 +0200 Subject: [PATCH 04/13] Added more documentation --- Sources/Vapor/Middleware/FileMiddleware.swift | 1 + Sources/Vapor/Utilities/FileIO.swift | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift index fe61bf2564..1806733079 100644 --- a/Sources/Vapor/Middleware/FileMiddleware.swift +++ b/Sources/Vapor/Middleware/FileMiddleware.swift @@ -40,6 +40,7 @@ public final class FileMiddleware: Middleware { /// - defaultFile: The name of the default file to look for and serve if a request hits any public directory. Starting with `/` implies /// an absolute path from the public directory root. If `nil`, no default files are served. /// - directoryAction: Determines the action to take when the request doesn't have a trailing slash but matches a directory. + /// - advancedETagComparison: The method used when ETags are generated. If true, a byte-by-byte hash is created (and cached), otherwise a simple comparison based on the file's last modified date and size. public init(publicDirectory: String, defaultFile: String? = nil, directoryAction: DirectoryAction = .none, advancedETagComparison: Bool = true) { self.publicDirectory = publicDirectory.addTrailingSlash() self.defaultFile = defaultFile diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index 3e6322b975..5a1172dd2a 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -235,7 +235,7 @@ public struct FileIO { /// - path: Path to file on the disk. /// - chunkSize: Maximum size for the file data chunks. /// - mediaType: HTTPMediaType, if not specified, will be created from file extension. - /// - advancedETagComparison: The method used when ETags are generated. If true, a byte-by-byte hash is created, otherwise a simple comparison based on the file's last modified date and size. + /// - advancedETagComparison: The method used when ETags are generated. If true, a byte-by-byte hash is created (and cached), otherwise a simple comparison based on the file's last modified date and size. /// - onCompleted: Closure to be run on completion of stream. /// - returns: A `200 OK` response containing the file stream and appropriate headers. public func streamFile( From e2c3e4e8bf4b92b09e4b6fb2218f91b4395ab43c Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Mon, 15 May 2023 11:42:18 +0200 Subject: [PATCH 05/13] Removed unnecessary CryptoKit import --- Sources/Vapor/Middleware/FileMiddleware.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift index 1806733079..c4be904f9d 100644 --- a/Sources/Vapor/Middleware/FileMiddleware.swift +++ b/Sources/Vapor/Middleware/FileMiddleware.swift @@ -1,6 +1,5 @@ import Foundation import NIOCore -import CryptoKit /// Serves static files from a public directory. /// From 09507c17dfa6a8bf82d3c4ca53a5934a6a1d8598 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Mon, 15 May 2023 19:12:30 +0200 Subject: [PATCH 06/13] added closure return types --- Tests/VaporTests/FileTests.swift | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift index 42fe7ef717..bd4b60c7b5 100644 --- a/Tests/VaporTests/FileTests.swift +++ b/Tests/VaporTests/FileTests.swift @@ -9,7 +9,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -30,7 +30,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) } @@ -47,7 +47,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in var tmpPath: String repeat { tmpPath = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path @@ -71,7 +71,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -103,7 +103,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -135,7 +135,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -167,7 +167,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -193,7 +193,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -219,7 +219,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() @@ -374,7 +374,7 @@ final class FileTests: XCTestCase { let app = Application(.testing) defer { app.shutdown() } - app.get("file-stream") { req in + app.get("file-stream") { req -> EventLoopFuture in return req.fileio.streamFile(at: #file, advancedETagComparison: true) } From f192a76869863308fa80aa13623993d80d41dccd Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sat, 20 May 2023 14:41:50 +0200 Subject: [PATCH 07/13] incorporated first round of @0xTim's feedback --- Sources/Vapor/Middleware/FileMiddleware.swift | 2 +- Sources/Vapor/Utilities/FileIO.swift | 26 +++++++++---------- Tests/VaporTests/RequestTests.swift | 6 ++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift index c4be904f9d..d867097d3d 100644 --- a/Sources/Vapor/Middleware/FileMiddleware.swift +++ b/Sources/Vapor/Middleware/FileMiddleware.swift @@ -23,7 +23,7 @@ public final class FileMiddleware: Middleware { public static let publicDirectoryIsNotAFolder: Self = .init(description: "Cannot find any actual folder for the given Public Directory") } - public struct ETagHashes: StorageKey { + struct ETagHashes: StorageKey { public typealias Value = [String: FileHash] public struct FileHash { diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index d79e0e38e4..1dd63af652 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -224,7 +224,9 @@ public struct FileIO { /// Generates a chunked `Response` for the specified file. This method respects values in /// the `"ETag"` header and is capable of responding `304 Not Modified` if the file in question - /// has not been modified since last served. This method will also set the `"Content-Type"` header + /// has not been modified since last served. If `advancedETagComparison` is set to true, + /// the response will have its ETag field set to a byte-by-byte hash of the requested file. If set to false, a simple ETag consisting of the last modified date and file size + /// will be used. This method will also set the `"Content-Type"` header /// automatically if an appropriate `MediaType` can be found for the file's suffix. /// /// router.get("file-stream") { req in @@ -405,21 +407,19 @@ public struct FileIO { /// - lastModified: When the file was last modified. /// - Returns: An `EventLoopFuture` which holds the ETag. private func generateETagHash(path: String, lastModified: Date) -> EventLoopFuture { - return NIOLock().withLock { - var hashingDictionary = request.application.storage[FileMiddleware.ETagHashes.self] ?? [:] + var hashingDictionary = request.application.storage[FileMiddleware.ETagHashes.self] ?? [:] - if let hash = hashingDictionary[path], hash.lastModified == lastModified { - return request.eventLoop.makeSucceededFuture(hash.digest.hex) - } else { - return collectFile(at: path).map { buffer in - let digest = SHA256.hash(data: buffer.readableBytesView) + if let hash = hashingDictionary[path], hash.lastModified == lastModified { + return request.eventLoop.makeSucceededFuture(hash.digest.hex) + } else { + return collectFile(at: path).map { buffer in + let digest = SHA256.hash(data: buffer.readableBytesView) - // update hash in dictionary - hashingDictionary[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) - request.application.storage[FileMiddleware.ETagHashes.self] = hashingDictionary + // update hash in dictionary + hashingDictionary[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) + request.application.storage[FileMiddleware.ETagHashes.self] = hashingDictionary - return digest.hex - } + return digest.hex } } } diff --git a/Tests/VaporTests/RequestTests.swift b/Tests/VaporTests/RequestTests.swift index de9a6a3782..4cf5f847d3 100644 --- a/Tests/VaporTests/RequestTests.swift +++ b/Tests/VaporTests/RequestTests.swift @@ -261,13 +261,13 @@ final class RequestTests: XCTestCase { app.http.client.configuration.redirectConfiguration = .disallow app.get("redirect_normal") { - $0.redirect(to: "foo", redirectType: .normal) + $0.redirect(to: "foo", type: .normal) } app.get("redirect_permanent") { - $0.redirect(to: "foo", redirectType: .permanent) + $0.redirect(to: "foo", type: .permanent) } app.post("redirect_temporary") { - $0.redirect(to: "foo", redirectType: .temporary) + $0.redirect(to: "foo", type: .temporary) } try app.server.start(address: .hostname("localhost", port: 8080)) From 37f16d1e4379c1b93f907b7959055a12bb6fb0d4 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Thu, 25 May 2023 14:45:22 +0200 Subject: [PATCH 08/13] added test cases to account for advanced/simple etags --- Tests/VaporTests/FileTests.swift | 51 ++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift index e6c888ffa3..38e2c25495 100644 --- a/Tests/VaporTests/FileTests.swift +++ b/Tests/VaporTests/FileTests.swift @@ -3,6 +3,7 @@ import XCTest import Vapor import NIOCore import NIOHTTP1 +import Crypto final class FileTests: XCTestCase { func testStreamFile() throws { @@ -66,6 +67,52 @@ final class FileTests: XCTestCase { XCTAssertTrue(res.body.string.isEmpty) } } + + func testAdvancedETagHeaders() throws { + let app = Application(.testing) + defer { app.shutdown() } + + app.get("file-stream") { req -> EventLoopFuture in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in + do { + try result.get() + } catch { + XCTFail("File Stream should have succeeded") + } + } + } + + try app.testable(method: .running).test(.GET, "/file-stream") { res in + let fileData = try Data(contentsOf: URL(fileURLWithPath: #file)) + let digest = SHA256.hash(data: fileData) + let eTag = res.headers.first(name: "etag") + XCTAssertEqual(eTag, digest.hex) + } + } + + func testSimpleETagHeaders() throws { + let app = Application(.testing) + defer { app.shutdown() } + + app.get("file-stream") { req -> EventLoopFuture in + return req.fileio.streamFile(at: #file, advancedETagComparison: false) { result in + do { + try result.get() + } catch { + XCTFail("File Stream should have succeeded") + } + } + } + + try app.testable(method: .running).test(.GET, "/file-stream") { res in + let attributes = try FileManager.default.attributesOfItem(atPath: #file) + let modifiedAt = attributes[.modificationDate] as! Date + let fileSize = (attributes[.size] as? NSNumber)!.intValue + let fileETag = "\"\(modifiedAt.timeIntervalSince1970)-\(fileSize)\"" + + XCTAssertEqual(res.headers.first(name: .eTag), fileETag) + } + } func testStreamFileContentHeaderTail() throws { let app = Application(.testing) @@ -112,7 +159,7 @@ final class FileTests: XCTestCase { } } } - + var headerRequest = HTTPHeaders() headerRequest.range = .init(unit: .bytes, ranges: [.start(value: 20)]) try app.testable(method: .running).test(.GET, "/file-stream", headers: headerRequest) { res in @@ -168,7 +215,7 @@ final class FileTests: XCTestCase { defer { app.shutdown() } app.get("file-stream") { req in - return req.fileio.streamFile(at: #file) { result in + return req.fileio.streamFile(at: #file, advancedETagComparison: true) { result in do { try result.get() } catch { From 798e40d01bc136a7250270b69a59344699b2820a Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Fri, 19 Apr 2024 21:31:37 +0200 Subject: [PATCH 09/13] Incorporated PR comments - adjusted faulty comment - access storage directly to avoid concurrent overwrites of the entire storage --- Sources/Vapor/Utilities/FileIO.swift | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index 1dd63af652..4238707a4c 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -282,7 +282,7 @@ public struct FileIO { if advancedETagComparison { eTagFuture = generateETagHash(path: path, lastModified: modifiedAt) } else { - // Generate ETag value, "HEX value of last modified date" + "-" + "file size" + // Generate ETag value, "last modified date in epoch time" + "-" + "file size" eTagFuture = request.eventLoop.makeSucceededFuture("\"\(modifiedAt.timeIntervalSince1970)-\(fileSize)\"") } @@ -407,17 +407,14 @@ public struct FileIO { /// - lastModified: When the file was last modified. /// - Returns: An `EventLoopFuture` which holds the ETag. private func generateETagHash(path: String, lastModified: Date) -> EventLoopFuture { - var hashingDictionary = request.application.storage[FileMiddleware.ETagHashes.self] ?? [:] - - if let hash = hashingDictionary[path], hash.lastModified == lastModified { + if let hash = request.application.storage[FileMiddleware.ETagHashes.self]?[path], hash.lastModified == lastModified { return request.eventLoop.makeSucceededFuture(hash.digest.hex) } else { return collectFile(at: path).map { buffer in let digest = SHA256.hash(data: buffer.readableBytesView) - + // update hash in dictionary - hashingDictionary[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) - request.application.storage[FileMiddleware.ETagHashes.self] = hashingDictionary + request.application.storage[FileMiddleware.ETagHashes.self]?[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) return digest.hex } From 0dc971dfcb5eb36c09f65242a51ef51e36d6b266 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Fri, 19 Apr 2024 21:44:21 +0200 Subject: [PATCH 10/13] Added test for legacy streamFile --- Tests/VaporTests/FileTests.swift | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift index 38e2c25495..21fb78fa12 100644 --- a/Tests/VaporTests/FileTests.swift +++ b/Tests/VaporTests/FileTests.swift @@ -27,6 +27,27 @@ final class FileTests: XCTestCase { } } + func testLegacyStreamFile() throws { + let app = Application(.testing) + defer { app.shutdown() } + + app.get("file-stream") { req in + return req.fileio.streamFile(at: #file) { result in + do { + try result.get() + } catch { + XCTFail("File Stream should have succeeded") + } + } + } + + try app.testable(method: .running).test(.GET, "/file-stream") { res in + let test = "the quick brown fox" + XCTAssertNotNil(res.headers.first(name: .eTag)) + XCTAssertContains(res.body.string, test) + } + } + func testStreamFileConnectionClose() throws { let app = Application(.testing) defer { app.shutdown() } From 8631e42a1023571715d16f20171772ece6c92339 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Fri, 19 Apr 2024 23:46:25 +0200 Subject: [PATCH 11/13] Deprecated method to silence warnings --- Tests/VaporTests/FileTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift index ef43418857..0dd9345c63 100644 --- a/Tests/VaporTests/FileTests.swift +++ b/Tests/VaporTests/FileTests.swift @@ -27,6 +27,7 @@ final class FileTests: XCTestCase { } } + @available(*, deprecated) func testLegacyStreamFile() throws { let app = Application(.testing) defer { app.shutdown() } From 23a36b31fb9cd472a0147b65eb8e53eef0cf5367 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sat, 20 Apr 2024 00:17:15 +0200 Subject: [PATCH 12/13] Warning fixes --- .../HTTP/Headers/HTTPHeaders+ContentRange.swift | 6 +++--- Sources/Vapor/Middleware/FileMiddleware.swift | 4 ++-- Sources/Vapor/Utilities/FileIO.swift | 17 +++++++++-------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Sources/Vapor/HTTP/Headers/HTTPHeaders+ContentRange.swift b/Sources/Vapor/HTTP/Headers/HTTPHeaders+ContentRange.swift index 3130786111..493c26b082 100644 --- a/Sources/Vapor/HTTP/Headers/HTTPHeaders+ContentRange.swift +++ b/Sources/Vapor/HTTP/Headers/HTTPHeaders+ContentRange.swift @@ -5,7 +5,7 @@ extension HTTPHeaders { /// The unit in which `ContentRange`s and `Range`s are specified. This is usually `bytes`. /// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range - public enum RangeUnit: Equatable { + public enum RangeUnit: Sendable, Equatable { case bytes case custom(value: String) @@ -21,7 +21,7 @@ extension HTTPHeaders { /// Represents the HTTP `Range` request header. /// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range - public struct Range: Equatable { + public struct Range: Sendable, Equatable { public let unit: RangeUnit public let ranges: [HTTPHeaders.Range.Value] @@ -134,7 +134,7 @@ extension HTTPHeaders.Range { /// Represents one value of the `Range` request header. /// /// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range - public enum Value: Equatable { + public enum Value: Sendable, Equatable { ///Integer with single trailing dash, e.g. `25-` case start(value: Int) ///Integer with single leading dash, e.g. `-25` diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift index 8cbd0593cb..d1c4b6867e 100644 --- a/Sources/Vapor/Middleware/FileMiddleware.swift +++ b/Sources/Vapor/Middleware/FileMiddleware.swift @@ -27,8 +27,8 @@ public final class FileMiddleware: Middleware { public typealias Value = [String: FileHash] public struct FileHash { - var lastModified: Date - var digest: SHA256Digest + let lastModified: Date + let digestHex: String } } diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index 08d6bc65af..2ec8651aae 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -247,7 +247,7 @@ public struct FileIO: Sendable { chunkSize: Int = NonBlockingFileIO.defaultChunkSize, mediaType: HTTPMediaType? = nil, advancedETagComparison: Bool, - onCompleted: @escaping (Result) -> () = { _ in } + onCompleted: @Sendable @escaping (Result) -> () = { _ in } ) -> EventLoopFuture { // Get file attributes for this file. guard @@ -273,11 +273,6 @@ public struct FileIO: Sendable { } else { contentRange = nil } - // Create empty headers array. - var headers: HTTPHeaders = [:] - - // Respond with lastModified header - headers.lastModified = HTTPHeaders.LastModified(value: modifiedAt) var eTagFuture: EventLoopFuture @@ -289,6 +284,12 @@ public struct FileIO: Sendable { } return eTagFuture.map { fileETag in + // Create empty headers array. + var headers: HTTPHeaders = [:] + + // Respond with lastModified header + headers.lastModified = HTTPHeaders.LastModified(value: modifiedAt) + headers.replaceOrAdd(name: .eTag, value: fileETag) // Check if file has been cached already and return NotModified response if the etags match @@ -415,13 +416,13 @@ public struct FileIO: Sendable { /// - Returns: An `EventLoopFuture` which holds the ETag. private func generateETagHash(path: String, lastModified: Date) -> EventLoopFuture { if let hash = request.application.storage[FileMiddleware.ETagHashes.self]?[path], hash.lastModified == lastModified { - return request.eventLoop.makeSucceededFuture(hash.digest.hex) + return request.eventLoop.makeSucceededFuture(hash.digestHex) } else { return collectFile(at: path).map { buffer in let digest = SHA256.hash(data: buffer.readableBytesView) // update hash in dictionary - request.application.storage[FileMiddleware.ETagHashes.self]?[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digest: digest) + request.application.storage[FileMiddleware.ETagHashes.self]?[path] = FileMiddleware.ETagHashes.FileHash(lastModified: lastModified, digestHex: digest.hex) return digest.hex } From 0a9b49f9f5be5961a51af541db57785fbcad69a0 Mon Sep 17 00:00:00 2001 From: Linus <42340116+linus-hologram@users.noreply.github.com> Date: Sat, 20 Apr 2024 00:18:07 +0200 Subject: [PATCH 13/13] following @gwynne's style advice :) --- Sources/Vapor/Utilities/FileIO.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Vapor/Utilities/FileIO.swift b/Sources/Vapor/Utilities/FileIO.swift index 2ec8651aae..7a3820989f 100644 --- a/Sources/Vapor/Utilities/FileIO.swift +++ b/Sources/Vapor/Utilities/FileIO.swift @@ -247,7 +247,7 @@ public struct FileIO: Sendable { chunkSize: Int = NonBlockingFileIO.defaultChunkSize, mediaType: HTTPMediaType? = nil, advancedETagComparison: Bool, - onCompleted: @Sendable @escaping (Result) -> () = { _ in } + onCompleted: @escaping @Sendable (Result) -> () = { _ in } ) -> EventLoopFuture { // Get file attributes for this file. guard