diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 54768e6db..5e206c466 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -48,7 +48,7 @@ final class MediaUploadServer: Sendable { cleanOrphanedUploads() } - let context = UploadContext(processor: processor, uploader: uploader, internalClient: internalClient) + let handler = Handler(processor: processor, uploader: uploader, internalClient: internalClient) // A generous ceiling for receiving the upload body. The body read is // primarily bounded by the per-read idle timeout (which reaps a stalled @@ -65,9 +65,7 @@ final class MediaUploadServer: Sendable { bodyReadTimeout: bodyReadTimeout, cors: .permissive, delegate: ServerDelegate(), - handler: { request in - await Self.handleRequest(request, context: context) - } + handler: handler ) let uploadServer = MediaUploadServer(server: server, cleanupTask: cleanupTask) @@ -150,298 +148,327 @@ final class MediaUploadServer: Sendable { // MARK: - Request Handling - private static func handleRequest(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { - let parsed = request.parsed + /// Serves the upload server's requests. + /// + /// A `struct` rather than a closure over a context object: the dependencies become + /// stored properties and the request logic becomes instance methods, instead of + /// statics threading a context parameter through every call. It stores no reference + /// back to the `MediaUploadServer`, so it can't close the + /// `MediaUploadServer -> HTTPServer -> handler -> MediaUploadServer` loop that + /// would keep `deinit` — and therefore `stop()` — from ever running. Being a value + /// type is not what buys that: a `struct` storing the server would close the loop + /// just the same, which is why the statics above stay static. + /// + /// Everything here is held **strongly**, so a processor that admitted a file for + /// processing will process it, and an upload gated on a host uploader will be + /// delivered by it — the reads within a request can't disagree, and an in-flight + /// upload keeps the host's handlers alive until it unwinds. This matches Android, + /// which holds its `processor`/`uploader` as plain `val`s for the same reason. + /// + /// Strong is safe because `EditorViewController` owns `mediaProcessor` and + /// `mediaUploader` strongly too. A host object that retains the view controller + /// back already forms `EditorViewController -> mediaUploader -> + /// EditorViewController`, a cycle this handler can neither create nor prevent. + /// + /// Implicitly `Sendable`: `MediaProcessor` and `MediaUploader` are `Sendable` + /// protocols and `InternalMediaClient` is `@unchecked Sendable`. + private struct Handler: HTTPRequestHandler { + let processor: (any MediaProcessor)? + let uploader: (any MediaUploader)? + let internalClient: InternalMediaClient? + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + let parsed = request.parsed + + // Routes: POST /upload, and DELETE /media/ for the editor's orphan + // cleanup. (OPTIONS preflight is answered by the HTTP library under its + // permissive CORS policy.) Match on the path alone — the target carries + // a query string (e.g. `?_embed`, `?force=true`) relayed to WordPress. + let method = parsed.method.uppercased() + + if method == "POST", parsed.path == "/upload" { + return await handleUpload(request) + } - // Routes: POST /upload, and DELETE /media/ for the editor's orphan - // cleanup. (OPTIONS preflight is answered by the HTTP library under its - // permissive CORS policy.) Match on the path alone — the target carries - // a query string (e.g. `?_embed`, `?force=true`) relayed to WordPress. - let method = parsed.method.uppercased() + if method == "DELETE", let attachmentId = Self.attachmentId(fromPath: parsed.path) { + return await handleDelete(attachmentId, query: parsed.query) + } - if method == "POST", parsed.path == "/upload" { - return await handleUpload(request, context: context) + return MediaUploadServer.errorResponse(status: 404, message: "Not found") } - if method == "DELETE", let attachmentId = attachmentId(fromPath: parsed.path) { - return await handleDelete(attachmentId, query: parsed.query, internalClient: context.internalClient) - } + private func handleUpload(_ request: HTTPServer.Request) async -> HTTPResponse { + let parts: [MultipartPart] + do { + parts = try request.parsed.multipartParts() + } catch { + Logger.uploadServer.error("Multipart parse failed: \(error)") + return MediaUploadServer.errorResponse(status: 400, message: "Expected multipart/form-data") + } - return errorResponse(status: 404, message: "Not found") - } + // Find the file part (the first part with a filename). + guard let filePart = parts.first(where: { $0.filename != nil }) else { + return MediaUploadServer.errorResponse(status: 400, message: "No file found in request") + } - private static func handleUpload(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { - let parts: [MultipartPart] - do { - parts = try request.parsed.multipartParts() - } catch { - Logger.uploadServer.error("Multipart parse failed: \(error)") - return errorResponse(status: 400, message: "Expected multipart/form-data") - } + // The non-file parts (post, additionalData) and the original query + // (e.g. ?_embed) must reach WordPress too — relay them alongside the file. + let extraParts = parts.filter { $0.filename == nil } + let query = request.parsed.query + + let filename = filePart.filename ?? "upload" + let mimeType = filePart.contentType + + // Ask the processor — from metadata alone — whether it will touch a file like + // this. If not, forward the original upload to WordPress directly, skipping a + // full temp-file copy of a file the processor won't process (e.g. a video handed + // to an image-only processor). + // + // An uploader takes over delivery for *every* file, so with one set there is + // no passthrough to fall to and the gate can't decline the upload outright. + // It still decides whether `processFile` runs, though — a declined file is + // handed to the uploader unprocessed rather than to a processor that said it + // won't touch it — so the answer is carried into `processAndUpload` rather + // than discarded here. Asked exactly once per upload, matching Android. + let processorWantsFile = processor?.handlesFile(ofType: mimeType, named: filename) ?? false + guard uploader != nil || processorWantsFile else { + do { + return try await passthroughResponse(request, query: query) + } catch { + return Self.uploadErrorResponse(error) + } + } - // Find the file part (the first part with a filename). - guard let filePart = parts.first(where: { $0.filename != nil }) else { - return errorResponse(status: 400, message: "No file found in request") - } + // Someone wants the file — the processor, the uploader, or both. Stream the + // part body to a dedicated temp file for them: the library's RequestBody may + // be a byte-range slice of a larger temp file whose lifecycle is tied to ARC, + // so they need a standalone file that outlives the handler return. + let tempDir = MediaUploadServer.uploadsTempDirectory + try? FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - // The non-file parts (post, additionalData) and the original query - // (e.g. ?_embed) must reach WordPress too — relay them alongside the file. - let extraParts = parts.filter { $0.filename == nil } - let query = request.parsed.query - - let filename = filePart.filename ?? "upload" - let mimeType = filePart.contentType - - // Ask the processor — from metadata alone — whether it will touch a file like - // this. If not, forward the original upload to WordPress directly, skipping a - // full temp-file copy of a file the processor won't process (e.g. a video handed - // to an image-only processor). - // - // An uploader takes over delivery for *every* file, so with one set there is - // no passthrough to fall to and the gate can't decline the upload outright. - // It still decides whether `processFile` runs, though — a declined file is - // handed to the uploader unprocessed rather than to a processor that said it - // won't touch it — so the answer is carried into `processAndUpload` rather - // than discarded here. Asked exactly once per upload, matching Android. - let processorWantsFile = context.processor?.handlesFile(ofType: mimeType, named: filename) ?? false - guard context.uploader != nil || processorWantsFile else { + let fileURL = tempDir.appending(component: "\(UUID().uuidString)-\(MediaUploadServer.sanitizeFilename(filename))") do { - return try await passthroughResponse(request, query: query, internalClient: context.internalClient) + let inputStream = try filePart.body.makeInputStream() + try MediaUploadServer.writeStream(inputStream, to: fileURL) } catch { - return uploadErrorResponse(error) + try? FileManager.default.removeItem(at: fileURL) + Logger.uploadServer.error("Failed to write upload to disk: \(error)") + return MediaUploadServer.errorResponse(status: 500, message: "Failed to save file") } - } - // Someone wants the file — the processor, the uploader, or both. Stream the - // part body to a dedicated temp file for them: the library's RequestBody may - // be a byte-range slice of a larger temp file whose lifecycle is tied to ARC, - // so they need a standalone file that outlives the handler return. - let tempDir = uploadsTempDirectory - try? FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - - let fileURL = tempDir.appending(component: "\(UUID().uuidString)-\(sanitizeFilename(filename))") - do { - let inputStream = try filePart.body.makeInputStream() - try writeStream(inputStream, to: fileURL) - } catch { - try? FileManager.default.removeItem(at: fileURL) - Logger.uploadServer.error("Failed to write upload to disk: \(error)") - return errorResponse(status: 500, message: "Failed to save file") - } - - // From here on always clean up the original temp file. The processed - // file (if the processor produced a new one) is cleaned up inside - // processAndUpload so its throw paths are covered too. - defer { try? FileManager.default.removeItem(at: fileURL) } + // From here on always clean up the original temp file. The processed + // file (if the processor produced a new one) is cleaned up inside + // processAndUpload so its throw paths are covered too. + defer { try? FileManager.default.removeItem(at: fileURL) } - do { - let uploadResult = try await processAndUpload( - fileURL: fileURL, mimeType: mimeType, filename: filename, - extraParts: extraParts, query: query, - processorWantsFile: processorWantsFile, context: context - ) - switch uploadResult { - case .uploaded(let uploaded): - Logger.uploadServer.debug("Uploaded file to WordPress") - return relayResponse(uploaded) - case .passthrough: - // The processor didn't modify the file — forward the original request - // body to WordPress without re-encoding. - return try await passthroughResponse(request, query: query, internalClient: context.internalClient) + do { + let uploadResult = try await processAndUpload( + fileURL: fileURL, mimeType: mimeType, filename: filename, + extraParts: extraParts, query: query, + processorWantsFile: processorWantsFile + ) + switch uploadResult { + case .uploaded(let uploaded): + Logger.uploadServer.debug("Uploaded file to WordPress") + return Self.relayResponse(uploaded) + case .passthrough: + // The processor didn't modify the file — forward the original request + // body to WordPress without re-encoding. + return try await passthroughResponse(request, query: query) + } + } catch { + return Self.uploadErrorResponse(error) } - } catch { - return uploadErrorResponse(error) - } - } - - /// Forwards the original request body to WordPress unchanged (no multipart - /// re-encoding) and relays the response. Used when the processor won't touch - /// the file — it declined by metadata (`handlesFile` returned false) or - /// `processFile` returned `.original`. - private static func passthroughResponse( - _ request: HTTPServer.Request, query: String, internalClient: InternalMediaClient? - ) async throws -> HTTPResponse { - // As in `processAndUpload`: don't put bytes on the wire for a torn-down - // editor, regardless of whether the HTTP client honors cancellation. - try Task.checkCancellation() - - Logger.uploadServer.debug("Passthrough: forwarding original request body to WordPress") - guard let body = request.parsed.body, - let contentType = request.parsed.header("Content-Type"), - let internalClient else { - return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) } - let response = try await internalClient.passthroughUpload(body: body, contentType: contentType, query: query) - return relayResponse(response) - } - /// The attachment ID in a `/media/` path, or `nil` if the path is not one. - /// - /// Deliberately narrow: this server relays media operations, not arbitrary - /// REST requests, so only a numeric attachment ID under `/media/` matches. - private static func attachmentId(fromPath path: String) -> String? { - let components = path.split(separator: "/", omittingEmptySubsequences: true) - guard components.count == 2, components[0] == "media" else { return nil } - let id = String(components[1]) - guard !id.isEmpty, id.allSatisfy(\.isNumber) else { return nil } - return id - } - - /// Relays the editor's orphan cleanup to WordPress. - /// - /// Core's media upload middleware deletes the attachment when every - /// `post-process` retry fails. A cross-origin editor cannot issue that - /// request directly — api-fetch tunnels `DELETE` as a `POST` carrying - /// `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the - /// browser blocks it at preflight. Relaying it here lets the cleanup run. - private static func handleDelete( - _ attachmentId: String, query: String, internalClient: InternalMediaClient? - ) async -> HTTPResponse { - guard let internalClient else { - return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) - } - do { - let response = try await internalClient.deleteMedia(attachmentId: attachmentId, query: query) - return relayResponse(response) - } catch { - return uploadErrorResponse(error) + /// Forwards the original request body to WordPress unchanged (no multipart + /// re-encoding) and relays the response. Used when the processor won't touch + /// the file — it declined by metadata (`handlesFile` returned false) or + /// `processFile` returned `.original`. + private func passthroughResponse( + _ request: HTTPServer.Request, query: String + ) async throws -> HTTPResponse { + // As in `processAndUpload`: don't put bytes on the wire for a torn-down + // editor, regardless of whether the HTTP client honors cancellation. + try Task.checkCancellation() + + Logger.uploadServer.debug("Passthrough: forwarding original request body to WordPress") + guard let body = request.parsed.body, + let contentType = request.parsed.header("Content-Type"), + let internalClient else { + return MediaUploadServer.errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) + } + let response = try await internalClient.passthroughUpload(body: body, contentType: contentType, query: query) + return Self.relayResponse(response) + } + + /// The attachment ID in a `/media/` path, or `nil` if the path is not one. + /// + /// Deliberately narrow: this server relays media operations, not arbitrary + /// REST requests, so only a numeric attachment ID under `/media/` matches. + private static func attachmentId(fromPath path: String) -> String? { + let components = path.split(separator: "/", omittingEmptySubsequences: true) + guard components.count == 2, components[0] == "media" else { return nil } + let id = String(components[1]) + guard !id.isEmpty, id.allSatisfy(\.isNumber) else { return nil } + return id + } + + /// Relays the editor's orphan cleanup to WordPress. + /// + /// Core's media upload middleware deletes the attachment when every + /// `post-process` retry fails. A cross-origin editor cannot issue that + /// request directly — api-fetch tunnels `DELETE` as a `POST` carrying + /// `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the + /// browser blocks it at preflight. Relaying it here lets the cleanup run. + private func handleDelete( + _ attachmentId: String, query: String + ) async -> HTTPResponse { + guard let internalClient else { + return MediaUploadServer.errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) + } + do { + let response = try await internalClient.deleteMedia(attachmentId: attachmentId, query: query) + return Self.relayResponse(response) + } catch { + return Self.uploadErrorResponse(error) + } } - } - - /// Relays WordPress's exact status, body, and relayable headers to the editor - /// so it sees the same attachment object (or error) as a direct upload. - /// - /// The headers matter for recovery: `x-wp-upload-attachment-id` is what lets - /// the editor retry `post-process` for an upload whose metadata generation - /// fataled server-side, rather than surfacing a permanent failure and - /// leaving an orphaned attachment behind. - /// - /// The response's own `Content-Type` wins over the JSON default. `HTTPResponse` - /// serializes every header it is given, so appending the default unconditionally - /// would emit the name twice for a processor that sets it. - private static func relayResponse(_ response: MediaUploadResponse) -> HTTPResponse { - let hasContentType = response.headers.keys.contains { $0.lowercased() == "content-type" } - return HTTPResponse( - status: response.statusCode, - headers: (hasContentType ? [] : [("Content-Type", "application/json")]) - + response.headers.map { ($0.key, $0.value) }, - body: response.body - ) - } - /// Builds the 500 response for a failed upload. A cancelled connection task - /// (editor abort / server stop) surfaces here too — as CancellationError or - /// URLError.cancelled — but isn't a failure and the server closes the - /// connection without sending this response (see HTTPServer's cancellation - /// check), so log that quietly. - private static func uploadErrorResponse(_ error: any Error) -> HTTPResponse { - if Task.isCancelled { - Logger.uploadServer.debug("Upload cancelled") - } else { - Logger.uploadServer.error("Upload processing failed: \(error)") + /// Relays WordPress's exact status, body, and relayable headers to the editor + /// so it sees the same attachment object (or error) as a direct upload. + /// + /// The headers matter for recovery: `x-wp-upload-attachment-id` is what lets + /// the editor retry `post-process` for an upload whose metadata generation + /// fataled server-side, rather than surfacing a permanent failure and + /// leaving an orphaned attachment behind. + /// + /// The response's own `Content-Type` wins over the JSON default. `HTTPResponse` + /// serializes every header it is given, so appending the default unconditionally + /// would emit the name twice for a processor that sets it. + private static func relayResponse(_ response: MediaUploadResponse) -> HTTPResponse { + let hasContentType = response.headers.keys.contains { $0.lowercased() == "content-type" } + return HTTPResponse( + status: response.statusCode, + headers: (hasContentType ? [] : [("Content-Type", "application/json")]) + + response.headers.map { ($0.key, $0.value) }, + body: response.body + ) } - return errorResponse(status: 500, message: error.localizedDescription) - } - - // MARK: - Processor Pipeline - - /// Result of the processing + upload pipeline. - private enum UploadResult { - /// The uploader or internal media client completed the upload; - /// carries the raw WordPress response to relay. - case uploaded(MediaUploadResponse) - /// The processor didn't modify the file, so the original body is forwarded. - /// The caller should forward the original request body to WordPress. - case passthrough - } - private static func processAndUpload( - fileURL: URL, mimeType: String, filename: String, - extraParts: [MultipartPart], query: String, - processorWantsFile: Bool, context: UploadContext - ) async throws -> UploadResult { - // Step 1: Process (resize, transcode, etc.) — but only for a file the - // processor's metadata gate accepted. `handlesFile` returning false is the - // processor saying it won't touch a file like this, so handing it one anyway - // would break the contract the gate documents. With an uploader set the file - // still gets delivered; it just skips processing on its way there. - let processed: ProcessedProxyFile - if let processor = context.processor, processorWantsFile { - processed = try await processor.processFile(at: fileURL, mimeType: mimeType, filename: filename) - } else { - processed = .original - } + /// Builds the 500 response for a failed upload. A cancelled connection task + /// (editor abort / server stop) surfaces here too — as CancellationError or + /// URLError.cancelled — but isn't a failure and the server closes the + /// connection without sending this response (see HTTPServer's cancellation + /// check), so log that quietly. + private static func uploadErrorResponse(_ error: any Error) -> HTTPResponse { + if Task.isCancelled { + Logger.uploadServer.debug("Upload cancelled") + } else { + Logger.uploadServer.error("Upload processing failed: \(error)") + } + return MediaUploadServer.errorResponse(status: 500, message: error.localizedDescription) + } + + // MARK: - Processor Pipeline + + /// Result of the processing + upload pipeline. + private enum UploadResult { + /// The uploader or internal media client completed the upload; + /// carries the raw WordPress response to relay. + case uploaded(MediaUploadResponse) + /// The processor didn't modify the file, so the original body is forwarded. + /// The caller should forward the original request body to WordPress. + case passthrough + } + + private func processAndUpload( + fileURL: URL, mimeType: String, filename: String, + extraParts: [MultipartPart], query: String, processorWantsFile: Bool + ) async throws -> UploadResult { + // Step 1: Process (resize, transcode, etc.) — but only for a file the + // processor's metadata gate accepted. `handlesFile` returning false is the + // processor saying it won't touch a file like this, so handing it one anyway + // would break the contract the gate documents. With an uploader set the file + // still gets delivered; it just skips processing on its way there. + let processed: ProcessedProxyFile + if let processor, processorWantsFile { + processed = try await processor.processFile(at: fileURL, mimeType: mimeType, filename: filename) + } else { + processed = .original + } - // Resolve the file to upload and its metadata. `.processed` uses the - // processor's values verbatim, so a format change is reported to WordPress. - let uploadURL: URL - let uploadMimeType: String - let uploadFilename: String - switch processed { - case .original: - uploadURL = fileURL - uploadMimeType = mimeType - uploadFilename = filename - case let .processed(url, processedMimeType, processedFilename): - uploadURL = url - uploadMimeType = processedMimeType - uploadFilename = processedFilename - } + // Resolve the file to upload and its metadata. `.processed` uses the + // processor's values verbatim, so a format change is reported to WordPress. + let uploadURL: URL + let uploadMimeType: String + let uploadFilename: String + switch processed { + case .original: + uploadURL = fileURL + uploadMimeType = mimeType + uploadFilename = filename + case let .processed(url, processedMimeType, processedFilename): + uploadURL = url + uploadMimeType = processedMimeType + uploadFilename = processedFilename + } - // The processed file (if the processor produced a new one) is ours to - // clean up — on success it has been uploaded, on failure it is abandoned. - // Cleaning up here rather than in the caller covers the throw paths too. - defer { - if uploadURL != fileURL { - try? FileManager.default.removeItem(at: uploadURL) + // The processed file (if the processor produced a new one) is ours to + // clean up — on success it has been uploaded, on failure it is abandoned. + // Cleaning up here rather than in the caller covers the throw paths too. + defer { + if uploadURL != fileURL { + try? FileManager.default.removeItem(at: uploadURL) + } } - } - // The editor was torn down (or the client disconnected) while we processed. - // Don't start an outbound upload whose response nobody will read — it would - // create an attachment neither GutenbergKit nor the host knows to clean up. - // Checking here rather than relying on the HTTP client to notice cancellation - // keeps this true for a host-injected `URLSessionProtocol` that doesn't. - try Task.checkCancellation() - - // Step 2: deliver. An uploader owns delivery on the host's own stack and - // returns the finished attachment JSON (or throws); GutenbergKit relays that - // as a success and never runs its own recovery behind it. - if let uploader = context.uploader { - let upload = MediaUpload( - fileURL: uploadURL, - mimeType: uploadMimeType, - filename: uploadFilename, - fields: try await formFields(from: extraParts), - query: query - ) - let attachment = try await uploader.upload(upload) - return .uploaded(MediaUploadResponse(statusCode: 201, body: attachment)) - } + // The editor was torn down (or the client disconnected) while we processed. + // Don't start an outbound upload whose response nobody will read — it would + // create an attachment neither GutenbergKit nor the host knows to clean up. + // Checking here rather than relying on the HTTP client to notice cancellation + // keeps this true for a host-injected `URLSessionProtocol` that doesn't. + try Task.checkCancellation() + + // Step 2: deliver. An uploader owns delivery on the host's own stack and + // returns the finished attachment JSON (or throws); GutenbergKit relays that + // as a success and never runs its own recovery behind it. + if let uploader { + let upload = MediaUpload( + fileURL: uploadURL, + mimeType: uploadMimeType, + filename: uploadFilename, + fields: try await Self.formFields(from: extraParts), + query: query + ) + let attachment = try await uploader.upload(upload) + return .uploaded(MediaUploadResponse(statusCode: 201, body: attachment)) + } - if let internalClient = context.internalClient { - // Unmodified — forward the original request body directly, skipping - // multipart re-encoding. - if case .original = processed { - return .passthrough + if let internalClient { + // Unmodified — forward the original request body directly, skipping + // multipart re-encoding. + if case .original = processed { + return .passthrough + } + let result = try await internalClient.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) + return .uploaded(result) + } else { + throw UploadError.noUploader } - let result = try await internalClient.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) - return .uploaded(result) - } else { - throw UploadError.noUploader } - } - /// The editor's non-file form parts as ordered, UTF-8-decoded fields. - /// - /// A list rather than a dictionary so repeated names (e.g. a `field[]` array) - /// survive verbatim, in the order the editor sent them. - private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { - var fields: [MediaUploadField] = [] - for part in parts { - fields.append(MediaUploadField(name: part.name, value: String(decoding: try await part.body.data, as: UTF8.self))) + /// The editor's non-file form parts as ordered, UTF-8-decoded fields. + /// + /// A list rather than a dictionary so repeated names (e.g. a `field[]` array) + /// survive verbatim, in the order the editor sent them. + private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { + var fields: [MediaUploadField] = [] + for part in parts { + fields.append(MediaUploadField(name: part.name, value: String(decoding: try await part.body.data, as: UTF8.self))) + } + return fields } - return fields } private static func errorResponse(status: Int, message: String) -> HTTPResponse { @@ -564,36 +591,6 @@ enum UploadError: Error, LocalizedError { } } -// MARK: - Upload Context - -/// Container for the media processor, host uploader, and internal media client, -/// captured by the HTTPServer handler closure and read on each request. -/// -/// All are held **strongly**, so a processor that admitted a file for processing -/// will process it — the three reads within a request can't disagree, and an -/// in-flight upload keeps the host's processor alive until it unwinds. That lifetime -/// comes from the handler closure, which the listener retains for the server's -/// lifetime; it therefore holds just as well on the paths that take the client -/// alone rather than the whole context. This matches Android, which holds its -/// `processor` as a plain `val` for the same reason. -/// -/// Strong is safe *given* `EditorViewController` now owns `mediaProcessor` -/// strongly too — but be exact about what that trades away. Weak here did break one -/// ring: every other edge in `EditorViewController → uploadServer → HTTPServer → -/// listener → newConnectionHandler → handler → UploadContext → processor` is strong, -/// so this was its only weak link. What it could not break is the shorter ring -/// straight through the property. A host that retains the view controller back now -/// leaks either way, so weak here buys a partial guard in exchange for the processor -/// vanishing mid-request — which is the failure that was actually being hit. -/// -/// A `struct`, so it is implicitly `Sendable`: `MediaProcessor` and `MediaUploader` -/// are `Sendable` protocols and `InternalMediaClient` is `@unchecked Sendable`. -private struct UploadContext: Sendable { - let processor: (any MediaProcessor)? - let uploader: (any MediaUploader)? - let internalClient: InternalMediaClient? -} - // MARK: - Internal Media Client /// GutenbergKit's own client for the configured site, built from the site credentials @@ -601,8 +598,8 @@ private struct UploadContext: Sendable { /// /// Not an implementation of any host-facing protocol — it is the thing that actually /// performs GutenbergKit's media requests. It delivers uploads the host did not take -/// over, and relays the editor's media deletes: the editor only ever asks to delete -/// `/wp/v2/media/` on the configured site, so that is where the relay sends it. +/// over, and relays the editor's media deletes: every attachment lives on the +/// configured site, so that is where its deletion goes. class InternalMediaClient: @unchecked Sendable { private let httpClient: EditorHTTPClientProtocol private let siteApiRoot: URL diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift new file mode 100644 index 000000000..e4ec8c020 --- /dev/null +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift @@ -0,0 +1,42 @@ +#if canImport(Network) + +import Foundation + +/// Serves requests for an ``HTTPServer``. +/// +/// The closure form of +/// ``HTTPServer/start(name:port:listenOnAllInterfaces:requiresAuthentication:maxRequestBodySize:maxConnections:readTimeout:bodyReadTimeout:idleTimeout:startTimeout:cors:delegate:handler:)-(_,_,_,_,_,_,_,_,_,_,_,_,@escaping@Sendable(HTTPServer.Request)async->HTTPResponse)`` +/// is the right tool for a handler that needs no state. Conform to this instead when +/// the handler has dependencies: they become stored properties, and the request +/// methods become ordinary instance methods rather than statics threading a context +/// parameter through every call. +/// +/// ## Lifetimes +/// +/// The server retains its handler for its lifetime, so a handler must not strongly +/// hold the object that owns the server, directly or transitively: +/// `owner → HTTPServer → handler → owner` is a cycle, the owner's `deinit` never runs, +/// and `stop()` is never called — a silently stranded listener, not a crash. +/// +/// A value type is **not** protection. A `struct` handler storing the owner closes the +/// same ring: the server captures the struct into a heap node, and its stored properties +/// are strong edges out of it. This protocol is deliberately **not** `AnyObject`-constrained +/// so a handler *can* be a `struct` holding only what it needs — not because a `struct` is +/// safe by construction. Either shape works; both must stay leaves, the same discipline +/// ``HTTPServerDelegate`` documents. +/// +/// The usual trap is the object that starts the server also serving it — a view controller +/// starting it in `viewDidLoad` and stopping it in `deinit` is the shape that bites, because +/// the cycle disables the very teardown meant to break it. Conform a separate leaf type, or +/// call `stop()` from a hook that does run. +public protocol HTTPRequestHandler: Sendable { + /// The response for a request the server has parsed and authenticated. + /// + /// Called once per request, concurrently across connections — hence `Sendable`. + /// Cancellation is cooperative: the server cancels this task when the client + /// disconnects or the server stops, and discards whatever a cancelled task + /// returns, so check `Task.isCancelled` before any side effect you can't undo. + func handle(_ request: HTTPServer.Request) async -> HTTPResponse +} + +#endif // canImport(Network) diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index ae5f0a33d..be7001e18 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -277,6 +277,46 @@ public final class HTTPServer: Sendable { } } + /// Starts a server that serves requests from an ``HTTPRequestHandler`` object + /// rather than a closure. + /// + /// Everything else behaves identically — this forwards to the closure form. Reach + /// for it when the handler has dependencies to hold: a `struct` conformer stores + /// them and serves from instance methods, instead of statics threading a context + /// parameter through every call. See ``HTTPRequestHandler`` for the (short) + /// lifetime rules. + public static func start( + name: String, + port: UInt16? = nil, + listenOnAllInterfaces: Bool = false, + requiresAuthentication: Bool = true, + maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize, + maxConnections: Int = HTTPServer.defaultMaxConnections, + readTimeout: Duration = HTTPServer.defaultReadTimeout, + bodyReadTimeout: Duration? = nil, + idleTimeout: Duration = HTTPServer.defaultIdleTimeout, + startTimeout: Duration = HTTPServer.defaultStartTimeout, + cors: CORSPolicy = .none, + delegate: HTTPServerDelegate? = nil, + handler: some HTTPRequestHandler + ) async throws -> HTTPServer { + try await start( + name: name, + port: port, + listenOnAllInterfaces: listenOnAllInterfaces, + requiresAuthentication: requiresAuthentication, + maxRequestBodySize: maxRequestBodySize, + maxConnections: maxConnections, + readTimeout: readTimeout, + bodyReadTimeout: bodyReadTimeout, + idleTimeout: idleTimeout, + startTimeout: startTimeout, + cors: cors, + delegate: delegate, + handler: { await handler.handle($0) } + ) + } + /// Races `operation` against `timeout`, throwing ``HTTPServerError/startTimeout`` /// if the timeout wins. Used to bound the wait for the listener to become ready /// so a caller — such as the editor load awaiting the upload server's bind — diff --git a/ios/Sources/GutenbergKitHTTP/README.md b/ios/Sources/GutenbergKitHTTP/README.md index 43721b305..86c14e380 100644 --- a/ios/Sources/GutenbergKitHTTP/README.md +++ b/ios/Sources/GutenbergKitHTTP/README.md @@ -46,6 +46,24 @@ server.stop() Pass `nil` (or omit `port`) to let the system assign an available port — useful for tests or when running multiple servers. +#### Handlers with state + +A closure is right for a handler that needs no state. When the handler has dependencies, conform a type to `HTTPRequestHandler` and pass it as `handler:` instead — the dependencies become stored properties and the request logic becomes instance methods, rather than statics threading a context parameter through every call. + +```swift +struct MediaHandler: HTTPRequestHandler { + let uploader: Uploader + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + await uploader.upload(request.parsed.body) + } +} + +let server = try await HTTPServer.start(name: "media", handler: MediaHandler(uploader: uploader)) +``` + +The server retains its handler, so a handler must not strongly hold the object that owns the server, directly or transitively — `owner → HTTPServer → handler → owner` is a cycle, and the owner's `deinit` would never run. A value type is **not** protection here: a `struct` handler storing the owner closes the same ring, because the server captures the struct into a heap node and its stored properties are strong edges out of it. `HTTPRequestHandler` is deliberately not `AnyObject`-constrained so a handler *can* be a `struct` holding only what it needs — not because a `struct` is safe by construction. Either shape works, as long as it stays a leaf. + When `requiresAuthentication` is enabled (the default), each request must include a `Proxy-Authorization: Bearer ` header carrying the server's randomly-generated token. The server uses `Proxy-Authorization` per RFC 9110 §11.7.1 rather than `Authorization`, so the client's `Authorization` header remains available for upstream credentials (e.g. HTTP Basic auth to the remote server). Unauthenticated requests receive a `407 Proxy Authentication Required` response with a `Proxy-Authenticate: Bearer` challenge header. ### Proxying via URLSession diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift index 17be7d603..fc86f22a9 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift @@ -38,6 +38,36 @@ struct HTTPServerStartTests { // rather than suspending its caller indefinitely. #expect(elapsed < .seconds(5)) } + + @Test("serves requests from an HTTPRequestHandler object, carrying its state") + func servesFromRequestHandlerObject() async throws { + // The point of the object overload: the handler holds its dependencies as + // stored properties and serves from an instance method, so a consumer with + // state doesn't need statics threading a context through every call. + let server = try await HTTPServer.start( + name: "handler-object-test", + requiresAuthentication: false, + handler: EchoHandler(greeting: "hello from a struct") + ) + defer { server.stop() } + + let url = URL(string: "http://127.0.0.1:\(server.port)/anything")! + let (data, response) = try await URLSession.shared.data(from: url) + + #expect((response as? HTTPURLResponse)?.statusCode == 200) + #expect(String(decoding: data, as: UTF8.self) == "hello from a struct") + } +} + +/// A value-type handler, holding only a `String` — so it has no strong edge back to +/// whatever owns the server. ``HTTPRequestHandler`` isn't `AnyObject`-constrained so a +/// handler *can* take this shape; a `struct` storing the owner would still cycle. +private struct EchoHandler: HTTPRequestHandler { + let greeting: String + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + HTTPResponse(status: 200, body: Data(greeting.utf8)) + } } #endif diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 50d0a9f1d..841e53285 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -194,7 +194,7 @@ struct MediaUploadServerTests { /// conform, and the server actually calls it. /// /// Every other conformer in the tree is a class, so without this nothing exercises the - /// boxed-existential path — copied into `UploadContext`, captured by the `@Sendable` + /// boxed-existential path — copied into `Handler`, captured by the `@Sendable` /// handler closure, read again at `processFile`. Re-imposing a class requirement, or /// breaking that path, would otherwise compile and pass green and surface only in a /// host's build. @@ -597,7 +597,7 @@ struct MediaUploadServerTests { // The server-side half of the ownership story, and the one nothing else covers. // `EditorViewController.stopMediaHandling()` clears its own properties *and* stops // the server, because releasing only one leaves the loop routed through the other: - // `listener -> newConnectionHandler -> handler -> UploadContext -> processor -> server`. + // `listener -> newConnectionHandler -> Handler -> processor -> server`. // // Polled rather than asserted outright, unlike `retainsProcessorForServerLifetime`: // `releaseConnectionHandler()` opens the loop on the caller's thread, but it is not