Skip to content

feat: process GutenbergKit media uploads with app Media settings - #25824

Open
dcalhoun wants to merge 30 commits into
trunkfrom
feat/process-gutenberg-kit-media-uploads
Open

feat: process GutenbergKit media uploads with app Media settings#25824
dcalhoun wants to merge 30 commits into
trunkfrom
feat/process-gutenberg-kit-media-uploads

Conversation

@dcalhoun

@dcalhoun dcalhoun commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description

Ref CMM-1249. Ref CMM-2155.

Related:

Integrates GutenbergKit's native media upload pipeline (wordpress-mobile/GutenbergKit#357) so device media picked in GutenbergKit is processed natively before upload, honoring the app's Media settings: Optimize Images, Max Image Upload Size, Image Quality, Max Video Upload Size, and Remove Location From Media. Previously these uploads went directly from the WebView to the REST API with no processing, bypassing the settings the legacy editor and My Site > Media already honor.

How it works

  • GBKMediaUploadProcessor implements GutenbergKit's MediaUploadDelegate. When the editor uploads a device file, GutenbergKit's localhost upload server hands the staged file to processFile, which configures MediaImageExporter or MediaVideoExporter with the same option mapping as MediaImportService:
    • Images are resized/recompressed per the Optimize Images settings, GPS EXIF is stripped per Remove Location, and non-web-safe formats (e.g. HEIC) are converted to JPEG, mirroring ItemProviderMediaExporter.
    • Videos are always transcoded per the Max Video Upload Size preset, matching the legacy iOS path (MediaImportService sets the preset unconditionally) and unlike Android, where video optimization is opt-in. The free-plan 5-minute duration limit is checked as a safety net, but the editor rejects video on free WP.com plans against the site's allowed mime types first, so videoLimitExceeded should rarely be what a user hits.
    • GIFs, SVGs, and non-media files pass through untouched (GutenbergKit forwards the original bytes verbatim).
    • When processing would be a no-op (optimization and location stripping disabled, web-safe format), the original file passes through without re-encoding.
  • handlesFile declines files the processor provably won't touch — GIFs, SVGs, and non-media files — so GutenbergKit forwards the original request body without first copying the file to disk. Images and videos are always claimed: what happens to them depends on Media settings or on the file's contents, and declining is unrecoverable because the delegate never sees the file again. A parity test asserts that anything declined would have returned .original, across the settings matrix.
  • Processed files upload under the name the editor sent, with only the extension taken from the export since a conversion changes it. The export's own name is unusable: GutenbergKit stages each upload as <uuid>-<filename> and MediaImageExporter(url:) seeds its output name from that, so using it would put the UUID into the attachment's slug and title — IMG_1234.HEIC becoming 09314a8e-…-img_1234.jpeg, and only when optimization is on, since unprocessed uploads keep the editor's name.
  • The site's allowed file types are not re-checked. GutenbergKit validates uploads in the WebView against the site's real allowedMimeTypes from /wp-block-editor/v1/settings before they reach the delegate, so anything arriving at processFile has already passed the authoritative check. Mirroring MediaURLExporter.exportURL and enforcing Blog.allowedFileTypes here could only ever produce false rejections: it cannot catch a type the editor missed, while Blog.allowedFileTypes is a cached blog option that can lag the server and MediaImportService.defaultAllowableFileExtensions is a 19-entry static list with no text types — enough to fail a .txt or .csv on a WP.com site whose allowed_file_types permits it. This is a deliberate divergence from the legacy path: parity matters where the two could produce different uploads, and this gate only ever accepted or refused one. Raised by review of the corresponding Android PR.
  • Upload itself still uses GutenbergKit's default uploader, which relays the raw WordPress REST response to the editor, so attachment objects, sub-sizes, and error notices behave identically to a direct upload.
  • Wired unconditionally in PostGBKEditorViewController (covers posts, pages, and custom post types). If GutenbergKit's upload server cannot start, it degrades gracefully to the standard WebView upload path.

Handling uploads the editor describes imprecisely

The multipart headers are less trustworthy than they look. Two cases would otherwise fail valid uploads outright:

  • No file extension. GutenbergKit names its temp file after the multipart filename, and the editor doesn't guarantee that carries an extension — its native inserter derives one from a URL path segment (media.url.split('/').pop() || 'media'). Resolving the type from the path alone yields public.data, which conforms to no media type, and the export would be rejected outright. The type falls back to the reported MIME type, and exports go through the concrete exporter rather than MediaURLExporter, which re-derives the type from the extension in exportURL and would reject the file a second time. MediaImageExporter reads the type from the file's contents via CGImageSourceGetType.
  • MIME types that need normalizing. Content-Type may carry parameters and arbitrary casing (RFC 9110 §8.3), and GutenbergKit's multipart parser substitutes text/plain for a part that sent no Content-Type at all (RFC 7578 §4.4) — it picks the file part by the presence of a filename parameter, not by content type. Left as-is, image/jpeg; charset=binary resolves to a dynamic type conforming to nothing, and a real photo announced as text/plain looks like a document. Parameters are stripped, casing lowered, and the placeholder types treated as absent, falling back to the filename extension.
  • Both delegate methods classify the type in the same order. handlesFile sees only the multipart headers and processFile sees the file, but both resolve the file's own type first and the reported one only as a fallback — handlesFile standing in for the not-yet-written file with the filename extension. Resolving in opposite orders would let an upload named photo.jpg carrying a mislabeled Content-Type: application/pdf be declined as a document and forwarded unprocessed, skipping downscaling and GPS stripping on a file processFile treats as an image. Declining is unrecoverable, so the two cannot disagree.

Temporary file handling

Every export is written to a single temporary directory, created once and reused. GutenbergKit deletes the file it was handed once the upload finishes — on the failure path as well as the success path — so no cleanup code is needed here at all.

Sharing one directory is safe despite URL.incrementalFilename() being an unlocked check-then-act fileExists loop: GutenbergKit writes each upload to <uuid>-<filename> and both exporters name their output after that lastPathComponent, so two concurrent exports cannot resolve to the same name. A fresh directory per export would instead need cleanup, since nothing sweeps those and each upload would leave an empty one behind for the lifetime of the container.

The one case neither side covers is a crash mid-upload. Those files land in the directory the next session reuses, and iOS reclaims the temporary directory under storage pressure and across app updates.

Notes

  • EXIF orientation. The no-op passthrough skips the legacy exporter's unconditional orientation normalization, so a sideways-shot photo uploads with its orientation flag intact rather than rotated into its pixels. This is deliberate. That normalization dates to May 2017 and predates WordPress 5.3 (Nov 2019), whose wp_create_image_subsizes rotates server-side for every site, self-hosted included. Baking in a rotation the server performs anyway would cost a lossy re-encode of a photo the user explicitly asked not to optimize. Android rotates here, but as a port of the same pre-5.3 workaround — and it already passes flagged files through untouched on WP.com. See Editor: vertical images appear horizontal in previews and on the web for self-hosted and Jetpack sites #12703 and core changeset 46202.
  • Divergence from legacy. The no-op passthrough itself is a deliberate difference: the legacy path always re-encodes, and with optimization off imageQualityForUpload still returns .high, so legacy rewrites a JPEG at quality 0.9. Passing the original bytes through avoids that generation loss. It only applies when the user has turned off both Optimize Images and Remove Location, which both default to on.
  • Uploaded attachments are not inserted into the local Core Data Media store, so My Site > Media shows them after the next sync. This matches GutenbergKit's pre-existing behavior; syncing after upload is a possible follow-up.
  • Blog.videoDurationLimit only returns a limit for free WP.com sites, which are the same sites whose allowed_file_types exclude video. The limit is kept for defense in depth — it still applies if the editor's mime-type filtering changes or a site allows video on a free plan — but it isn't the path a user takes today.
  • No shared media code is modified. The production diff is GBKMediaUploadProcessor.swift (new) and five lines of PostGBKEditorViewController.swift, both under NewGutenberg/, plus the GutenbergKit dependency bump. Legacy Gutenberg and Aztec select different view controllers in EditorFactory and never construct this processor.

