Skip to content

fix: decode support attachment file paths before uploading - #26008

Open
jkmassel wants to merge 1 commit into
trunkfrom
jkmassel/fix-support-attachment-path-encoding
Open

fix: decode support attachment file paths before uploading#26008
jkmassel wants to merge 1 commit into
trunkfrom
jkmassel/fix-support-attachment-path-encoding

Conversation

@jkmassel

@jkmassel jkmassel commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes a bug where a support ticket or reply fails to send if an attached screenshot's filename needs percent-encoding — a space is enough.

Same defect as #26005, in the other two wordpress-rs params types that carry filesystem paths. Found by sweeping the codebase for the shape after that PR.

Summary

  • URL.path() percent-encodes by default, unlike the legacy url.path property, which returns a decoded path.
  • CreateSupportTicketParams and AddMessageToSupportConversationParams both passed the encoded form as attachments, so wordpress-rs looked for files that don't exist on disk.
  • Reachable with ordinary content: the attachment keeps the picked photo's original filename, and macOS names screenshots Screen Shot 2026-09-08 at 10.31.15.png.

Root Cause

WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift: both call sites built the attachment list with attachments.map { $0.path() }.

Those strings are filesystem paths, not URL components. In wordpress-rs they become MultipartFormFile.file_path — the same struct field MediaCreateParams.filePath feeds — and SafeRequestExecutor opens each one directly:

do {
    try form.append(.init(fileAtPath: file.filePath, ...))
} catch {
    throw RequestExecutionError.MediaFileNotFound(filePath: file.filePath)
}

The filename is not app-generated. ScreenshotPicker.swift builds the attachment URL as directory.appendingPathComponent(received.file.lastPathComponent). The directory is app-generated — URL.cachesDirectory plus a UUID — but the last component is copied verbatim from the file PhotosUI exports, which preserves the asset's original filename.

The failure takes the whole request with it. SupportForm catches and shows an error alert, so a user attaching a screenshot to report a bug can't file the report.

The encoding is wider than spaces

path() escapes non-ASCII too, which puts non-English filenames in scope:

URL(fileURLWithPath: "/tmp/Screen Shot 1.png").path()   // "/tmp/Screen%20Shot%201.png"  ❌
URL(fileURLWithPath: "/tmp/100% done.png").path()       // "/tmp/100%25%20done.png"      ❌
URL(fileURLWithPath: "/tmp/café.png").path()            // "/tmp/cafe%CC%81.png"         ❌

Fix

WordPress/Classes/ViewRelated/NewSupport/SupportDataProvider.swift: added an initializer to each params type that takes attachmentURLs: [URL] and owns the conversion, so the call sites pass URLs and can't reintroduce the encoded form. Mirrors MediaCreateParams.init?(media:) from #26005.

Test plan

  • Added SupportAttachmentFilePathTests — six cases across both params types: a filename with a space, one with a %, one non-ASCII, controls needing no encoding, and an empty attachment list.
  • Verified the three encoding tests fail without the fix, reproducing the exact encoded strings (Screen%20Shot%201.png, cafe%CC%81.png), and that the three controls pass either way.
  • WordPressTest/SupportAttachmentFilePathTests — 6/6 pass on an iOS 26.4 simulator.

Not device-verified. The one link not confirmed by reading code is whether PhotosUI's exported lastPathComponent retains the space in practice — everything upstream and downstream of that is.

Notes

Modules/Sources/WordPressMediaLibrary/Upload/UploadSourceMaterializer.swift already builds MediaCreateParams(filePath: destURL.path) with the decoded property at five sites, so the newer media library was never affected.

Related issues

Follow-up

The conversion belongs in the library, not here — Automattic/wordpress-rs#1623 adds attachmentURLs: [URL] initializers to both params types.

The app pins wordpress-rs at exact: "0.8.0" and can't consume that until it ships and the app bumps, so the extensions in this PR stand in until then. When the app next bumps the dependency, delete them and pass attachmentURLs: straight through.

Support attachments are handed to wordpress-rs as filesystem paths, which it
opens directly. URL.path() percent-encodes by default, so an attachment whose
filename needs encoding produced a path that doesn't exist on disk and failed
the whole ticket with MediaFileNotFound.

Same shape as the media upload fix in #26005, in the other two params types
that take file paths.
@jkmassel jkmassel self-assigned this Sep 8, 2026
@jkmassel jkmassel added this to the 27.3 milestone Sep 8, 2026
@wpmobilebot

Copy link
Copy Markdown
Contributor
App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number34432
VersionPR #26008
Bundle IDorg.wordpress.alpha
Commite9ce5be
Installation URL1aivg5c5nah60
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

Copy link
Copy Markdown
Contributor
App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number34432
VersionPR #26008
Bundle IDcom.jetpack.alpha
Commite9ce5be
Installation URL448gc5rudbp4g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants