Skip to content

fix: decode the local file path before handing it to the media upload API - #26005

Open
jkmassel wants to merge 1 commit into
trunkfrom
jkmassel/fix-media-file-path-encoding
Open

fix: decode the local file path before handing it to the media upload API#26005
jkmassel wants to merge 1 commit into
trunkfrom
jkmassel/fix-media-file-path-encoding

Conversation

@jkmassel

@jkmassel jkmassel commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes a bug where any media file whose name needs percent-encoding — a space is enough — fails to upload through the WordPress core REST API.

Summary

  • URL.path() percent-encodes by default, unlike the legacy url.path property, which returns a decoded path.
  • MediaCreateParams passed the encoded form as filePath, so wordpress-rs looked for a file that doesn't exist on disk.
  • Reachable with ordinary content: screen recordings and files imported from macOS routinely have spaces in their names.

Root Cause

WordPress/Classes/Services/MediaServiceRemoteCoreREST.swift: MediaCreateParams.init?(media:) built filePath with localURL.path().

The two APIs differ in a way the names don't advertise:

let url = URL(fileURLWithPath: "/tmp/media/screen recording 1.mp4")
url.path                          // "/tmp/media/screen recording 1.mp4"  ✅
url.path()                        // "/tmp/media/screen%20recording%201.mp4"  ❌
url.path(percentEncoded: false)   // "/tmp/media/screen recording 1.mp4"  ✅

Observed on device, uploading a screen recording:

WpApiError.MediaFileNotFound(filePath:
  ".../Documents/Media/screenrecording_08-27-2024%2012-12-08_1.mp4")

The file on disk was screenrecording_08-27-2024 12-12-08_1.mp4. Confirmed by listing the app's Documents/Media directory on the device — the name there has a literal space.

The export itself succeeds, so the failure lands late: the media object is created, media_service_upload_started fires, and the upload then fails with MediaFileNotFound.

Fix

WordPress/Classes/Services/MediaServiceRemoteCoreREST.swift: pass localURL.path(percentEncoded: false).

Test plan

  • Added MediaCreateParamsFilePathTests — a filename with a space, one with a %, and a control that needs no encoding.
  • Verified the test fails without the fix, reproducing the exact production string (screen%20recording%201.mp4), and that the control passes either way.
  • Device A/B on a self-hosted site (iPhone 15 Pro, iOS 27): the same 39-second screen recording failed with MediaFileNotFound before the change and uploaded successfully after it.

Notes

MediaCreateParams's extension is no longer private, so the test can reach it. The comment on it says why.

Four other .path() call sites share this shape and are not changed here — SupportDataProvider (x2), AsyncImageKit/ImageDownloader, and WordPressCore/DiskCache. They looked safe because the paths are app-generated, but that was not verified and is worth a separate sweep.

… API

`URL.path()` percent-encodes by default, unlike the legacy `url.path`
property. `MediaCreateParams` passed the encoded form as `filePath`, so any
media file whose name needs encoding could not be found on disk:

    WpApiError.MediaFileNotFound(filePath:
      ".../screenrecording_08-27-2024%2012-12-08_1.mp4")

while the file on disk was `screenrecording_08-27-2024 12-12-08_1.mp4`.

A space is enough to trigger it, which makes this reachable with ordinary
content: screen recordings and files imported from macOS routinely have
spaces in their names.
@jkmassel jkmassel self-assigned this Sep 8, 2026
@jkmassel jkmassel added this to the 27.3 milestone Sep 8, 2026
@dangermattic

Copy link
Copy Markdown
Collaborator
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@jkmassel
jkmassel requested a review from crazytonyli September 8, 2026 21:33
@jkmassel
jkmassel marked this pull request as ready for review September 8, 2026 21:33
@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 Number34428
VersionPR #26005
Bundle IDorg.wordpress.alpha
Commitb1f5001
Installation URL4c59spefarpb0
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 Number34428
VersionPR #26005
Bundle IDcom.jetpack.alpha
Commitb1f5001
Installation URL0hsv9cji63420
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.

3 participants