diff --git a/Tests/KeystoneTests/Tests/ViewRelated/NewSupport/SupportAttachmentFilePathTests.swift b/Tests/KeystoneTests/Tests/ViewRelated/NewSupport/SupportAttachmentFilePathTests.swift new file mode 100644 index 000000000000..f767d14fdf1c --- /dev/null +++ b/Tests/KeystoneTests/Tests/ViewRelated/NewSupport/SupportAttachmentFilePathTests.swift @@ -0,0 +1,78 @@ +import Foundation +import Testing +import WordPressAPI + +@testable import WordPress + +/// Support attachments are handed to `wordpress-rs` as filesystem paths, which it opens directly. +/// `URL.path()` percent-encodes by default, so a filename needing encoding produced a path that +/// doesn't exist on disk and failed the whole ticket. Same shape as the media upload bug in #26005. +struct SupportAttachmentFilePathTests { + + /// A screenshot picked from the library keeps its original filename, and macOS names those + /// with spaces. + @Test func newTicketDecodesPercentEncodingInAttachmentPaths() { + let params = CreateSupportTicketParams( + subject: "Subject", + message: "Message", + application: "jetpack", + attachmentURLs: [URL(fileURLWithPath: "/tmp/attachments/Screen Shot 1.png")] + ) + + #expect(params.attachments == ["/tmp/attachments/Screen Shot 1.png"]) + } + + /// Characters beyond the space get encoded too — including non-ASCII, which `path()` renders + /// as UTF-8 escapes. + @Test func newTicketDecodesPercentEncodingBeyondSpaces() { + let params = CreateSupportTicketParams( + subject: "Subject", + message: "Message", + application: "jetpack", + attachmentURLs: [ + URL(fileURLWithPath: "/tmp/attachments/100% done.png"), + URL(fileURLWithPath: "/tmp/attachments/café.png") + ] + ) + + #expect(params.attachments == ["/tmp/attachments/100% done.png", "/tmp/attachments/café.png"]) + } + + /// A name needing no encoding has to survive untouched. + @Test func newTicketLeavesAnOrdinaryPathAlone() { + let params = CreateSupportTicketParams( + subject: "Subject", + message: "Message", + application: "jetpack", + attachmentURLs: [URL(fileURLWithPath: "/tmp/attachments/IMG_0001.png")] + ) + + #expect(params.attachments == ["/tmp/attachments/IMG_0001.png"]) + } + + /// The reply path builds a different params type, so it needs its own coverage. + @Test func replyDecodesPercentEncodingInAttachmentPaths() { + let params = AddMessageToSupportConversationParams( + message: "Message", + attachmentURLs: [URL(fileURLWithPath: "/tmp/attachments/Screen Shot 1.png")] + ) + + #expect(params.attachments == ["/tmp/attachments/Screen Shot 1.png"]) + } + + @Test func replyLeavesAnOrdinaryPathAlone() { + let params = AddMessageToSupportConversationParams( + message: "Message", + attachmentURLs: [URL(fileURLWithPath: "/tmp/attachments/IMG_0001.png")] + ) + + #expect(params.attachments == ["/tmp/attachments/IMG_0001.png"]) + } + + /// No attachments is the common case and must not trip the mapping. + @Test func emptyAttachmentsProduceNoPaths() { + let params = AddMessageToSupportConversationParams(message: "Message", attachmentURLs: []) + + #expect(params.attachments.isEmpty) + } +} diff --git a/WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift b/WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift index ea4c6df0161f..83c84c7cdcc0 100644 --- a/WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift +++ b/WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift @@ -305,7 +305,7 @@ actor WpSupportConversationDataProvider: SupportConversationDataProvider { subject: subject, message: message, application: "jetpack", - attachments: attachments.map { $0.path() } + attachmentURLs: attachments ) return try await self.wpcomClient.api @@ -323,7 +323,7 @@ actor WpSupportConversationDataProvider: SupportConversationDataProvider { ) async throws -> Conversation { let params = AddMessageToSupportConversationParams( message: message, - attachments: attachments.map { $0.path() } + attachmentURLs: attachments ) let conversation = try await self.wpcomClient.api @@ -336,6 +336,33 @@ actor WpSupportConversationDataProvider: SupportConversationDataProvider { } } +// Not `private`: the file-path handling below is covered by `SupportAttachmentFilePathTests`. +// +// Both params types carry `attachments` as filesystem paths, which `wordpress-rs` turns into +// `MultipartFormFile.file_path` and opens directly. `URL.path()` percent-encodes by default, so a +// filename with a space — a macOS screenshot, say — becomes a path that doesn't exist on disk and +// the whole request fails with `MediaFileNotFound`. These initializers own that conversion so the +// call sites can pass URLs. +extension CreateSupportTicketParams { + init(subject: String, message: String, application: String, attachmentURLs: [URL]) { + self.init( + subject: subject, + message: message, + application: application, + attachments: attachmentURLs.map { $0.path(percentEncoded: false) } + ) + } +} + +extension AddMessageToSupportConversationParams { + init(message: String, attachmentURLs: [URL]) { + self.init( + message: message, + attachments: attachmentURLs.map { $0.path(percentEncoded: false) } + ) + } +} + actor WpDiagnosticsDataProvider: DiagnosticsDataProvider { func fetchDiskCacheUsage() async throws -> WordPressCoreProtocols.DiskCacheUsage { try await DiskCache.shared.diskUsage()