Testing instructions

  1. Image resize: In App Settings > Media, enable Optimize Images with Max Image Upload Size 2000. Open a post in GutenbergKit, add an Image block, and upload a photo larger than 2000px from the device. Verify the upload succeeds and the stored full-size image is at most 2000px (inspect the attachment in wp-admin or the block's URL).
  2. No-op passthrough: Disable Optimize Images and Remove Location From Media, upload the same photo, and verify it retains its original dimensions and file size (the original bytes should be uploaded unchanged).
  3. Location stripping: Enable Remove Location From Media, upload a GPS-tagged photo, download the original from the site, and verify it has no GPS EXIF data.
  4. HEIC: Upload a HEIC image via the Files app and verify it arrives as JPEG.
  5. GIF: Upload an animated GIF and verify it still animates on the published post.
  6. Portrait photo: With Optimize Images off, upload a photo shot in portrait orientation and verify it appears upright on the published post (WordPress rotates it server-side).
  7. Video: On a site that allows video, set Max Video Upload Size to 480p, upload a video, and verify the transcoded resolution.
  8. Disallowed type: On a free WordPress.com site, try to insert a video. Verify the editor shows a notice that the file type is disallowed — this comes from GutenbergKit's own check against the site's allowed mime types, not from the processor.
  9. Documents upload unchanged: Insert a .txt or .csv via the Files app on a site that accepts it (self-hosted, or WP.com with the type allowed). Verify it uploads rather than failing with "This file type is not allowed" — the processor no longer second-guesses the editor's validation.
  10. Multiple at once: Select several photos in one go and verify every one uploads intact, with no missing or duplicated images.
  11. Verify behavior on both a WordPress.com site and a self-hosted site with an application password.

@dcalhoun dcalhoun added Media Gutenberg Editing and display of Gutenberg blocks. [Type] Enhancement labels Jul 22, 2026
@dcalhoun dcalhoun added this to the 27.2 milestone Jul 22, 2026
@dcalhoun
dcalhoun force-pushed the feat/process-gutenberg-kit-media-uploads branch from 5eb00ce to f373395 Compare July 22, 2026 20:12
Comment thread WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift Outdated
@dcalhoun
dcalhoun marked this pull request as ready for review July 22, 2026 20:37
@dcalhoun
dcalhoun requested a review from jkmassel July 22, 2026 20:37
@wpmobilebot

wpmobilebot commented Jul 22, 2026

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 Number34014
VersionPR #25824
Bundle IDorg.wordpress.alpha
Commit6526986
Installation URL75dflnjmjfiug
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 22, 2026

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 Number34014
VersionPR #25824
Bundle IDcom.jetpack.alpha
Commit6526986
Installation URL7rr4svb046s48
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@dangermattic

dangermattic commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator
1 Warning
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

Gates the upcoming native media upload processing for the experimental
block editor, with a Debug-menu override for quick disabling.
Points GutenbergKit at the XCFramework snapshot for
wordpress-mobile/GutenbergKit#357, which adds the native media upload
server and MediaUploadDelegate. Swap to a tagged release before merge.
Implements GutenbergKit's MediaUploadDelegate so device media picked in
the experimental block editor is processed natively before upload,
honoring the app's Media settings: Optimize Images, Max Image Upload
Size, Image Quality, Max Video Upload Size, and Remove Location From
Media. Previously these uploads went directly from the WebView to the
REST API with no processing.

GBKMediaUploadProcessor mirrors the exporter option mapping used by
MediaImportService and reuses MediaURLExporter, so the editor now
matches the behavior of the legacy editor and My Site > Media. GIFs
and non-media files pass through untouched, and non-web-safe image
formats (e.g. HEIC) are converted to JPEG. Uploads still use
GutenbergKit's default uploader, which relays the raw WordPress
response to the editor.
Verifies resizing, GPS stripping, HEIC-to-JPEG conversion, GIF and
no-op passthrough, disallowed file extensions, and the video duration
limit using the existing media fixtures.
Adds the gbkMediaUploadOptimization flag to the Experimental Features
list and aligns display names with the "New Block Editor (NBE)"
naming.
Removes the gbkMediaUploadOptimization gate ahead of reverting the
flag. GutenbergKit degrades gracefully if the upload server cannot
start, so a dedicated kill switch isn't needed.
Moves the pin off the pr-build/357 snapshot now that
wordpress-mobile/GutenbergKit#357 has merged. Trunk also carries the
follow-up hardening in #561, which adds a defaulted handlesFile(ofType:
named:) to MediaUploadDelegate, so GBKMediaUploadProcessor conforms
unchanged.

No tagged release includes #357 yet — v0.19.0 predates it. Swap to a
tagged release before merge.
@dcalhoun
dcalhoun force-pushed the feat/process-gutenberg-kit-media-uploads branch from e65c714 to 435046a Compare August 17, 2026 12:36
@wpmobilebot

wpmobilebot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

