Automattic / Automattic/wordpress-rs

Swift and Kotlin classify a present-but-unreadable upload file differently (`MediaFileUnreadable` vs `MediaFileNotFound`)

Open
#1,558 0 comments 0 reactions 1 assignee Claimed by @jkmassel View on GitHub
Bug Error Handling Kotlin Swift
Dominant language
Rust
Stars
36
Forks
5
Avg merge
17h 30m
Merged PRs (30d)
43

Description

## Summary

* For a media upload file that **exists but can't be read** (`chmod 000`), or a **directory** at the file path, the Swift and Kotlin executors classify the *same input* as *different* typed errors: **Swift →** `MediaFileUnreadable`, **Kotlin →** `MediaFileNotFound`.
* This straddles the exact `MediaFileNotFound` (couldn't be opened at all) vs `MediaFileUnreadable` (opened, then failed) distinction that Automattic/wordpress-rs#1546 introduced — and Automattic/wordpress-rs#1546's CHANGELOG states "Both executors produce it," a portability the two paths don't actually deliver for this input.

## Root Cause

* **Kotlin** pre-screens every file field before building the body: `canBeUploaded() = exists() && isFile && canRead()` (`native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt:286`). A `chmod 000` file fails `canRead()` and a directory fails `isFile`, so both throw `MediaFileNotFound` at `WpRequestExecutor.kt:143-144` — before the stream is ever opened.
* **Swift** builds the field with `FileManager.default.attributesOfItem(atPath:)` (`native/swift/Sources/wordpress-api/MultipartForm.swift:63`), a `stat` that needs no read permission on the file, so construction succeeds. The failure surfaces later at serialization (`open()`/`read()` → `streamStatus == .error` / `bytesRead < 0`, `MultipartForm.swift:204-234`) as `.inaccessibleFile(filePath:)`, which maps to `MediaFileUnreadable` (`SafeRequestExecutor.swift:652`).
* The genuinely-absent-file case and the deleted-between-check-and-open case *agree* across platforms (both `MediaFileNotFound`, both `MediaFileUnreadable` respectively). Only the present-but-unreadable / non-regular case diverges.

## Impact

A shared cross-platform consumer keying on the error type behaves inconsistently by platform:

* **User messaging**: iOS shows "could not be read", Android shows "not found" — for a file that is sitting right there. The Android copy is actively misleading (it exists).
* **Analytics**: the same root cause double-buckets across `MediaFileUnreadable` and `MediaFileNotFound`, so neither platform's rate is comparable.
* **Retry / recovery**: a consumer that treats `NotFound` as terminal but `Unreadable` as retryable (e.g. an iCloud-evicted file that may re-materialize — the PR's own motivation) retries on iOS and gives up on Android for identical input.

The input class isn't exotic on mobile: `NSFileProtectionComplete` files on a locked device, and file-provider / iCloud-evicted items, are all `stat`-succeeds-but-`read`-fails.

## Notes

* **Pre-existing, not a regression.** Kotlin's `canBeUploaded()` predicate predates Automattic/wordpress-rs#1546; before Automattic/wordpress-rs#1545/Automattic/wordpress-rs#1546, Swift *crashed* on a directory (`Data(bytesNoCopy:count:-1)` traps) and *silently truncated* a `chmod 000` upload. So this is strictly better than the prior state — just still not aligned.
* `MediaFileUnreadable` is arguably the *more* correct label here (the file exists; "not found" asserts absence, which is false). So aligning most likely means teaching Kotlin's pre-flight to distinguish `exists && !canRead` from truly-absent, **not** regressing Swift back to `MediaFileNotFound`.

## Suggested Fix

1. Split Kotlin's `canBeUploaded()` so `exists() && isFile() && !canRead()` (and a directory) route to `MediaFileUnreadable` rather than `MediaFileNotFound`, matching Swift. ⚠️ This touches a pre-flight predicate with its own TOCTOU surface, so it needs its own test.
2. Either way, soften Automattic/wordpress-rs#1546's CHANGELOG line so it stops promising "Both executors produce it" for a present-but-unreadable file until the classification is actually aligned.

## Related issues

* Follow-up from the review of Automattic/wordpress-rs#1546 (fixes Automattic/wordpress-rs#1541). Part of the Swift executor error audit (Automattic/wordpress-rs#1497).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.