diff --git a/Modules/Package.resolved b/Modules/Package.resolved index b36c195bea18..6a70c94bb00e 100644 --- a/Modules/Package.resolved +++ b/Modules/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "97462f04f7472535df3293d3f7601aaff8a4684769091394d4b1ca5bde9f8ed1", + "originHash" : "61e4f4fbfd341181ca9b893646e997a1b3b76afe110c2500eb4b500401ef1e34", "pins" : [ { "identity" : "alamofire", @@ -131,8 +131,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/wordpress-mobile/GutenbergKit", "state" : { - "revision" : "b6604e26792725b2e8125b3b715e1d4a298441b6", - "version" : "0.19.0" + "revision" : "297fcb55e3694c4f325468386972ac89f414cc70", + "version" : "0.20.0-alpha.0" } }, { diff --git a/Modules/Package.swift b/Modules/Package.swift index 4b1217724507..ec15ba8a087f 100644 --- a/Modules/Package.swift +++ b/Modules/Package.swift @@ -62,7 +62,7 @@ let package = Package( revision: "b34794c9a3f32312e1593d4a3d120572afa0d010" ), .package(url: "https://github.com/zendesk/support_sdk_ios", from: "8.0.3"), - .package(url: "https://github.com/wordpress-mobile/GutenbergKit", from: "0.19.0"), + .package(url: "https://github.com/wordpress-mobile/GutenbergKit", from: "0.20.0-alpha.0"), .package( url: "https://github.com/automattic/wordpress-rs", exact: "0.6.0" diff --git a/Package.resolved b/Package.resolved index e4f561d43c78..bba5f23b413e 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "e5b9b888f12b9e2adfe5e293101989f58ec52d16d70ef0002c28093d8c2ed39f", + "originHash" : "1bbbb11e671a32eda51b479a3e29fd18839bb400ff80395fba2ed7a8553e9baa", "pins" : [ { "identity" : "alamofire", @@ -131,8 +131,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/wordpress-mobile/GutenbergKit", "state" : { - "revision" : "7180587f49d3c3bfdb34cc3e80b2a9d22a3cd93e", - "version" : "0.18.1" + "revision" : "297fcb55e3694c4f325468386972ac89f414cc70", + "version" : "0.20.0-alpha.0" } }, { diff --git a/Tests/KeystoneTests/Tests/Features/Gutenberg/PostGBKEditorViewControllerTests.swift b/Tests/KeystoneTests/Tests/Features/Gutenberg/PostGBKEditorViewControllerTests.swift index 9b9125c082f2..ea8d4bf980f4 100644 --- a/Tests/KeystoneTests/Tests/Features/Gutenberg/PostGBKEditorViewControllerTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Gutenberg/PostGBKEditorViewControllerTests.swift @@ -6,14 +6,36 @@ import UIKit @testable import WordPress @testable import WordPressData +/// Keeps every editor these tests build alive for the lifetime of the process. +/// +/// `viewDidLoad` starts the editor's load on a `Task` that reaches +/// `startUploadServer()` after the test method returns, where GutenbergKit +/// asserts an assigned `mediaUploadDelegate` is still alive. Tripping that +/// precondition crashes the test host, killing every concurrently running suite +/// — so the damage is not contained to this file. +/// +/// Only a test can reach that state. `PostGBKEditorViewController` holds the +/// editor and its `GBKMediaUploadProcessor` as strong `let`s on one object, so +/// in the app they always die together and the load's `[weak self]` is already +/// nil. Here a window is the controller's only owner, so releasing it mid-load +/// leaves the editor reachable with a dead delegate. +/// +/// Retention must outlive the *suite instance*: Swift Testing builds a fresh +/// one per test, so an instance property dies at the very deadline being +/// missed. The race is timing-dependent, so a green run does not prove the +/// hazard is gone. +@MainActor +private enum RetainedEditors { + static var windows: [UIWindow] = [] +} + @MainActor @Suite(.serialized) struct PostGBKEditorViewControllerTests { - @Test("presents the site media library for GutenbergKit requests") - func presentsSiteMediaLibrary() throws { - let context = ContextManager.forTesting().mainContext - let blog = BlogBuilder(context).build() + /// Builds an editor, retaining it for the process's lifetime so the load it + /// starts cannot outlive its delegate. See ``RetainedEditors``. + private func makeEditor(blog: Blog) -> PostGBKEditorViewController { let viewController = PostGBKEditorViewController( postId: nil, postType: .post, @@ -26,6 +48,15 @@ struct PostGBKEditorViewControllerTests { window.rootViewController = viewController window.makeKeyAndVisible() viewController.loadViewIfNeeded() + RetainedEditors.windows.append(window) + return viewController + } + + @Test("presents the site media library for GutenbergKit requests") + func presentsSiteMediaLibrary() throws { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let viewController = makeEditor(blog: blog) let data = Data( #"{"allowedTypes":["image"],"multiple":true,"value":[],"contextId":"test"}"#.utf8 @@ -108,18 +139,7 @@ struct PostGBKEditorViewControllerTests { for blog: Blog, requesting mediaIds: [Int] ) throws -> SiteMediaPickerViewController { - let viewController = PostGBKEditorViewController( - postId: nil, - postType: .post, - title: "", - content: "", - status: "draft", - blog: blog - ) - let window = UIWindow() - window.rootViewController = viewController - window.makeKeyAndVisible() - viewController.loadViewIfNeeded() + let viewController = makeEditor(blog: blog) let value = mediaIds.map(String.init).joined(separator: ",") let data = Data( diff --git a/Tests/KeystoneTests/Tests/Features/Media/GBKMediaUploadProcessorTests.swift b/Tests/KeystoneTests/Tests/Features/Media/GBKMediaUploadProcessorTests.swift new file mode 100644 index 000000000000..ec2417d4f651 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Features/Media/GBKMediaUploadProcessorTests.swift @@ -0,0 +1,570 @@ +import Foundation +import ImageIO +import Testing +import UniformTypeIdentifiers +import WordPressShared + +@testable import WordPress + +struct GBKMediaUploadProcessorTests { + + // MARK: - Images + + @Test func imageIsResizedWhenOptimizationEnabled() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("test-image-device-photo-gps.jpg") + + let result = try await processor.processFile(at: url, mimeType: "image/jpeg", filename: url.lastPathComponent) + + guard case .processed(let outputURL, let mimeType, let filename) = result else { + Issue.record("Expected a processed file") + return + } + defer { cleanUp(outputURL) } + let size = try imageSize(at: outputURL) + #expect(max(size.width, size.height) == 200) + #expect(mimeType == "image/jpeg") + #expect(filename.hasPrefix("test-image-device-photo-gps")) + } + + @Test func imageIsUntouchedWhenProcessingWouldBeNoOp() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = false + settings.removeLocationSetting = false + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("test-image-device-photo-gps.jpg") + + let result = try await processor.processFile(at: url, mimeType: "image/jpeg", filename: url.lastPathComponent) + + guard case .original = result else { + Issue.record("Expected the original file to pass through") + return + } + } + + @Test func gpsDataIsStrippedWhenRemoveLocationEnabled() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = false + settings.removeLocationSetting = true + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("test-image-device-photo-gps.jpg") + + let result = try await processor.processFile(at: url, mimeType: "image/jpeg", filename: url.lastPathComponent) + + guard case .processed(let outputURL, _, _) = result else { + Issue.record("Expected a processed file") + return + } + defer { cleanUp(outputURL) } + #expect(try imageProperties(at: url)[kCGImagePropertyGPSDictionary] != nil) + #expect(try imageProperties(at: outputURL)[kCGImagePropertyGPSDictionary] == nil) + } + + @Test func heicIsConvertedToJPEG() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = false + settings.removeLocationSetting = false + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("iphone-photo.heic") + + let result = try await processor.processFile(at: url, mimeType: "image/heic", filename: url.lastPathComponent) + + guard case .processed(let outputURL, let mimeType, let filename) = result else { + Issue.record("Expected a processed file") + return + } + defer { cleanUp(outputURL) } + #expect(mimeType == "image/jpeg") + #expect(filename.hasSuffix(".jpg") || filename.hasSuffix(".jpeg")) + } + + /// Destination names come from a check-then-act `fileExists` loop with no + /// locking, and GutenbergKit processes uploads concurrently, so concurrent + /// exports sharing one directory must still resolve to distinct files. + /// + /// They do because GutenbergKit writes each upload to `-` + /// and the exporters name their output after that, so every export name is + /// already unique before the loop is consulted. The sources here carry that + /// prefix, as they do in production. + @Test func concurrentExportsDoNotCollide() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + + let sources = try (0..<8) + .map { _ in + try copyFixture("test-image-device-photo-gps.jpg", as: "\(UUID().uuidString)-photo.jpg") + } + defer { sources.forEach(cleanUp) } + + let outputURLs = try await withThrowingTaskGroup(of: URL.self) { group in + for source in sources { + group.addTask { + let result = try await processor.processFile( + at: source, + mimeType: "image/jpeg", + filename: "photo.jpg" + ) + guard case .processed(let outputURL, _, _) = result else { + throw ProcessingError.expectedProcessedFile + } + return outputURL + } + } + return try await group.reduce(into: [URL]()) { $0.append($1) } + } + defer { outputURLs.forEach(cleanUp) } + + // Every export is its own file, and every one of them survived the + // others finishing rather than being overwritten or swept away. + #expect(Set(outputURLs).count == outputURLs.count) + for outputURL in outputURLs { + #expect(FileManager.default.fileExists(atPath: outputURL.path)) + #expect(max(try imageSize(at: outputURL).width, try imageSize(at: outputURL).height) == 200) + } + } + + // MARK: - GIFs and other files + + @Test func gifPassesThroughUntouched() async throws { + let processor = makeProcessor(settings: makeSettings()) + let url = try fixtureURL("test-gif.gif") + + let result = try await processor.processFile(at: url, mimeType: "image/gif", filename: url.lastPathComponent) + + guard case .original = result else { + Issue.record("Expected the original file to pass through") + return + } + } + + /// SVG conforms to `UTType.image`, so it reaches the image branch, but + /// ImageIO cannot decode or encode it. It must pass through untouched + /// rather than fail in the exporter. + @Test func svgPassesThroughUntouched() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.removeLocationSetting = true + let processor = makeProcessor(settings: settings) + let url = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).svg") + try #""# + .write(to: url, atomically: true, encoding: .utf8) + defer { cleanUp(url) } + + let result = try await processor.processFile( + at: url, + mimeType: "image/svg+xml", + filename: url.lastPathComponent + ) + + guard case .original = result else { + Issue.record("Expected the original file to pass through") + return + } + } + + /// The editor validates uploads against the site's real `allowedMimeTypes` + /// before they reach the delegate, so the processor does not second-guess it + /// with `Blog.allowedFileTypes` — a cached option that can lag the server and + /// could only reject a file the server would have accepted. + @Test func documentPassesThroughWhateverTheSiteAllows() async throws { + let processor = makeProcessor(settings: makeSettings()) + let url = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).txt") + try "plain text".write(to: url, atomically: true, encoding: .utf8) + defer { cleanUp(url) } + + let result = try await processor.processFile( + at: url, + mimeType: "text/plain", + filename: url.lastPathComponent + ) + + guard case .original = result else { + Issue.record("Expected the original file to pass through") + return + } + } + + // MARK: - Files without an extension + + /// GutenbergKit names the temp file after the multipart `filename`, which + /// the editor does not guarantee carries an extension (its native inserter + /// derives one from a URL path segment). Such a file resolves to + /// `public.data`, so the reported MIME type has to stand in for the type. + @Test func extensionlessImageIsProcessedUsingReportedMIMEType() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + let url = try copyFixtureDroppingExtension("test-image-device-photo-gps.jpg") + defer { cleanUp(url) } + + let result = try await processor.processFile( + at: url, + mimeType: "image/jpeg", + filename: url.lastPathComponent + ) + + guard case .processed(let outputURL, let mimeType, _) = result else { + Issue.record("Expected a processed file") + return + } + defer { cleanUp(outputURL) } + #expect(mimeType == "image/jpeg") + #expect(max(try imageSize(at: outputURL).width, try imageSize(at: outputURL).height) == 200) + } + + /// The fallback only applies when the URL yields no type of its own — a + /// mismatched MIME type must not override what the file actually is. + @Test func fileExtensionWinsOverReportedMIMEType() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = false + settings.removeLocationSetting = false + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("test-gif.gif") + + let result = try await processor.processFile(at: url, mimeType: "image/jpeg", filename: url.lastPathComponent) + + // Classified as a GIF from the extension, not as a JPEG from the + // reported type, so it passes through instead of being re-encoded. + guard case .original = result else { + Issue.record("Expected the original file to pass through") + return + } + } + + /// An extension no UTI declares resolves to a *dynamic* type rather than to + /// `public.data`, so it clears a `!= .data` check while still conforming to + /// nothing. `.jfif` is a plain JPEG WordPress accepts, and the reported + /// `image/jpeg` says so, so it has to be processed like any other JPEG + /// instead of failing the upload outright. + @Test func undeclaredExtensionIsProcessedUsingReportedMIMEType() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + let url = try copyFixture("test-image-device-photo-gps.jpg", as: "photo.jfif") + defer { cleanUp(url) } + // Guards the premise: if `jfif` ever gains a real declaration this test + // stops exercising the dynamic-type path. + #expect(try #require(url.typeIdentifier.flatMap(UTType.init)).isDynamic) + + let result = try await processor.processFile( + at: url, + mimeType: "image/jpeg", + filename: "photo.jfif" + ) + + guard case .processed(let outputURL, let mimeType, _) = result else { + throw ProcessingError.expectedProcessedFile + } + defer { cleanUp(outputURL) } + #expect(mimeType == "image/jpeg") + #expect(max(try imageSize(at: outputURL).width, try imageSize(at: outputURL).height) == 200) + } + + /// `handlesFile` resolves the type from the extension, which is dynamic for + /// `.jfif` too. It has to discard it the same way, or the file is claimed + /// (or declined) on a classification `processFile` does not share. + @Test func undeclaredExtensionIsClaimedFromItsReportedMIMEType() { + let processor = makeProcessor(settings: makeSettings()) + #expect(processor.handlesFile(ofType: "image/jpeg", named: "photo.jfif")) + // Nothing decidable from either signal still means claim, so + // `processFile` gets to read the bytes. + #expect(processor.handlesFile(ofType: "text/plain", named: "photo.jfif")) + } + + @Test func extensionlessFileWithUnusableMIMETypeThrows() async throws { + let processor = makeProcessor(settings: makeSettings()) + let url = try copyFixtureDroppingExtension("test-image-device-photo-gps.jpg") + defer { cleanUp(url) } + + await #expect(throws: MediaURLExporter.URLExportError.self) { + try await processor.processFile(at: url, mimeType: "not-a-mime-type", filename: url.lastPathComponent) + } + } + + // MARK: - Output naming + + /// GutenbergKit names the temp file it hands over `-`, and + /// the returned name becomes the attachment's slug and title. The name the + /// editor sent must survive processing, or the UUID ends up in both. + @Test func processedFileKeepsTheNameTheEditorSent() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + let url = try copyFixture("test-image-device-photo-gps.jpg", as: "\(UUID().uuidString)-img_1234.jpg") + defer { cleanUp(url) } + + let result = try await processor.processFile(at: url, mimeType: "image/jpeg", filename: "IMG_1234.jpg") + + guard case .processed(let outputURL, _, let filename) = result else { + throw ProcessingError.expectedProcessedFile + } + defer { cleanUp(outputURL) } + // The basename is the editor's, with no trace of the UUID the temp file + // carried. The extension is the export's — `.jpg` normalizes to the + // type's preferred `.jpeg` even though the format did not change. + #expect(filename == "IMG_1234.jpeg") + } + + /// A conversion changes the bytes, so the extension has to follow them even + /// though the basename does not. + @Test func convertedFileKeepsItsNameButTakesTheExportExtension() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + let processor = makeProcessor(settings: settings) + let url = try fixtureURL("iphone-photo.heic") + + let result = try await processor.processFile(at: url, mimeType: "image/heic", filename: "IMG_1234.HEIC") + + guard case .processed(let outputURL, let mimeType, let filename) = result else { + throw ProcessingError.expectedProcessedFile + } + defer { cleanUp(outputURL) } + #expect(mimeType == "image/jpeg") + #expect(filename == "IMG_1234.jpeg") + } + + // MARK: - handlesFile + + /// The invariant the metadata gate rests on: declining a file must mean + /// `processFile` would have returned it unchanged. If this fails, the gate + /// is skipping work that `processFile` would actually have done. + @Test(arguments: [true, false], [true, false]) + func decliningAFileImpliesProcessFileWouldNotTouchIt( + optimizationEnabled: Bool, + removeLocation: Bool + ) async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = optimizationEnabled + settings.removeLocationSetting = removeLocation + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + + let fixtures: [(filename: String, mimeType: String)] = [ + ("test-image-device-photo-gps.jpg", "image/jpeg"), + ("iphone-photo.heic", "image/heic"), + ("test-gif.gif", "image/gif"), + ("test-video-device-gps.m4v", "video/mp4") + ] + + for fixture in fixtures { + guard !processor.handlesFile(ofType: fixture.mimeType, named: fixture.filename) else { + continue + } + let url = try fixtureURL(fixture.filename) + let result = try await processor.processFile( + at: url, + mimeType: fixture.mimeType, + filename: fixture.filename + ) + guard case .original = result else { + Issue.record("Declined \(fixture.filename) but processFile would have processed it") + return + } + } + } + + @Test func gifIsDeclinedBeforeBeingCopiedToDisk() { + let processor = makeProcessor(settings: makeSettings()) + #expect(!processor.handlesFile(ofType: "image/gif", named: "animation.gif")) + } + + /// SVG conforms to `UTType.image`, but ImageIO cannot decode it, so + /// `processFile` returns it unchanged for any settings. Claiming it would + /// buy a full temp-file copy that is handed straight back. + @Test func svgIsDeclinedBeforeBeingCopiedToDisk() { + let processor = makeProcessor(settings: makeSettings()) + #expect(!processor.handlesFile(ofType: "image/svg+xml", named: "logo.svg")) + #expect(!processor.handlesFile(ofType: "text/plain", named: "logo.svg")) + } + + @Test func imagesAndVideosAreAlwaysClaimed() { + let processor = makeProcessor(settings: makeSettings()) + #expect(processor.handlesFile(ofType: "image/jpeg", named: "photo.jpg")) + #expect(processor.handlesFile(ofType: "image/heic", named: "photo.heic")) + #expect(processor.handlesFile(ofType: "video/mp4", named: "clip.mp4")) + } + + /// `processFile` returns every document unchanged, so claiming one would + /// only spend a full temp-file copy to hand it straight back. + @Test func documentsAreDeclinedBeforeBeingCopiedToDisk() { + let processor = makeProcessor(settings: makeSettings()) + #expect(!processor.handlesFile(ofType: "application/pdf", named: "doc.pdf")) + #expect(!processor.handlesFile(ofType: "text/csv", named: "data.csv")) + } + + /// A part with no `Content-Type` arrives as `text/plain`, and a real + /// `Content-Type` may carry parameters or arbitrary casing. None of those + /// may cause a photo to be mistaken for a document and declined. + @Test( + arguments: [ + "image/jpeg; charset=binary", + "IMAGE/JPEG", + "text/plain", + "application/octet-stream", + "" + ] + ) + func imagesAreClaimedWhateverTheReportedMIMEType(mimeType: String) { + let processor = makeProcessor(settings: makeSettings()) + #expect(processor.handlesFile(ofType: mimeType, named: "photo.jpg")) + } + + /// Nothing decidable from the metadata means the file is claimed, so + /// `processFile` can read the type off the bytes instead. + @Test func unrecognizableMetadataIsClaimed() { + let processor = makeProcessor(settings: makeSettings()) + #expect(processor.handlesFile(ofType: "text/plain", named: "upload")) + } + + /// `handlesFile` must resolve the type the way `processFile` does — the + /// extension first — or a mislabeled `Content-Type` silently declines a + /// photo that would have been downscaled and stripped. Declining is + /// unrecoverable, so the two cannot disagree. + @Test func mislabeledContentTypeDoesNotDeclineAnImage() async throws { + let settings = makeSettings() + settings.imageOptimizationEnabled = true + settings.removeLocationSetting = true + settings.maxImageSizeSetting = 200 + let processor = makeProcessor(settings: settings) + + #expect(processor.handlesFile(ofType: "application/pdf", named: "photo.jpg")) + + // And the claim is warranted: `processFile` really does process it. + let url = try fixtureURL("test-image-device-photo-gps.jpg") + let result = try await processor.processFile(at: url, mimeType: "application/pdf", filename: "photo.jpg") + guard case .processed(let outputURL, _, _) = result else { + throw ProcessingError.expectedProcessedFile + } + cleanUp(outputURL) + } + + // MARK: - Videos + + /// WebM and WMV conform to `UTType.movie`, so they classify as video, but + /// AVFoundation cannot open either — `MediaVideoExporter` fails them on + /// `AVURLAsset.isExportable`. Exporting them anyway would turn uploads + /// WordPress accepts into errors, so they pass through instead. + @Test(arguments: [("clip.webm", "video/webm"), ("clip.wmv", "video/x-ms-wmv")]) + func videoAVFoundationCannotReadPassesThroughUntouched( + filename: String, + mimeType: String + ) async throws { + let processor = makeProcessor(settings: makeSettings()) + // Contents are never read: the type alone decides, before any export. + let url = try copyFixture("test-video-device-gps.m4v", as: filename) + defer { cleanUp(url) } + + let result = try await processor.processFile(at: url, mimeType: mimeType, filename: filename) + + guard case .original = result else { + Issue.record("Expected \(filename) to pass through unprocessed") + return + } + // And it is declined up front, so the temp copy is skipped entirely. + #expect(!processor.handlesFile(ofType: mimeType, named: filename)) + } + + /// The formats AVFoundation does read must still be claimed and exported, + /// or the preset and duration limit stop being applied to real video. + @Test(arguments: ["clip.mp4", "clip.mov", "clip.m4v", "clip.avi"]) + func readableVideoIsStillClaimed(filename: String) { + let processor = makeProcessor(settings: makeSettings()) + #expect(processor.handlesFile(ofType: "video/mp4", named: filename)) + } + + @Test func videoExceedingDurationLimitThrows() async throws { + let processor = GBKMediaUploadProcessor( + videoDurationLimit: 1, + makeMediaSettings: makeSettingsFactory(makeSettings()) + ) + let url = try fixtureURL("test-video-device-gps.m4v") + + await #expect(throws: (any Error).self) { + try await processor.processFile(at: url, mimeType: "video/mp4", filename: url.lastPathComponent) + } + } + + // MARK: - Helpers + + private func makeProcessor(settings: MediaSettings) -> GBKMediaUploadProcessor { + GBKMediaUploadProcessor( + videoDurationLimit: nil, + makeMediaSettings: makeSettingsFactory(settings) + ) + } + + private func makeSettings() -> MediaSettings { + MediaSettings(database: EphemeralKeyValueDatabase()) + } + + private func makeSettingsFactory(_ settings: MediaSettings) -> @Sendable () -> MediaSettings { + nonisolated(unsafe) let settings = settings + return { settings } + } + + private func fixtureURL(_ filename: String) throws -> URL { + let bundle = Bundle(for: BundleAnchor.self) + let name = (filename as NSString).deletingPathExtension + let ext = (filename as NSString).pathExtension + let url = try #require(bundle.url(forResource: name, withExtension: ext)) + return url + } + + /// Copies a fixture to a temporary file under a different name, mirroring + /// the `-` temp file GutenbergKit hands to `processFile`. + private func copyFixture(_ filename: String, as destinationName: String) throws -> URL { + let source = try fixtureURL(filename) + let destination = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + .appendingPathComponent(destinationName, isDirectory: false) + try FileManager.default.createDirectory( + at: destination.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try FileManager.default.copyItem(at: source, to: destination) + return destination + } + + /// Copies a fixture to a temporary file with no path extension, mirroring + /// an upload whose multipart `filename` carried none. + private func copyFixtureDroppingExtension(_ filename: String) throws -> URL { + let source = try fixtureURL(filename) + let destination = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: false) + try FileManager.default.copyItem(at: source, to: destination) + #expect(destination.pathExtension.isEmpty) + return destination + } + + private func imageProperties(at url: URL) throws -> [CFString: Any] { + let source = try #require(CGImageSourceCreateWithURL(url as CFURL, nil)) + let properties = try #require(CGImageSourceCopyPropertiesAtIndex(source, 0, nil) as? [CFString: Any]) + return properties + } + + private func imageSize(at url: URL) throws -> CGSize { + let properties = try imageProperties(at: url) + let width = try #require(properties[kCGImagePropertyPixelWidth] as? CGFloat) + let height = try #require(properties[kCGImagePropertyPixelHeight] as? CGFloat) + return CGSize(width: width, height: height) + } + + private func cleanUp(_ url: URL) { + try? FileManager.default.removeItem(at: url) + } + + private enum ProcessingError: Error { + case expectedProcessedFile + } +} + +/// Anchor for resolving the test bundle from Swift Testing suites. +private final class BundleAnchor {} diff --git a/WordPress/Classes/ViewRelated/NewGutenberg/GBKMediaUploadProcessor.swift b/WordPress/Classes/ViewRelated/NewGutenberg/GBKMediaUploadProcessor.swift new file mode 100644 index 000000000000..be7bde2d0e21 --- /dev/null +++ b/WordPress/Classes/ViewRelated/NewGutenberg/GBKMediaUploadProcessor.swift @@ -0,0 +1,430 @@ +import AVFoundation +import Foundation +import GutenbergKit +import UniformTypeIdentifiers +import WordPressData + +/// Processes media files picked in the GutenbergKit editor before upload, +/// applying the app's Media settings (image optimization, max upload size, +/// image quality, video resolution, and location stripping). +/// +/// Assigned to `GutenbergKit.EditorViewController.mediaUploadDelegate`, which +/// holds it weakly and invokes it off the main actor, so the type is `Sendable` +/// and snapshots the `Blog`-derived value it needs at initialization. +final class GBKMediaUploadProcessor: MediaUploadDelegate, Sendable { + private let videoDurationLimit: TimeInterval? + private let makeMediaSettings: @Sendable () -> MediaSettings + + /// The temporary directory exports are written to. + /// + /// One directory, reused: GutenbergKit deletes the file it was handed on + /// both the success and the failure path, so nothing here needs cleanup. A + /// fresh directory per export would, since nothing sweeps those. + /// + /// Sharing it is safe despite `incrementalFilename()`'s unlocked + /// check-then-act loop: GutenbergKit writes each upload to + /// `-` and the exporters name their output after it, so + /// concurrent exports cannot resolve to the same name. + private let makeExportDirectory: @Sendable () -> MediaDirectory + + private static let exportDirectoryID = UUID(uuidString: "1D8A4E5C-1F3B-4E7A-9C2D-6B0F8A5E3C71")! + + /// Raster image types the WordPress REST API reliably accepts. Other image + /// formats (e.g. HEIC) are converted to JPEG during processing, mirroring + /// `ItemProviderMediaExporter`. + /// + /// Only consulted for an `.image` export, so it lists just the types that + /// reach that branch. GIF and SVG are web-safe too but are absent: both + /// return `.original` before any of this is read — GIF from its own + /// `expectedExport` case, SVG because ImageIO can neither decode nor encode + /// it. + private static let webSafeImageTypes: Set = [.png, .jpeg] + + @MainActor + convenience init(blog: Blog) { + self.init(videoDurationLimit: blog.videoDurationLimit) + } + + init( + videoDurationLimit: TimeInterval?, + makeMediaSettings: @escaping @Sendable () -> MediaSettings = { MediaSettings() }, + makeExportDirectory: @escaping @Sendable () -> MediaDirectory = { + .temporary(id: GBKMediaUploadProcessor.exportDirectoryID) + } + ) { + self.videoDurationLimit = videoDurationLimit + self.makeMediaSettings = makeMediaSettings + self.makeExportDirectory = makeExportDirectory + } + + // MARK: - MediaUploadDelegate + + /// Whether the file is worth materializing for `processFile`. + /// + /// GutenbergKit calls this from the multipart headers alone, before + /// streaming the upload to a temp file. Returning `false` skips that copy + /// and forwards the original request body to WordPress unchanged, so it is + /// only correct where `processFile` would return `.original` for *any* + /// Media settings — the metadata here cannot answer anything finer. + /// + /// This is a fast path, never a second place the policy lives: every `false` + /// below mirrors a branch of `processFile` that ignores `settings`. + /// Declining is also unrecoverable — the file is never seen again — so + /// anything undecidable from metadata claims the file and decides for real + /// once the bytes are on disk. + func handlesFile(ofType mimeType: String, named filename: String) -> Bool { + // The file doesn't exist yet, so stand in for it with the filename + // extension and resolve in the same order `sourceType(of:)` does: the + // file's own type first, the reported one only as a fallback. Reversing + // the two here would let a mislabeled `Content-Type` decline a file + // `processFile` would have classified — and processed — from its + // extension. Both signals are untrustworthy in ways `processFile` can + // recover from and this cannot, hence the bias toward `true`. + guard let type = Self.type(ofExtensionIn: filename) ?? Self.type(ofMIMEType: mimeType) else { + return true + } + guard let expected = try? Self.expectedExport(of: nil, type: type) else { + return true + } + switch expected { + case .gif, .other: + // Always returned unchanged, whatever the settings: only images and + // videos are processed. + return false + case .image: + // SVG conforms to `UTType.image` but is returned unchanged for any + // settings, because ImageIO cannot decode it (see `processFile`). + // Declining it skips a temp-file copy that could never be used. + // + // Every other image may be downscaled, stripped, or converted + // depending on settings and on the file's contents, so decide in + // `processFile`. + return type != .svg + case .video: + // Exported to apply the preset and duration limit — except for the + // containers AVFoundation cannot read, which `processFile` returns + // unchanged for any settings (see there). + return Self.isExportableVideoType(type) + } + } + + func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { + let sourceType = Self.sourceType(of: url, reportedMIMEType: mimeType) + let expected = try Self.expectedExport(of: url, type: sourceType) + let settings = makeMediaSettings() + + switch expected { + case .gif: + // GIFs are uploaded unchanged; processing would only copy the file. + return .original + case .other: + // Non-media files are uploaded unchanged. + // + // Deliberately narrower than `MediaURLExporter.exportURL`, which + // also rejects extensions outside the site's allowed list. That + // check belongs to the legacy picker, which hands over arbitrary + // files with nothing having vetted them. + // + // Nothing vets the file here either — the editor validates uploads + // against `allowedMimeTypes`, but that value reaches it only from + // `/wp-block-editor/v1/settings`, a route the Gutenberg plugin + // provides and WordPress core does not. Without it the setting stays + // nil and the check passes everything through. + // + // Re-checking here would not recover it. `Blog.allowedFileTypes` is + // a cached option that can lag the server, so rejecting from it + // could only ever refuse a file the server would have accepted. The + // server is the authority, and it rejects with a message the native + // upload relay passes back to the editor verbatim. + return .original + case .image: + // SVG conforms to `UTType.image`, so it lands here, but ImageIO + // cannot decode or encode it: the export would fail rather than + // produce a file. Upload it unchanged, like a GIF. + if sourceType == .svg { + return .original + } + + // Skip the export when nothing would change the file: no + // downscaling, no location stripping, and no format conversion. + // + // This is narrower than "processing changes nothing". With + // optimization off, `imageQualityForUpload` is still `.high`, so a + // web-safe image that reaches the exporter is re-encoded at that + // quality even though `imageSizeForUpload` leaves its dimensions + // alone. That mirrors `MediaImportService`, which maps the same + // settings the same way. + // + // Skipping the export also skips the exporter's unconditional EXIF + // orientation normalization, so a sideways-shot photo uploads with + // its orientation flag intact rather than rotated into its pixels. + // That is deliberate: the normalization predates WordPress 5.3, + // whose `wp_create_image_subsizes` rotates on the server for every + // site, self-hosted included. Re-encoding here to bake in a + // rotation the server performs anyway would cost a lossy pass on a + // photo the user asked not to optimize. + // + // See WordPress-iOS#12703 and core changeset 46202. + if !settings.imageOptimizationEnabled, + !settings.removeLocationSetting, + let sourceType, + Self.webSafeImageTypes.contains(sourceType) + { + return .original + } + case .video: + // AVFoundation cannot open every container that conforms to + // `.movie`, and `MediaVideoExporter` rejects the ones it cannot + // read rather than passing them along. Upload those unchanged: the + // preset, duration limit, and location stripping are all + // unavailable for them, and failing the upload to say so would + // reject files WordPress accepts (`wmv` is even in the app's own + // `MediaImportService.defaultAllowableFileExtensions`). + if let sourceType, !Self.isExportableVideoType(sourceType) { + return .original + } + } + + let exportImageType = Self.exportImageType(for: expected, sourceType: sourceType) + let export = try await makeExporter( + for: url, + expected: expected, + settings: settings, + exportImageType: exportImageType, + directory: makeExportDirectory() + ) + .export() + + let mimeType = try Self.mimeType(of: export.url, exportImageType: exportImageType) + return .processed( + export.url, + mimeType: mimeType, + filename: Self.uploadFilename(original: filename, exportURL: export.url) + ) + } + + // MARK: - Output naming + + /// The name the processed file is uploaded under, which WordPress turns + /// into the attachment's slug and title. + /// + /// The editor's name, with the extension from the export because a + /// conversion changes it. The export's own name carries the UUID prefix + /// GutenbergKit gave the temp file, so it can't be used. + private static func uploadFilename(original: String, exportURL: URL) -> String { + let name = (original as NSString).lastPathComponent + let base = (name as NSString).deletingPathExtension + guard !base.isEmpty else { + return exportURL.lastPathComponent + } + let exportExtension = exportURL.pathExtension + guard !exportExtension.isEmpty else { + return base + } + return "\(base).\(exportExtension)" + } + + // MARK: - Exporter configuration + + /// Builds an exporter configured from the app's Media settings, mirroring + /// the option mapping in `MediaImportService`. + /// + /// Returns the concrete exporter for the branch rather than + /// `MediaURLExporter`, which re-derives the type from the path extension in + /// `exportURL` and so would reject a file classified via its reported MIME + /// type. `MediaImageExporter` reads the type from the file's contents with + /// `CGImageSourceGetType`, so it handles an extensionless image correctly. + private func makeExporter( + for url: URL, + expected: MediaURLExporter.URLExportExpectation, + settings: MediaSettings, + exportImageType: UTType?, + directory: MediaDirectory + ) -> any MediaExporter { + switch expected { + case .video: + let exporter = MediaVideoExporter(url: url) + exporter.mediaDirectoryType = directory + var options = MediaVideoExporter.Options() + options.stripsGeoLocationIfNeeded = settings.removeLocationSetting + options.exportPreset = settings.maxVideoSizeSetting.videoPreset + options.durationLimit = videoDurationLimit + exporter.options = options + return exporter + case .image, .gif, .other: + // Only images reach the exporter: `.gif` and `.other` return + // `.original` before this point. + let exporter = MediaImageExporter(url: url) + exporter.mediaDirectoryType = directory + var options = MediaImageExporter.Options() + options.maximumImageSize = maximumImageSize(from: settings) + options.stripsGeoLocationIfNeeded = settings.removeLocationSetting + options.imageCompressionQuality = settings.imageQualityForUpload.doubleValue + // `exportImageType` is the destination type `MediaImageExporter` + // writes, and it also determines the output file extension. Left + // nil, the source type is kept. + options.exportImageType = exportImageType?.identifier + exporter.options = options + return exporter + } + } + + private func maximumImageSize(from settings: MediaSettings) -> CGFloat? { + let maxUploadSize = settings.imageSizeForUpload + return maxUploadSize < Int.max ? CGFloat(maxUploadSize) : nil + } + + // MARK: - Type resolution + + /// The type of the file to process. + /// + /// Resolved from the file itself, falling back to the type the editor + /// reported. The URL resolves its type from the path extension alone, and + /// two kinds of upload defeat that: + /// + /// - No extension at all. GutenbergKit names the temp file after the + /// multipart `filename`, which the editor does not guarantee carries an + /// extension (its native inserter derives one from a URL path segment). + /// Such a file resolves to the generic `public.data`. + /// - An extension no UTI declares. `photo.jfif` is a plain JPEG WordPress + /// accepts, but nothing claims `jfif`, so it resolves to a *dynamic* type + /// synthesized from the extension (`dyn.ah62d4rv4ge80y3xmq2`). + /// + /// Neither conforms to any media type, so `expectedExport` would throw and + /// fail an upload that the reported `Content-Type` describes perfectly. + /// Both therefore defer to it — for `photo.jfif`, `image/jpeg`. + private static func sourceType(of url: URL, reportedMIMEType: String) -> UTType? { + guard let type = url.typeIdentifier.flatMap(UTType.init), !isUninformative(type) else { + return type(ofMIMEType: reportedMIMEType) + } + return type + } + + /// Whether a type says nothing about the file's format and should give way + /// to the reported MIME type. See `sourceType(of:reportedMIMEType:)`. + private static func isUninformative(_ type: UTType) -> Bool { + type == .data || type.isDynamic + } + + /// Whether AVFoundation can read the container, and so whether + /// `MediaVideoExporter` can export it at all. + /// + /// `expectedExport` routes anything conforming to `.video` or `.movie` to + /// the video branch, which is broader than what AVFoundation opens: WebM + /// and WMV both conform to `.movie` but are absent from + /// `audiovisualTypes()`, and `MediaVideoExporter` fails them on + /// `AVURLAsset.isExportable`. + /// + /// Matched by conformance rather than identity, because a file can be a + /// subtype of a listed format without appearing in the list itself: + /// DRM-wrapped MPEG-4 conforms to `public.mpeg-4` but is absent. + private static func isExportableVideoType(_ type: UTType) -> Bool { + exportableVideoTypes.contains { type.conforms(to: $0) } + } + + private static let exportableVideoTypes: Set = Set( + AVURLAsset.audiovisualTypes().compactMap { UTType($0.rawValue) } + ) + + /// The type a reported MIME type names, or `nil` when it names nothing + /// usable. + /// + /// `UTType(mimeType:)` matches the bare `type/subtype` only, so the header + /// is normalized first. Two shapes reach us that it would otherwise miss: + /// + /// - Parameters and casing: `Content-Type` may carry parameters + /// (`image/jpeg; charset=binary`) and its casing is not significant + /// (RFC 9110 §8.3). Left as-is, both resolve to a dynamic UTType that + /// conforms to nothing. + /// - Placeholders: GutenbergKit's multipart parser defaults a part with no + /// `Content-Type` to `text/plain` (RFC 7578 §4.4), and it picks the file + /// part by the presence of a `filename` parameter rather than by content + /// type — so a real image can arrive labeled `text/plain`. Treating that + /// as authoritative would classify a photo as a document. + private static func type(ofMIMEType mimeType: String) -> UTType? { + let normalized = mimeType.prefix(while: { $0 != ";" }) + .trimmingCharacters(in: .whitespaces) + .lowercased() + guard !normalized.isEmpty, !placeholderMIMETypes.contains(normalized) else { + return nil + } + return UTType(mimeType: normalized) + } + + /// MIME types that carry no information about the file. `octet-stream` is + /// the generic "unknown bytes" type; `text/plain` is what GutenbergKit's + /// multipart parser substitutes for a part that sent no `Content-Type`. + private static let placeholderMIMETypes: Set = [ + "application/octet-stream", "text/plain" + ] + + /// The type a filename's extension names, for use before the file exists. + /// `processFile` reads the type off the file itself instead. + /// + /// Discards a dynamic type for the same reason `sourceType` does, and to + /// stay in step with it: an extension no UTI declares must fall through to + /// the reported MIME type in both places, or `handlesFile` would classify a + /// `.jfif` from a type that conforms to nothing while `processFile` + /// classifies the same file as the JPEG it is. + private static func type(ofExtensionIn filename: String) -> UTType? { + let fileExtension = (filename as NSString).pathExtension.lowercased() + guard !fileExtension.isEmpty else { + return nil + } + return UTType(filenameExtension: fileExtension).flatMap { isUninformative($0) ? nil : $0 } + } + + /// Classifies a file the way `MediaURLExporter.expectedExport(with:)` does, + /// but from an already-resolved type so the caller can supply one the URL + /// alone cannot provide. + /// - Parameter url: The file being classified, or `nil` when only the type + /// is known — `handlesFile` runs before the file exists. + private static func expectedExport( + of url: URL?, + type: UTType? + ) throws -> MediaURLExporter.URLExportExpectation { + if let url, !url.isFileURL { + throw MediaURLExporter.URLExportError.invalidFileURL + } + guard let type else { + throw MediaURLExporter.URLExportError.unknownFileUTI + } + if type == .gif { + return .gif + } else if type.conforms(to: .video) || type.conforms(to: .movie) { + return .video + } else if type.conforms(to: .image) { + return .image + } else if type.conforms(to: .content) || type.conforms(to: .zip) { + return .other + } + throw MediaURLExporter.URLExportError.unsupportedFileType + } + + /// The type `MediaImageExporter` should write, or `nil` to keep the source + /// type. Only image exports convert: everything the REST API accepts as-is + /// is left alone, and the rest becomes JPEG. + private static func exportImageType( + for expected: MediaURLExporter.URLExportExpectation, + sourceType: UTType? + ) -> UTType? { + guard case .image = expected, let sourceType else { + return nil + } + return webSafeImageTypes.contains(sourceType) ? nil : .jpeg + } + + /// The MIME type of a finished export. + /// + /// An image export writes `exportImageType` when set, so that value is + /// authoritative and needs no round-trip through the output path. Only the + /// cases that keep the source type — every video, and a web-safe image — + /// fall back to reading the file back. + private static func mimeType(of url: URL, exportImageType: UTType?) throws -> String { + let type = exportImageType ?? url.typeIdentifier.flatMap(UTType.init) + guard let mimeType = type?.preferredMIMEType else { + throw MediaURLExporter.URLExportError.unknownFileUTI + } + return mimeType + } +} diff --git a/WordPress/Classes/ViewRelated/NewGutenberg/PostGBKEditorViewController.swift b/WordPress/Classes/ViewRelated/NewGutenberg/PostGBKEditorViewController.swift index 46e498603e0d..4f227a83dcb6 100644 --- a/WordPress/Classes/ViewRelated/NewGutenberg/PostGBKEditorViewController.swift +++ b/WordPress/Classes/ViewRelated/NewGutenberg/PostGBKEditorViewController.swift @@ -16,6 +16,9 @@ class PostGBKEditorViewController: UIViewController, GutenbergKit.EditorViewCont private lazy var mediaPickerHelper = GutenbergMediaPickerHelper(context: self, blog: blog) + /// Retains the media upload processor, which the editor holds weakly. + private let mediaUploadProcessor: GBKMediaUploadProcessor + private var keyboardShowObserver: Any? private var keyboardHideObserver: Any? private var keyboardFrame = CGRect.zero @@ -55,10 +58,12 @@ class PostGBKEditorViewController: UIViewController, GutenbergKit.EditorViewCont dependencies: cachedDependencies, mediaPicker: MediaPickerController(blog: blog) ) + self.mediaUploadProcessor = GBKMediaUploadProcessor(blog: blog) super.init(nibName: nil, bundle: nil) self.editorViewController.delegate = self + self.editorViewController.mediaUploadDelegate = mediaUploadProcessor } required init?(coder aDecoder: NSCoder) {