dcalhoun and others added 6 commits August 17, 2026 09:08
SVG conforms to `UTType.image`, so `MediaURLExporter.expectedExport`
classifies it as `.image` and it reaches the exporter. ImageIO cannot
decode or encode SVG: `CGImageSourceCreateWithURL` returns a source with
zero images, and both `CGImageSourceCreateThumbnailAtIndex` and
`CGImageDestinationCreateWithURL` return nil. The export therefore fails
instead of producing a file.

Return the original file for SVG, as we already do for GIF, and drop
`.svg` from `webSafeImageTypes`. That set decides whether an image needs
converting to JPEG, so it should only hold raster formats ImageIO can
actually read and write; SVG's membership there implied it could reach
the exporter safely.

Only sites whose plan allows SVG can upload one, so this is not reachable
on plans where the picker greys the file out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`MediaDirectory.temporary` is a computed property returning
`.temporary(id: UUID())`, so each export was written to a fresh
`tmp/<uuid>/Media/`. GutenbergKit's cleanup removes only the file at
`uploadURL`, never the two enclosing directories, so every processed
image or video left an empty directory pair behind for the lifetime of
the process.

Write every export to one directory identified by a fixed UUID.
`MediaFileManager.makeLocalMediaURL` increments filenames, so uploads
sharing a source name do not collide. The ID is stable across launches
so a directory orphaned by a crash is reused rather than accumulating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed the guard skips processing "when it would be a
no-op", which is broader than what the condition tests. With image
optimization off, `imageSizeForUpload` returns `Int.max` (no downscale)
but `imageQualityForUpload` returns `.high`, so an image that falls
through is still re-encoded at 0.9 quality.

Describe what the guard actually checks and note that the quality
mapping matches `MediaImportService`. No behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`makeExporter` is shared by the image and video paths, and it derived
`exportImageType` from the source URL's type alone. A video's UTI is not
in `webSafeImageTypes`, so every video export was configured to write
JPEG. `MediaURLExporter.exportVideo` ignores `imageOptions`, so this had
no effect, but it stated something untrue about the export.

Pass the classification `processFile` already computed and set
`exportImageType` only for an image. No behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`exportsShareOneTemporaryDirectory` cleaned up each export inside the
loop body, so `defer` fired at the end of every iteration and deleted
the file before the next export ran. `incrementalFilename` only
increments while a file exists, so all three exports resolved to the
same path and the directory set was trivially of size one. The test
passed whether or not exports shared a directory.

Collect the output URLs and clean them up after the loop so all three
files coexist, and assert they are three distinct paths in one
directory. Verified by reverting dd8c5bb locally: the test now fails
with three directories instead of one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The root cross-platform package still pinned GutenbergKit to the trunk
branch, left over from tracking trunk for native media uploads. Modules
moved to the 0.20.0-alpha.0 tag, so re-resolve the root graph to match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dcalhoun dcalhoun changed the title feat: process New Block Editor media uploads with app Media settings feat: process GutenbergKit media uploads with app Media settings Aug 18, 2026
Three related defects in GBKMediaUploadProcessor, all in the path between
classifying an upload and handing the export back to GutenbergKit.

Uploads without a file extension failed outright. `expectedExport` resolves
the type from the path extension alone, and GutenbergKit names its 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`, which conforms to no media type, so the
export was rejected and the editor saw a 500 for a file that previously
uploaded fine. Fall back to the reported MIME type, which the delegate
already receives and ignored.

Exports also had to stop going through `MediaURLExporter`, which re-derives
the type from the path extension in `exportURL` and would reject the file
again after it was classified. Use the concrete exporter per branch instead:
`MediaImageExporter` reads the type from the file's contents via
`CGImageSourceGetType`, so an extensionless image exports correctly.

Replace the fixed export directory with one per export. Destination names
come from `URL.incrementalFilename()`, a check-then-act `fileExists` loop
with no locking, and uploads are processed concurrently, so a shared
directory let two exports of the same source name resolve to the same path
and clobber each other. Nothing sweeps that directory either — GutenbergKit
removes only the file it is handed, and `MediaFileManager` cleans just the
uploads directory — so a failure after the export wrote a file abandoned a
full-size payload for the lifetime of the container. Clean up on the failure
path.

Derive the reported MIME type from `exportImageType` rather than re-reading
the output path, so it comes from the value the export was configured with.
GutenbergKit asks the delegate, from the multipart headers alone, whether
it will handle a file before streaming the upload to a temp file. The
default is `true`, so every upload was materialized in full even when
`processFile` immediately returned it unchanged — GIFs, and documents on a
site with no extension restriction to enforce.

Implement the gate for exactly those cases. It only declines where
`processFile` returns `.original` for any Media settings, so it stays a fast
path rather than a second place the policy lives; a parity test asserts that
invariant across the settings matrix. Images and videos are always claimed:
what happens to them depends on settings or on the file's contents, and
declining is unrecoverable because the file is never seen again.

Deciding from the reported MIME type needs it normalized first. `Content-Type`
may carry parameters and arbitrary casing (RFC 9110 §8.3), and GutenbergKit's
multipart parser substitutes `text/plain` for a part that sent no
`Content-Type` at all (RFC 7578 §4.4) — it picks the file part by the presence
of a `filename` parameter, not by content type. Left as-is, `image/jpeg;
charset=binary` resolves to a dynamic type that conforms to nothing, and a
real photo announced as `text/plain` looks like a document. Strip parameters,
lower the casing, and treat the placeholder types as absent, falling back to
the filename extension. `processFile` shares the helper, which also fixes the
same misreading in its extensionless-upload fallback.

Record why the image passthrough leaves EXIF orientation alone: the legacy
exporter's unconditional normalization predates WordPress 5.3, whose
`wp_create_image_subsizes` rotates server-side for every site, self-hosted
included. Baking in a rotation the server performs anyway would cost a lossy
re-encode of a photo the user asked not to optimize.
@dcalhoun

Copy link
Copy Markdown
Member Author

@jkmassel this work integrating GBK's media upload delegate is now ready for review. I updated the integration to match the latest GBK APIs and re-tested. Will you please review when you can? 🙇🏻‍♂️

@wpmobilebot wpmobilebot modified the milestones: 27.2, 27.3 Aug 19, 2026
@wpmobilebot

Copy link
Copy Markdown
Contributor

Version 27.2 has now entered code-freeze, so the milestone of this PR has been updated to 27.3.

processFile rejected a document whose extension was missing from both
MediaImportService.defaultAllowableFileExtensions and the site's
Blog.allowedFileTypes, mirroring MediaURLExporter.exportURL. That check
belongs to the legacy picker, which hands over arbitrary files with nothing
having vetted them. Here GutenbergKit has already validated the upload in the
WebView against the site's real allowedMimeTypes from
/wp-block-editor/v1/settings, so anything reaching processFile has passed the
authoritative check.

The second gate could therefore only ever produce false rejections: it cannot
catch a type the editor missed, but it can disagree with the server and
refuse a file the server would have accepted. Blog.allowedFileTypes is a
cached blog option that can lag the site's actual configuration, and
defaultAllowableFileExtensions is a 19-entry static list carrying no text
types — so a .txt or .csv on a WP.com site with a populated allowed_file_types
failed with "This file type is not allowed" where it previously uploaded.

Drop the check and let the server's own error surface if one is warranted.
Images and videos never reached it, so nothing that gets processed changes.

handlesFile's .other branch existed only to claim documents for that check, so
it now declines them unconditionally, joining .gif. Documents skip the
full temp-file copy GutenbergKit writes before processFile — the copy only
ever handed the file straight back.

This deliberately diverges from the legacy editor. Parity matters where the
two paths could produce different uploads; this gate only ever accepted or
refused one, so removing it cannot make the GutenbergKit path emit a
different file.

The convenience init's HEIC removal goes with it: it only ever narrowed the
allowlist. HEIC handling is unaffected — exportImageType converts it to JPEG,
as heicIsConvertedToJPEG pins.
Both tests built a PostGBKEditorViewController in a local, called
loadViewIfNeeded(), and returned. viewDidLoad kicks off prepareEditor() on a
Task, so the load reaches startUploadServer() after the test method has
already returned and released the controller.

Once this branch assigns mediaUploadDelegate, that is fatal. GutenbergKit
holds the delegate weakly and the controller is the only owner of its
GBKMediaUploadProcessor, so the delegate deallocates with the controller while
the editor itself is still alive inside the Task's [weak self]. That is exactly
the state GutenbergKit's precondition exists to catch:

    precondition(!(mediaUploadDelegateWasAssigned && mediaUploadDelegate == nil),
                 "mediaUploadDelegate was released before the editor loaded")

The crash log from CI symbolicates to it directly — EXC_BREAKPOINT in
_assertionFailure, called from startUploadServer() ← loadEditor(dependencies:)
← prepareEditor() ← closure #2 in viewDidLoad().

Because it kills the test host, the damage was not contained to this suite.
Swift Testing runs separate suites concurrently, so the four suites in flight
died with it (reported as "Crash: WordPress at <external symbol>") and
EditorConfigurationTests, torn down mid-test, read a Blog it never built and
failed on "Bearer token" and a stray 643603.example.com host. Six failures,
one cause, none of them in the suite responsible.

Route both tests through a makeEditor(blog:) helper that retains each window —
and through it the controller and processor — for the suite's lifetime. That
restores the production invariant: the controller outlives its own load. Only
the test's scope was shorter than the async work it started.

This does not reproduce locally: whether the Task reaches startUploadServer()
before or after the controller deallocates is a race, and a faster simulator
wins it. It failed on all three CI retries.

The suite becomes a final class so the retention array can be stored; Swift
Testing instantiates the type per test either way, and .serialized is kept.
`handlesFile` resolved `MIME ?? extension` while `processFile`'s
`sourceType(of:)` resolves `file ?? MIME`. The two classified differently
whenever the signals disagreed.

An upload named `photo.jpg` carrying a mislabeled `Content-Type:
application/pdf` resolved to `com.adobe.pdf`, classified `.other`, and was
declined — so it reached WordPress with no downscaling and, more importantly,
no GPS stripping. `processFile` would have resolved `public.jpeg` from the
extension and processed it.

Declining is unrecoverable: GutenbergKit forwards the original request body and
the file is never seen again. The gate therefore has to resolve in the same
order `processFile` does, standing in for the not-yet-written file with the
filename extension and falling back to the reported type.

This does not weaken the bias toward claiming a file. An unknown extension
resolves to a dynamic UTType that conforms to nothing, so `expectedExport`
throws and `handlesFile` still returns `true`, exactly as before.

The doc comment claimed "every `false` below mirrors a branch of `processFile`
that ignores `settings`", which is what this restores.
`processFile` returned `export.url.lastPathComponent`, and GutenbergKit sends
that verbatim as the multipart `filename`, which WordPress turns into the
attachment's slug and title.

That name is not the user's. GutenbergKit streams the upload to a temp file it
names `<uuid>-<filename>`, and `MediaImageExporter(url:)` seeds its output name
from `url.lastPathComponent`, so a photo picked as `IMG_1234.HEIC` uploaded as
`09314a8e-06a3-4b32-9d0d-b07c299fad5b-img_1234.jpeg`. The clean `filename` was
passed into `processFile` and never used.

The `.original` path was unaffected, so the same photo got a clean name with
optimization off and a UUID-laden one with it on.

Take the basename from the caller and only the extension from the export, since
a conversion (HEIC to JPEG, MOV to MP4) still has to be reflected in the name.
One consequence worth noting: the extension is normalized to the type's
preferred form, so a `.jpg` upload is now named `.jpeg` even when the format
does not change. The extension always matching the bytes is worth more than
preserving the spelling.

The existing test asserted only `hasPrefix("test-image-device-photo-gps")`,
which passed because it called `processFile` with a fixture URL directly rather
than through the `<uuid>-` name the server actually produces. The new test
routes through that name.
`makeExportDirectory()` minted `<tmp>/<uuid>/Media/` per export and only the
failure path removed it, so every processed upload left an empty directory
behind for the lifetime of the app's container. Nothing else reclaims them:
GutenbergKit deletes only the file it was handed, its own `cleanOrphanedUploads`
sweep scans just `GutenbergKit-uploads`, and `MediaFileManager`'s cleanup covers
`.uploads`.

Write every export to a single directory identified by a fixed ID. Created once
and reused, it never accumulates, so no cleanup code is needed at all.

The per-export directory existed to avoid a race in `incrementalFilename()`, an
unlocked check-then-act `fileExists` loop, on the assumption that two concurrent
exports could resolve to the same name. They cannot: GutenbergKit writes each
upload to `<uuid>-<filename>` and both exporters name their output after that
`lastPathComponent`, so every export name is already unique before the loop is
consulted. The test that covered the race passed the same source URL to every
task, a shape production never produces; it now uses the prefixed names the
editor actually sends.

Nor is any failure-path cleanup needed. GutenbergKit removes the file it was
handed on both the success and the failure path, so the only uncovered case is a
crash mid-upload. Those files land in the directory the next session reuses, and
iOS reclaims the temporary directory under storage pressure and across app
updates.
`processFile` returns `.original` for SVG whatever the settings, because ImageIO
can neither decode nor encode it. By the metadata gate's own rule — decline
anything `processFile` returns unchanged for any settings — SVG qualifies.

It was claimed anyway: `image/svg+xml` resolves to `public.svg-image`, which
conforms to `UTType.image`, so the gate fell into the image branch and returned
`true`. GutenbergKit then streamed the whole body to a temp file only for
`processFile` to hand it straight back, which is exactly the copy the gate
exists to avoid.
Both sites that read the set sit behind an `.image` classification: the
skip-export check in `processFile` and `exportImageType`, which guards on
`case .image`. `expectedExport` returns `.gif` for a GIF before it ever reaches
the `.image` branch, so no GIF can reach either one.

The member was therefore dead. Its doc comment reasoned carefully about why SVG
was absent while carrying an entry that could not be hit, which is a misleading
signal for anyone editing the set later. Rewrite the comment to say what the set
is actually for — the types that reach the image branch — and name GIF and SVG
together as the two web-safe formats deliberately handled earlier.

No behavior change.
viewDidLoad starts the editor's load on a Task that reaches
startUploadServer() after the test method has returned, where GutenbergKit
asserts an assigned mediaUploadDelegate is still alive. Since this branch
assigns that delegate, a controller released at scope exit leaves the editor
reachable with a dead delegate and trips the precondition.

That kills the test host, so the fallout lands elsewhere: concurrently running
suites report "Crash: WordPress at <external symbol>", and suites torn down
mid-test report assertion failures against state they never set up. Six
failures, none of them in the suite responsible.

Only a test can reach this. 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] on the editor is already nil.
Here a window is the controller's only owner, so releasing it mid-load
separates the two lifetimes. There is no production path.

Move retention to file scope. An earlier attempt stored the windows on the
suite itself, which does not work: Swift Testing builds a fresh suite instance
per test, so the array was released the moment a test returned — the very
deadline being missed.

This is hardening, not a build fix. The race is timing-dependent and CI is
currently green with the hazard still present, so no red-to-green
demonstration is possible; a passing run does not prove it is gone. The
failure mode is expensive enough — a host crash that corrupts unrelated
suites — to be worth closing regardless.
`sourceType(of:reportedMIMEType:)` fell back to the reported MIME type
only when the URL resolved to exactly `public.data`. An extension nothing
declares resolves instead to a *dynamic* type synthesized from it, which
clears that check while conforming to no media type, so `expectedExport`
threw `unsupportedFileType` and the upload failed with a 500.

`photo.jfif` is the case that matters: a plain JPEG WordPress accepts,
whose `Content-Type` says `image/jpeg`, uploaded fine before the editor
was given a media upload delegate. Defer to the reported type for a
dynamic type as well, and discard it in `type(ofExtensionIn:)` too so
`handlesFile` keeps classifying the file the same way `processFile` does.
The video branch exported unconditionally, but it is reached by UTType
conformance to `.video` or `.movie`, which is broader than the set of
containers AVFoundation opens. WebM and WMV both conform to `.movie` and
are absent from `AVURLAsset.audiovisualTypes()`, so `MediaVideoExporter`
failed them on `guard asset.isExportable` and the upload returned a 500.

Both formats are ones WordPress accepts — `wmv` is in the app's own
`MediaImportService.defaultAllowableFileExtensions` — and both uploaded
fine before the editor was given a media upload delegate. Return them
unchanged instead, and decline them in `handlesFile` so the temp copy is
skipped too. The export preset, duration limit, and location stripping
are unavailable for these files either way; failing the upload to say so
would only reject media the server would have taken.

Types are matched by conformance rather than identity so a subtype of a
readable format is not swept up with the unreadable ones.
The comment said the editor had already validated the file against the
site's `allowedMimeTypes` from `/wp-block-editor/v1/settings` before the
upload reached the processor. It has not: that route is provided by the
Gutenberg plugin, not WordPress core, so on a site without the plugin the
setting stays nil and the editor's check passes everything through.

State that instead, and keep the reason the branch is still correct — a
cached `Blog.allowedFileTypes` can lag the server, so rejecting from it
could only refuse a file the server would accept.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gutenberg Editing and display of Gutenberg blocks. Media [Type] Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants