From 872880b6395f51da9fdd3e62bc4df4e9c5c26ec7 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 10:15:23 -0400 Subject: [PATCH 01/27] build: bump GutenbergKit to PR 357 media-upload-delegate snapshot Consumes the snapshot build of wordpress-mobile/GutenbergKit#357, which adds the MediaUploadDelegate API for host-side media upload processing. To be swapped to a tagged release before merge. Co-Authored-By: Claude Opus 4.8 (1M context) --- gradle/libs.versions.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 5c2fc92b3667..a3615bb1e62c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -73,7 +73,8 @@ google-play-review = '2.0.2' google-services = '4.5.0' gravatar = '2.5.0' greenrobot-eventbus = '3.3.1' -gutenberg-kit = 'v0.19.0' +# TODO: Swap to a tagged GutenbergKit release before merge (snapshot of wordpress-mobile/GutenbergKit#357) +gutenberg-kit = '357-45219181e3aa60b5ce3119367758d90e6b6beb0b' gutenberg-mobile = 'v1.121.0' indexos-media-for-mobile = '43a9026f0973a2f0a74fa813132f6a16f7499c3a' jackson-databind = '2.12.7.1' From 663c1018ddfb945b4e1ac7c525c38a1efeafbc64 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 10:17:59 -0400 Subject: [PATCH 02/27] feat: expose media optimization prefs and EXIF strip via wrappers Adds video optimization and strip-image-location accessors to AppPrefsWrapper, and a stripImageLocation passthrough to MediaUtilsWrapper, so the upcoming GutenbergKit media upload processor can read settings and strip GPS EXIF through injectable, testable wrappers. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../wordpress/android/ui/prefs/AppPrefsWrapper.kt | 12 ++++++++++++ .../org/wordpress/android/util/MediaUtilsWrapper.kt | 3 +++ 2 files changed, 15 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt index 8fc18b7bf05e..708e80fa5b3e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt @@ -49,6 +49,18 @@ class AppPrefsWrapper @Inject constructor(val buildConfigWrapper: BuildConfigWra get() = AppPrefs.isAztecEditorEnabled() set(enabled) = AppPrefs.setAztecEditorEnabled(enabled) + val isVideoOptimize: Boolean + get() = AppPrefs.isVideoOptimize() + + val videoOptimizeWidth: Int + get() = AppPrefs.getVideoOptimizeWidth() + + val videoOptimizeQuality: Int + get() = AppPrefs.getVideoOptimizeQuality() + + val isStripImageLocation: Boolean + get() = AppPrefs.isStripImageLocation() + var postListAuthorSelection: AuthorFilterSelection get() = AppPrefs.getAuthorFilterSelection() set(value) = AppPrefs.setAuthorFilterSelection(value) diff --git a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt index b906fbd95702..f0781a8728e7 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt @@ -35,6 +35,9 @@ class MediaUtilsWrapper @Inject constructor(private val appContext: Context) { fun isVideoMimeType(mimeType: String?): Boolean = org.wordpress.android.fluxc.utils.MediaUtils.isVideoMimeType(mimeType) + fun stripImageLocation(imagePath: String) = + org.wordpress.android.fluxc.utils.MediaUtils.stripLocation(imagePath) + fun isInMediaStore(mediaUri: Uri?): Boolean = MediaUtils.isInMediaStore(mediaUri) From b07bf23c712c1707ec0d11a6be5255dabfdb026d Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 10:19:32 -0400 Subject: [PATCH 03/27] feat: add GBKMediaUploadProcessor for GutenbergKit media uploads Implements GutenbergKit's MediaUploadDelegate to process device media picked in the experimental block editor before upload, honoring the app's media settings the same way the legacy editor pipeline does: - Images are optimized per the max size/quality settings via WPMediaUtils.getOptimizedMedia, with GPS EXIF stripped when the strip-location setting is on, and the reported mime type/extension corrected to the actual output format (non-PNG inputs, including HEIC, re-encode to JPEG). - When processing would be a no-op, the original file passes through untouched, avoiding a needless lossy re-encode. - Sideways-captured images are rotated for self-hosted sites when optimization is off (WP.com rotates server-side; issue #5737). - GIFs always pass through to preserve animation. - Videos enforce the free-plan 5-minute duration limit and transcode via WPVideoUtils/m4m only when the optimize-video setting is on, keeping the transcoded file only when smaller than the original. Transcodes are serialized to bound codec/memory pressure. - File types disallowed by the site plan are rejected with a localized message relayed to the editor as a notice. Nothing wires the processor yet; that lands separately. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 251 ++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt new file mode 100644 index 000000000000..0098bb3ef18b --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -0,0 +1,251 @@ +package org.wordpress.android.ui.posts.editor + +import android.content.Context +import android.net.Uri +import android.webkit.MimeTypeMap +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import org.m4m.IProgressListener +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.ui.prefs.AppPrefsWrapper +import org.wordpress.android.util.AppLog +import org.wordpress.android.util.MediaUtils +import org.wordpress.android.util.MediaUtilsWrapper +import org.wordpress.android.util.WPVideoUtils +import org.wordpress.gutenberg.MediaUploadDelegate +import org.wordpress.gutenberg.ProcessedProxyFile +import java.io.File +import kotlin.coroutines.resume + +/** + * Processes device media picked in the GutenbergKit editor before upload, honoring the app's + * media settings (image optimization, quality, EXIF location stripping, video optimization) the + * same way the legacy editor's upload pipeline does. + * + * Set as [org.wordpress.gutenberg.GutenbergView.mediaUploadDelegate]; GutenbergKit invokes + * [processFile] for every editor upload and uploads the result itself (this class deliberately + * does not override `uploadFile`, so GutenbergKit's default uploader posts to `/wp/v2/media` + * and relays WordPress's raw response to the editor). + * + * Contract notes (see GutenbergKit's MediaUploadServer): + * - Returning [ProcessedProxyFile.Original] makes GutenbergKit forward the original request body + * byte-for-byte — mutations to the staged [File] are NOT uploaded. Any change intended for + * WordPress must be returned as [ProcessedProxyFile.Processed]. + * - Processed output files are deleted by GutenbergKit after the upload, so they are written to + * the cache dir and never registered in the app's media store. + * - Thrown exceptions are relayed to the editor as an error notice showing the exception message, + * so messages must be localized and user-facing. + */ +class GBKMediaUploadProcessor( + private val site: SiteModel, + private val appContext: Context, + private val mediaUtilsWrapper: MediaUtilsWrapper, + private val appPrefsWrapper: AppPrefsWrapper, + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, +) : MediaUploadDelegate { + /** + * Serializes video transcodes. GutenbergKit's upload server handles requests concurrently, + * but parallel m4m hardware transcodes are memory/codec-heavy; the legacy pipeline + * effectively serialized them through the upload queue. + */ + private val transcodeMutex = Mutex() + + override suspend fun processFile( + file: File, + mimeType: String, + filename: String + ): ProcessedProxyFile = withContext(ioDispatcher) { + val resolvedMimeType = resolveMimeType(mimeType, filename) + + // Reject types the site's plan doesn't allow (e.g. audio on free WP.com plans) with a + // localized message instead of an opaque server-side error. + if (!mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType)) { + throw GBKMediaUploadException(appContext.getString(R.string.error_media_file_type_not_allowed)) + } + + when { + // Never re-encode GIFs — it would flatten animation. Passthrough skips even a copy. + resolvedMimeType == MIME_GIF -> ProcessedProxyFile.Original + mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> processVideo(file, filename) + resolvedMimeType.startsWith(MIME_IMAGE_PREFIX) -> processImage(file, resolvedMimeType, filename) + // Non-media files (documents, archives, audio on paid plans) upload unchanged. + else -> ProcessedProxyFile.Original + } + } + + private suspend fun processVideo(file: File, filename: String): ProcessedProxyFile { + if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, Uri.fromFile(file))) { + throw GBKMediaUploadException( + appContext.getString(R.string.error_media_video_duration_exceeds_limit) + ) + } + + // Match the legacy pipeline: transcode only when the user enabled video optimization. + if (!appPrefsWrapper.isVideoOptimize) return ProcessedProxyFile.Original + + val output = transcodeMutex.withLock { transcodeVideo(file) } ?: return ProcessedProxyFile.Original + + // Match VideoOptimizer: only use the transcoded file when it is actually smaller. + if (output.length() >= file.length()) { + output.delete() + return ProcessedProxyFile.Original + } + + return ProcessedProxyFile.Processed( + file = output, + mimeType = MIME_MP4, + filename = "${filename.substringBeforeLast('.')}.mp4" + ) + } + + /** + * Transcodes the video per the user's optimization settings, mirroring the legacy + * [org.wordpress.android.ui.uploads.VideoOptimizer] semantics: any failure (no composer, + * m4m error) resolves to null so the caller falls back to uploading the original. + */ + @Suppress("TooGenericExceptionCaught", "SwallowedException") + private suspend fun transcodeVideo(input: File): File? = suspendCancellableCoroutine { continuation -> + val output = File(appContext.cacheDir, MediaUtils.generateTimeStampedFileName(MIME_MP4)) + val listener = object : IProgressListener { + override fun onMediaStart() = Unit + override fun onMediaProgress(progress: Float) = Unit + override fun onMediaPause() = Unit + + // onMediaStop fires both on completion (before onMediaDone) and on manual stop, so + // only onMediaDone/onError complete the coroutine, guarded against double-resume. + override fun onMediaStop() = Unit + + override fun onMediaDone() { + if (continuation.isActive) continuation.resume(output) + } + + override fun onError(exception: Exception) { + AppLog.e(AppLog.T.MEDIA, "GBKMediaUploadProcessor > video transcode failed", exception) + output.delete() + if (continuation.isActive) continuation.resume(null) + } + } + + val composer = try { + WPVideoUtils.getVideoOptimizationComposer( + appContext, + input.absolutePath, + output.absolutePath, + listener, + appPrefsWrapper.videoOptimizeWidth, + appPrefsWrapper.videoOptimizeQuality + ) + } catch (npe: NullPointerException) { + // m4m throws NPEs on some malformed inputs; the legacy pipeline guards this too. + AppLog.w(AppLog.T.MEDIA, "GBKMediaUploadProcessor > NPE getting composer: ${npe.message}") + null + } + + if (composer == null) { + output.delete() + continuation.resume(null) + return@suspendCancellableCoroutine + } + + continuation.invokeOnCancellation { + try { + composer.stop() + } catch (e: Exception) { + AppLog.w(AppLog.T.MEDIA, "GBKMediaUploadProcessor > error stopping composer: ${e.message}") + } + output.delete() + } + + composer.start() + } + + private fun processImage(file: File, mimeType: String, filename: String): ProcessedProxyFile { + // getOptimizedMedia returns null when optimization is disabled or a no-op. It can also + // return the *input* path unchanged (GIF-like skips, decode failures inside + // ImageUtils.optimizeImage) — treat that as "not optimized" too, otherwise the original + // file would be mislabeled with a corrected JPEG mime type below. + val optimizedPath = mediaUtilsWrapper.getOptimizedMedia(file.absolutePath, false) + ?.path + ?.takeIf { it != file.absolutePath } + + if (optimizedPath != null) { + return processedImage(File(optimizedPath), mimeType, filename) + } + + // With optimization off, WP.com rotates sideways-captured images server-side but + // self-hosted sites don't, so rotate physically (legacy parity — see issue #5737). + // Returns null when no rotation is needed. + if (!site.isWPCom) { + val rotatedPath = mediaUtilsWrapper.fixOrientationIssue(file.absolutePath, false) + ?.path + ?.takeIf { it != file.absolutePath } + if (rotatedPath != null) { + return processedImage(File(rotatedPath), mimeType, filename) + } + } + + if (appPrefsWrapper.isStripImageLocation && mimeType in EXIF_MIME_TYPES) { + // A copy is required: returning Original makes GutenbergKit forward the original + // request body byte-for-byte, so stripping EXIF from the staged file in place would + // silently upload the un-stripped bytes. + val copy = File.createTempFile("gbk-media", ".${file.extension}", appContext.cacheDir) + file.copyTo(copy, overwrite = true) + mediaUtilsWrapper.stripImageLocation(copy.absolutePath) + return ProcessedProxyFile.Processed(copy, mimeType, filename) + } + + // No-op: optimization off/unneeded, no rotation, no location strip. Passing the original + // through avoids the needless lossy re-encode the legacy pipeline never did either. + return ProcessedProxyFile.Original + } + + /** + * Wraps an optimized/rotated image file, stripping GPS EXIF when enabled and correcting the + * reported mime type and filename: ImageUtils re-encodes PNG to PNG and everything else + * (including HEIC/WebP) to JPEG bytes while keeping the original file extension, so the + * metadata sent to WordPress must reflect the actual output format. + */ + private fun processedImage(output: File, inputMimeType: String, filename: String): ProcessedProxyFile { + if (appPrefsWrapper.isStripImageLocation) { + // getOptimizedMedia copies the original's EXIF (including GPS) onto its output, so + // the strip must run on the output — matching the legacy strip-at-upload behavior. + mediaUtilsWrapper.stripImageLocation(output.absolutePath) + } + + val basename = filename.substringBeforeLast('.') + return if (inputMimeType == MIME_PNG) { + ProcessedProxyFile.Processed(output, MIME_PNG, "$basename.png") + } else { + ProcessedProxyFile.Processed(output, MIME_JPEG, "$basename.jpg") + } + } + + private fun resolveMimeType(mimeType: String, filename: String): String { + if (mimeType.isNotBlank() && mimeType != MIME_OCTET_STREAM) return mimeType + val extension = filename.substringAfterLast('.', "").lowercase() + return MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension) ?: MIME_OCTET_STREAM + } + + companion object { + private const val MIME_IMAGE_PREFIX = "image/" + private const val MIME_GIF = "image/gif" + private const val MIME_PNG = "image/png" + private const val MIME_JPEG = "image/jpeg" + private const val MIME_MP4 = "video/mp4" + private const val MIME_OCTET_STREAM = "application/octet-stream" + + /** Image formats that can carry EXIF GPS metadata. */ + private val EXIF_MIME_TYPES = setOf(MIME_JPEG, "image/heic", "image/heif", "image/webp") + } +} + +/** + * Thrown to reject an upload; GutenbergKit relays [message] to the editor as an error notice, + * so it must be localized and user-facing. + */ +class GBKMediaUploadException(message: String) : Exception(message) From adca47aa2f3a88acda4b2e83b02721e17690ffba Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 10:25:18 -0400 Subject: [PATCH 04/27] test: cover GBKMediaUploadProcessor decision table Adds JVM unit tests for the processor's decision table: optimized image output with corrected mime/extension (incl. HEIC to JPEG and PNG passthrough), no-op short-circuit, GPS stripping onto the optimized output and onto a copy when optimization is off, GIF passthrough, self-hosted orientation fix, plan-disallowed type and over-limit video rejections with localized messages, video passthrough when optimization is disabled, and same-path optimization results treated as unprocessed. Also adds a File-based isProhibitedVideoDuration overload to MediaUtilsWrapper so the processor avoids Uri.fromFile, keeping it testable on the JVM. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 3 +- .../android/util/MediaUtilsWrapper.kt | 4 + .../editor/GBKMediaUploadProcessorTest.kt | 250 ++++++++++++++++++ 3 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 0098bb3ef18b..a18e54428fdf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -1,7 +1,6 @@ package org.wordpress.android.ui.posts.editor import android.content.Context -import android.net.Uri import android.webkit.MimeTypeMap import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers @@ -79,7 +78,7 @@ class GBKMediaUploadProcessor( } private suspend fun processVideo(file: File, filename: String): ProcessedProxyFile { - if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, Uri.fromFile(file))) { + if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, file)) { throw GBKMediaUploadException( appContext.getString(R.string.error_media_video_duration_exceeds_limit) ) diff --git a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt index f0781a8728e7..6c31562b887d 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt @@ -8,6 +8,7 @@ import org.wordpress.android.editor.EditorMediaUtils import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.utils.MimeTypes.Plan import org.wordpress.android.util.AppLog.T +import java.io.File import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -63,6 +64,9 @@ class MediaUtilsWrapper @Inject constructor(private val appContext: Context) { fun isVideoFile(mediaUri: Uri): Boolean = isVideo(mediaUri) || isVideoMimeType(getMimeType(mediaUri)) + fun isProhibitedVideoDuration(context: Context, site: SiteModel, file: File): Boolean = + isProhibitedVideoDuration(context, site, Uri.fromFile(file)) + fun isProhibitedVideoDuration(context: Context, site: SiteModel, uri: Uri): Boolean { if (isVideoFile(uri) && site.hasFreePlan && !site.isActiveModuleEnabled("videopress")) { val retriever = MediaMetadataRetriever() diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt new file mode 100644 index 000000000000..29d3789846cc --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -0,0 +1,250 @@ +package org.wordpress.android.ui.posts.editor + +import android.content.Context +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.mockito.junit.MockitoJUnitRunner +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.ui.prefs.AppPrefsWrapper +import org.wordpress.android.util.MediaUtilsWrapper +import org.wordpress.gutenberg.ProcessedProxyFile +import java.io.File + +@ExperimentalCoroutinesApi +@RunWith(MockitoJUnitRunner::class) +class GBKMediaUploadProcessorTest : BaseUnitTest() { + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var appContext: Context + private lateinit var mediaUtilsWrapper: MediaUtilsWrapper + private lateinit var appPrefsWrapper: AppPrefsWrapper + private lateinit var stagedFile: File + + @Before + fun setUp() { + appContext = mock { + on { getString(R.string.error_media_file_type_not_allowed) } doReturn FILE_TYPE_ERROR + on { getString(R.string.error_media_video_duration_exceeds_limit) } doReturn VIDEO_LIMIT_ERROR + } + mediaUtilsWrapper = mock { + on { isMimeTypeSupportedBySitePlan(anyOrNull(), any()) } doReturn true + } + appPrefsWrapper = mock() + stagedFile = tempFolder.newFile("photo.jpg").apply { writeText("staged-bytes") } + } + + private fun createProcessor(site: SiteModel = wpComSite()) = GBKMediaUploadProcessor( + site = site, + appContext = appContext, + mediaUtilsWrapper = mediaUtilsWrapper, + appPrefsWrapper = appPrefsWrapper, + ioDispatcher = testDispatcher() + ) + + private fun wpComSite() = SiteModel().apply { setIsWPCom(true) } + + private fun selfHostedSite() = SiteModel().apply { setIsWPCom(false) } + + @Test + fun `image is optimized when optimization produces a new file`() = test { + val optimized = tempFolder.newFile("optimized.jpg") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isInstanceOf(ProcessedProxyFile.Processed::class.java) + result as ProcessedProxyFile.Processed + assertThat(result.file.absolutePath).isEqualTo(optimized.absolutePath) + assertThat(result.mimeType).isEqualTo("image/jpeg") + assertThat(result.filename).isEqualTo("photo.jpg") + } + + @Test + fun `image passes through when processing would be a no-op`() = test { + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(false) + + val result = createProcessor(wpComSite()).processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).fixOrientationIssue(any(), any()) + } + + @Test + fun `gps is stripped onto a copy when strip enabled and optimization off`() = test { + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + + val result = createProcessor(wpComSite()).processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isInstanceOf(ProcessedProxyFile.Processed::class.java) + result as ProcessedProxyFile.Processed + // Stripping must run on a copy, never on the staged file — Original passthrough would + // re-send the original request body and discard an in-place edit. + assertThat(result.file.absolutePath).isNotEqualTo(stagedFile.absolutePath) + assertThat(result.file.readText()).isEqualTo("staged-bytes") + assertThat(result.mimeType).isEqualTo("image/jpeg") + assertThat(result.filename).isEqualTo("photo.jpg") + verify(mediaUtilsWrapper).stripImageLocation(result.file.absolutePath) + result.file.delete() + } + + @Test + fun `gps is stripped from the optimized output when strip enabled`() = test { + val optimized = tempFolder.newFile("optimized.jpg") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) + .thenReturn(optimizedUri) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + + createProcessor().processFile(stagedFile, "image/jpeg", "photo.jpg") + + verify(mediaUtilsWrapper).stripImageLocation(optimized.absolutePath) + } + + @Test + fun `heic reports jpeg mime type and extension after optimization`() = test { + val heicStaged = tempFolder.newFile("photo.heic") + val optimized = tempFolder.newFile("optimized.heic") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(heicStaged.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(heicStaged, "image/heic", "photo.heic") + + assertThat(result).isInstanceOf(ProcessedProxyFile.Processed::class.java) + result as ProcessedProxyFile.Processed + assertThat(result.mimeType).isEqualTo("image/jpeg") + assertThat(result.filename).isEqualTo("photo.jpg") + } + + @Test + fun `png keeps png mime type and extension after optimization`() = test { + val pngStaged = tempFolder.newFile("art.png") + val optimized = tempFolder.newFile("optimized.png") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(pngStaged.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(pngStaged, "image/png", "art.png") + + result as ProcessedProxyFile.Processed + assertThat(result.mimeType).isEqualTo("image/png") + assertThat(result.filename).isEqualTo("art.png") + } + + @Test + fun `gif passes through untouched`() = test { + val gifStaged = tempFolder.newFile("anim.gif") + + val result = createProcessor().processFile(gifStaged, "image/gif", "anim.gif") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).getOptimizedMedia(any(), any()) + } + + @Test + fun `disallowed file type throws with localized message`() = test { + whenever(mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(anyOrNull(), any())).thenReturn(false) + val zipStaged = tempFolder.newFile("archive.zip") + + val thrown = runCatching { + createProcessor().processFile(zipStaged, "application/zip", "archive.zip") + }.exceptionOrNull() + + assertThat(thrown) + .isInstanceOf(GBKMediaUploadException::class.java) + .hasMessage(FILE_TYPE_ERROR) + } + + @Test + fun `video exceeding duration limit throws with localized message`() = test { + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any())).thenReturn(true) + val videoStaged = tempFolder.newFile("movie.mp4") + + val thrown = runCatching { + createProcessor().processFile(videoStaged, "video/mp4", "movie.mp4") + }.exceptionOrNull() + + assertThat(thrown) + .isInstanceOf(GBKMediaUploadException::class.java) + .hasMessage(VIDEO_LIMIT_ERROR) + } + + @Test + fun `video passes through when optimization disabled`() = test { + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any())).thenReturn(false) + whenever(appPrefsWrapper.isVideoOptimize).thenReturn(false) + val videoStaged = tempFolder.newFile("movie.mp4") + + val result = createProcessor().processFile(videoStaged, "video/mp4", "movie.mp4") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + } + + @Test + fun `optimization returning the input path is treated as not optimized`() = test { + // ImageUtils.optimizeImage returns the original path for skips/failures; wrapping it in + // Processed would mislabel the original file with a corrected mime type. + val inputPathUri = fileUri(stagedFile) + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) + .thenReturn(inputPathUri) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(false) + + val result = createProcessor(wpComSite()).processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + } + + @Test + fun `self-hosted image is rotated when optimization is off`() = test { + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(null) + val rotated = tempFolder.newFile("rotated.jpg") + val rotatedUri = fileUri(rotated) + whenever(mediaUtilsWrapper.fixOrientationIssue(stagedFile.absolutePath, false)) + .thenReturn(rotatedUri) + + val result = createProcessor(selfHostedSite()).processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isInstanceOf(ProcessedProxyFile.Processed::class.java) + result as ProcessedProxyFile.Processed + assertThat(result.file.absolutePath).isEqualTo(rotated.absolutePath) + } + + @Test + fun `non-media file allowed by the site plan passes through`() = test { + val docStaged = tempFolder.newFile("doc.pdf") + + val result = createProcessor().processFile(docStaged, "application/pdf", "doc.pdf") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + } + + private fun fileUri(file: File): android.net.Uri = mock { + on { path } doReturn file.absolutePath + } + + companion object { + private const val FILE_TYPE_ERROR = "This file type is not allowed" + private const val VIDEO_LIMIT_ERROR = "Uploading videos longer than 5 minutes requires a paid plan." + } +} From efce1279f4b661112bc9c3a9bd9f0db5edfd9506 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 10:28:48 -0400 Subject: [PATCH 05/27] feat: wire GBKMediaUploadProcessor into the GutenbergKit editor Sets the media upload delegate on GutenbergView so device media picked in the experimental block editor is processed per the app's media settings before upload. The fragment stores and forwards the delegate following the existing setNetworkRequestListener pattern (GutenbergView tolerates assignment before or after page load), and the activity constructs the processor with its SiteModel and injected wrappers. If GutenbergKit's upload server fails to start, uploads degrade to the existing WebView path unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/posts/GutenbergKitActivity.kt | 15 +++++++++++++++ .../ui/posts/editor/GutenbergKitEditorFragment.kt | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt index 8af182756744..fd85f47a9e52 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt @@ -65,6 +65,7 @@ import org.wordpress.android.editor.EditorImagePreviewListener import org.wordpress.android.editor.EditorImageSettingsListener import org.wordpress.android.editor.ExceptionLogger import org.wordpress.android.editor.gutenberg.DialogVisibility +import org.wordpress.android.ui.posts.editor.GBKMediaUploadProcessor import org.wordpress.android.ui.posts.editor.GutenbergKitEditorFragment import org.wordpress.android.ui.posts.editor.GutenbergKitNetworkLogger import org.wordpress.android.editor.savedinstance.SavedInstanceDatabase @@ -172,6 +173,7 @@ import org.wordpress.android.ui.posts.reactnative.ReactNativeRequestHandler import org.wordpress.android.ui.posts.sharemessage.EditJetpackSocialShareMessageActivity import org.wordpress.android.ui.posts.sharemessage.EditJetpackSocialShareMessageActivity.Companion.createIntent import org.wordpress.android.ui.prefs.AppPrefs +import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.ui.prefs.SiteSettingsInterface import org.wordpress.android.ui.prefs.SiteSettingsInterface.SiteSettingsListener import org.wordpress.android.ui.reader.utils.ReaderUtilsWrapper @@ -193,6 +195,7 @@ import org.wordpress.android.util.DateTimeUtilsWrapper import org.wordpress.android.util.DisplayUtils import org.wordpress.android.util.FluxCUtils import org.wordpress.android.util.MediaUtils +import org.wordpress.android.util.MediaUtilsWrapper import org.wordpress.android.util.NetworkUtils import org.wordpress.android.util.ReblogUtils import org.wordpress.android.util.ShortcutUtils @@ -387,6 +390,8 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene @Inject lateinit var editorJetpackSocialViewModel: EditorJetpackSocialViewModel @Inject lateinit var gutenbergKitNetworkLogger: GutenbergKitNetworkLogger @Inject lateinit var gutenbergKitSettingsBuilder: GutenbergKitSettingsBuilder + @Inject lateinit var mediaUtilsWrapper: MediaUtilsWrapper + @Inject lateinit var appPrefsWrapper: AppPrefsWrapper private lateinit var editPostNavigationViewModel: EditPostNavigationViewModel private lateinit var editPostSettingsViewModel: EditPostSettingsViewModel private lateinit var prepublishingViewModel: PrepublishingViewModel @@ -2260,6 +2265,16 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene } ) } + + // Process device media per the app's media settings before upload + editorFragment?.setMediaUploadDelegate( + GBKMediaUploadProcessor( + site = siteModel, + appContext = applicationContext, + mediaUtilsWrapper = mediaUtilsWrapper, + appPrefsWrapper = appPrefsWrapper, + ) + ) } VIEW_PAGER_PAGE_SETTINGS -> editPostSettingsFragment = fragment as EditPostSettingsFragment } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt index 820975ce3ec2..393f080ea756 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt @@ -39,6 +39,7 @@ import org.wordpress.gutenberg.GutenbergView.LogJsExceptionListener import org.wordpress.gutenberg.GutenbergView.OpenMediaLibraryListener import org.wordpress.gutenberg.GutenbergView.TitleAndContentCallback import org.wordpress.gutenberg.Media +import org.wordpress.gutenberg.MediaUploadDelegate import org.wordpress.gutenberg.model.EditorConfiguration import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit @@ -61,6 +62,7 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { private var onLogJsExceptionListener: LogJsExceptionListener? = null private var modalDialogStateListener: GutenbergView.ModalDialogStateListener? = null private var networkRequestListener: GutenbergView.NetworkRequestListener? = null + private var mediaUploadDelegate: MediaUploadDelegate? = null private var rootView: View? = null private var isXPostsEnabled: Boolean = false @@ -224,6 +226,9 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { networkRequestListener?.let( gutenbergView::setNetworkRequestListener ) + mediaUploadDelegate?.let { + gutenbergView.mediaUploadDelegate = it + } // Set up content provider for WebView refresh recovery gutenbergView.setLatestContentProvider( @@ -552,6 +557,11 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { gutenbergView?.setNetworkRequestListener(listener) } + fun setMediaUploadDelegate(delegate: MediaUploadDelegate) { + mediaUploadDelegate = delegate + gutenbergView?.mediaUploadDelegate = delegate + } + override fun onUndoPressed() { gutenbergView?.undo() } From 887c544534bad461a7919568dd44d3c73321609f Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 23 Jul 2026 15:58:38 -0400 Subject: [PATCH 06/27] build: bump WordPress-Utils to the orientation-fix snapshot Consumes the PR-build snapshot of WordPress-Utils-Android#156, which stops ImageUtils.getImageOrientation from logging a spurious "Volume data not found" error on every optimized upload staged in the app cache dir (GutenbergKit native media uploads). To be swapped to a tagged release once that PR merges. Co-Authored-By: Claude Opus 4.8 (1M context) --- gradle/libs.versions.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a3615bb1e62c..e19be2a68497 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -103,7 +103,8 @@ wordpress-aztec = 'v2.1.4' wordpress-lint = '2.2.0' wordpress-persistent-edittext = '1.0.2' wordpress-rs = '0.6.0' -wordpress-utils = '3.14.0' +# TODO: Restore to a tagged release once WordPress-Utils-Android#156 merges (PR-build snapshot) +wordpress-utils = '156-2e542df55715dff22c21def058bea551ee9569d0' automattic-ucrop = '2.2.11' zendesk = '5.5.3' turbine = '1.2.1' From b7ad60efb3ff99ff41f1b549738ee45db219f887 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 24 Jul 2026 09:13:35 -0400 Subject: [PATCH 07/27] style: suppress ReturnCount on the processor's decision-table methods processVideo and processImage are guard-clause decision tables where early returns read clearer than nesting; suppress ReturnCount to match the existing house style rather than fragment the logic. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/posts/editor/GBKMediaUploadProcessor.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index a18e54428fdf..e9ee71723b46 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -77,6 +77,7 @@ class GBKMediaUploadProcessor( } } + @Suppress("ReturnCount") private suspend fun processVideo(file: File, filename: String): ProcessedProxyFile { if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, file)) { throw GBKMediaUploadException( @@ -163,6 +164,7 @@ class GBKMediaUploadProcessor( composer.start() } + @Suppress("ReturnCount") private fun processImage(file: File, mimeType: String, filename: String): ProcessedProxyFile { // getOptimizedMedia returns null when optimization is disabled or a no-op. It can also // return the *input* path unchanged (GIF-like skips, decode failures inside From bafe2779118ca1665fd965946dfddb07c2c994a5 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 24 Jul 2026 15:06:15 -0400 Subject: [PATCH 08/27] fix: strip location only from ExifInterface-writable image formats EXIF_MIME_TYPES gated the copy-and-strip branch on formats that can carry GPS, but androidx ExifInterface.saveAttributes() can only rewrite JPEG, PNG, and WebP. The set omitted PNG (so location-bearing PNGs uploaded un-stripped, a regression vs. the legacy pipeline) and included HEIC/HEIF (where saveAttributes throws an IOException that stripLocation swallows, uploading a still-geotagged copy while appearing to honor the setting). Correct the set to {JPEG, PNG, WebP} and cover the PNG-stripped and HEIC-passthrough cases in the decision-table tests. Co-Authored-By: Claude Opus 4.8 --- .../posts/editor/GBKMediaUploadProcessor.kt | 9 ++++-- .../editor/GBKMediaUploadProcessorTest.kt | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index e9ee71723b46..fc2b06e5c573 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -240,8 +240,13 @@ class GBKMediaUploadProcessor( private const val MIME_MP4 = "video/mp4" private const val MIME_OCTET_STREAM = "application/octet-stream" - /** Image formats that can carry EXIF GPS metadata. */ - private val EXIF_MIME_TYPES = setOf(MIME_JPEG, "image/heic", "image/heif", "image/webp") + /** + * Formats androidx ExifInterface can actually strip GPS from: saveAttributes() supports + * only JPEG, PNG, and WebP. HEIC/HEIF are deliberately excluded — the library throws an + * IOException (swallowed by stripLocation), so listing them would make a doomed copy and + * upload a still-geotagged file while appearing to honor the strip-location setting. + */ + private val EXIF_MIME_TYPES = setOf(MIME_JPEG, MIME_PNG, "image/webp") } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 29d3789846cc..e1bcbffb7456 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -119,6 +119,36 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { verify(mediaUtilsWrapper).stripImageLocation(optimized.absolutePath) } + @Test + fun `png gps is stripped onto a copy when strip enabled and optimization off`() = test { + val pngStaged = tempFolder.newFile("art.png") + whenever(mediaUtilsWrapper.getOptimizedMedia(pngStaged.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + + // androidx ExifInterface can write PNG, so PNG must take the copy-and-strip branch. + val result = createProcessor(wpComSite()).processFile(pngStaged, "image/png", "art.png") + + assertThat(result).isInstanceOf(ProcessedProxyFile.Processed::class.java) + result as ProcessedProxyFile.Processed + assertThat(result.file.absolutePath).isNotEqualTo(pngStaged.absolutePath) + verify(mediaUtilsWrapper).stripImageLocation(result.file.absolutePath) + result.file.delete() + } + + @Test + fun `heic passes through when strip enabled and optimization off`() = test { + val heicStaged = tempFolder.newFile("photo.heic") + whenever(mediaUtilsWrapper.getOptimizedMedia(heicStaged.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + + // androidx ExifInterface cannot write HEIF, so copy-and-strip would silently fail and + // upload a still-geotagged copy — HEIC must not take the strip branch. + val result = createProcessor(wpComSite()).processFile(heicStaged, "image/heic", "photo.heic") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).stripImageLocation(any()) + } + @Test fun `heic reports jpeg mime type and extension after optimization`() = test { val heicStaged = tempFolder.newFile("photo.heic") From 149d909a63d68c184cd85cc709e2c7a50f606883 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 24 Jul 2026 15:17:27 -0400 Subject: [PATCH 09/27] style: drop stale SwallowedException suppression on transcodeVideo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit transcodeVideo logs each caught exception via .message, so Detekt's SwallowedException never fires — only TooGenericExceptionCaught does (NullPointerException and Exception are both on its generic-names list). Trim the suppression to the rule that actually triggers. Co-Authored-By: Claude Opus 4.8 --- .../android/ui/posts/editor/GBKMediaUploadProcessor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index fc2b06e5c573..27810da7a9dc 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -108,7 +108,7 @@ class GBKMediaUploadProcessor( * [org.wordpress.android.ui.uploads.VideoOptimizer] semantics: any failure (no composer, * m4m error) resolves to null so the caller falls back to uploading the original. */ - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") private suspend fun transcodeVideo(input: File): File? = suspendCancellableCoroutine { continuation -> val output = File(appContext.cacheDir, MediaUtils.generateTimeStampedFileName(MIME_MP4)) val listener = object : IProgressListener { From b486aa79c443de6b5efbe30a698adf50d8c1662c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 24 Jul 2026 15:49:16 -0400 Subject: [PATCH 10/27] docs: clarify the processFile plan check is a cache-aware fallback GutenbergKit validates uploads in the WebView against the site's allowedMimeTypes (fetched from /wp-block-editor/v1/settings and cached on disk) before the request reaches this delegate, so it rejects most disallowed types with its own localized message first. Document that this app-side check is a fallback for the cache-miss/undefined-settings path and can over-reject when its static MimeTypes table is stricter than the server's cached list. Co-Authored-By: Claude Opus 4.8 --- .../android/ui/posts/editor/GBKMediaUploadProcessor.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 27810da7a9dc..295e852c3780 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -61,8 +61,14 @@ class GBKMediaUploadProcessor( ): ProcessedProxyFile = withContext(ioDispatcher) { val resolvedMimeType = resolveMimeType(mimeType, filename) - // Reject types the site's plan doesn't allow (e.g. audio on free WP.com plans) with a - // localized message instead of an opaque server-side error. + // Fallback plan check. GutenbergKit's editor validates uploads in the WebView against the + // site's allowedMimeTypes (from /wp-block-editor/v1/settings) before the request reaches + // this delegate, so for most disallowed types the editor rejects with its own localized + // message first. Those settings are cached on disk and reused on later opens, so GB + // validates against whatever mime list was cached — not necessarily the site's current + // one. This check still fires when GB's list is absent (cache miss where editor settings + // resolve to undefined) or when the app's static MimeTypes table is stricter than the + // server's list — in which case it can over-reject a type the server would accept. if (!mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType)) { throw GBKMediaUploadException(appContext.getString(R.string.error_media_file_type_not_allowed)) } From f6494776e9f3dde81383dfa5bda538b40f5631f7 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 09:47:15 -0400 Subject: [PATCH 11/27] build: bump GutenbergKit to the trunk media-upload snapshot PR 357 merged as 016a00e5, and the review follow-up 561 ("harden the native media upload server") landed on top of it. Track trunk at 16aceb17 instead of the transient PR branch build, picking up 561's fixes: idle-based body reads so a steadily-streamed large upload is no longer failed on total duration, rejection of auth-exempt OPTIONS carrying a body, and cancellable reads so shutdown reaps an active connection. Still an untagged snapshot, so the TODO stays. v0.19.0 is tagged but predates both 357 and 561. Co-Authored-By: Claude Opus 5 (1M context) --- gradle/libs.versions.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e19be2a68497..c62bffac27d3 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -73,8 +73,8 @@ google-play-review = '2.0.2' google-services = '4.5.0' gravatar = '2.5.0' greenrobot-eventbus = '3.3.1' -# TODO: Swap to a tagged GutenbergKit release before merge (snapshot of wordpress-mobile/GutenbergKit#357) -gutenberg-kit = '357-45219181e3aa60b5ce3119367758d90e6b6beb0b' +# TODO: Swap to a tagged GutenbergKit release before merge (snapshot of GutenbergKit trunk) +gutenberg-kit = 'trunk-16aceb17716dbef9fb2289d316e27dc812a569f6' gutenberg-mobile = 'v1.121.0' indexos-media-for-mobile = '43a9026f0973a2f0a74fa813132f6a16f7499c3a' jackson-databind = '2.12.7.1' From 5e3ad0f4e76a0b8cbc7ae2d9fe37442d9977a482 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 09:47:25 -0400 Subject: [PATCH 12/27] fix: set the media upload delegate before the editor loads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GutenbergKit 561 made mediaUploadDelegate throw when assigned after the editor page starts loading: the delegate is captured once, in onPageStarted, and the setter now check()s !hasStartedLoading. The old setter started the upload server on assignment and re-synced window.GBKit into a loaded page, so a late set used to work. setMediaUploadDelegate pushed straight into a possibly-loaded view, which would now crash. Store the delegate only and let onCreateView be the single place it reaches the view, moved to immediately after the GutenbergView constructor — on the preloaded-dependencies fast path that constructor already kicks off the load. The ordering holds: FragmentPagerAdapter.instantiateItem defers its transaction to finishUpdate, so the activity's setMediaUploadDelegate call returns before onCreateView runs. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GutenbergKitEditorFragment.kt | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt index 393f080ea756..94cc5f64f630 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt @@ -192,6 +192,14 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { context = requireContext() ) + // Must be set before the editor loads: GutenbergKit captures the delegate when the page + // begins loading and throws from the setter afterward. The constructor already kicks off + // the load when dependencies are preloaded, so assign it here rather than alongside the + // listeners below. + mediaUploadDelegate?.let { + gutenbergView.mediaUploadDelegate = it + } + gutenbergViewContainer.addView( gutenbergView, FrameLayout.LayoutParams( @@ -226,9 +234,6 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { networkRequestListener?.let( gutenbergView::setNetworkRequestListener ) - mediaUploadDelegate?.let { - gutenbergView.mediaUploadDelegate = it - } // Set up content provider for WebView refresh recovery gutenbergView.setLatestContentProvider( @@ -557,9 +562,14 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { gutenbergView?.setNetworkRequestListener(listener) } + /** + * Sets the delegate that processes media before upload. Must be called before [onCreateView], + * which is the only place the delegate reaches the view: GutenbergKit captures it when the + * editor page begins loading and throws from its setter afterward, so pushing it into a live + * view here would crash. The field is the single source of truth. + */ fun setMediaUploadDelegate(delegate: MediaUploadDelegate) { mediaUploadDelegate = delegate - gutenbergView?.mediaUploadDelegate = delegate } override fun onUndoPressed() { From 2a9c5836d62e9c7aec5325e0c6c794f30ff9914a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 09:47:36 -0400 Subject: [PATCH 13/27] perf: skip the upload temp copy for files we pass through GutenbergKit 561 added handlesFile, a metadata-only gate consulted before the upload server materializes a request to a temp file. Declining relays the original body straight to WordPress. processFile returns Original for GIFs and non-media, so today those pay a full byte-for-byte copy only to reach the same passthrough. Decline them up front and skip the copy. Disallowed types are claimed rather than declined, so processFile still runs and throws the localized rejection. Declining would relay them to WordPress instead, spending a whole upload on a file the site's plan won't accept and replacing our message with the server's untranslated one. That matters because Gutenberg's own validateMimeTypeForUser early-returns when the cached allowedMimeTypes list is absent, leaving this check as the only validation on a cold cache. handlesFile is an optimization hint, not the enforcement point: it sees only the client-supplied mime type and filename, which can disagree with the file's bytes, so the plan check inside processFile stays authoritative. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 295e852c3780..d2954b176807 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -54,6 +54,36 @@ class GBKMediaUploadProcessor( */ private val transcodeMutex = Mutex() + /** + * Metadata-only gate GutenbergKit consults before copying an upload to a temp file. Declining + * makes it relay the original request body straight to WordPress, skipping a copy this + * delegate would not have used: [processFile] returns [ProcessedProxyFile.Original] for GIFs + * and non-media, so today those pay a full byte-for-byte copy only to be passed through. + * + * This is an optimization hint, never the enforcement point. It sees only the client-supplied + * mime type and filename, which can disagree with the file's actual bytes, so the plan check + * inside [processFile] stays authoritative — the copy here is a fast path, not a replacement. + */ + @Suppress("ReturnCount") + override fun handlesFile(mimeType: String, filename: String): Boolean { + val resolvedMimeType = resolveMimeType(mimeType, filename) + + // Claim disallowed types so processFile still runs and throws the localized rejection. + // Declining would forward them to WordPress instead, spending a full upload on a file the + // site's plan won't accept and surfacing the server's untranslated error in place of ours. + if (!mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType)) return true + + return when { + // Never re-encoded; processFile always returns Original. + resolvedMimeType == MIME_GIF -> false + // Both the duration check and the optional transcode need the file itself. + mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> true + resolvedMimeType.startsWith(MIME_IMAGE_PREFIX) -> true + // Non-media files (documents, archives, audio on paid plans) upload unchanged. + else -> false + } + } + override suspend fun processFile( file: File, mimeType: String, From d6db8eb64963fb28008db6fe4f41f5dc53c8b6da Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 09:47:44 -0400 Subject: [PATCH 14/27] test: cover the handlesFile decision table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the existing processFile cases: GIF and non-media are declined so the temp copy is skipped, images and videos are claimed. The disallowed-type case pairs with "disallowed file type throws with localized message", which uses the same mime type — that test would stop reflecting reality if handlesFile ever declined non-media unconditionally, since processFile would no longer run for it. Co-Authored-By: Claude Opus 5 (1M context) --- .../editor/GBKMediaUploadProcessorTest.kt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index e1bcbffb7456..3072b7354230 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -269,6 +269,38 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { assertThat(result).isEqualTo(ProcessedProxyFile.Original) } + @Test + fun `handlesFile declines gif so the copy is skipped`() { + assertThat(createProcessor().handlesFile("image/gif", "anim.gif")).isFalse() + } + + @Test + fun `handlesFile declines non-media so the copy is skipped`() { + assertThat(createProcessor().handlesFile("application/pdf", "doc.pdf")).isFalse() + } + + @Test + fun `handlesFile claims images`() { + assertThat(createProcessor().handlesFile("image/jpeg", "photo.jpg")).isTrue() + } + + @Test + fun `handlesFile claims videos`() { + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + + assertThat(createProcessor().handlesFile("video/mp4", "movie.mp4")).isTrue() + } + + @Test + fun `handlesFile claims disallowed types so processFile can reject them locally`() { + // Declining would relay the file to WordPress instead, wasting a full upload and replacing + // our localized message with the server's. Pairs with the processFile rejection test above, + // which uses the same mime type. + whenever(mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(anyOrNull(), any())).thenReturn(false) + + assertThat(createProcessor().handlesFile("application/zip", "archive.zip")).isTrue() + } + private fun fileUri(file: File): android.net.Uri = mock { on { path } doReturn file.absolutePath } From 43334af6dcec3c96c279a67eee64ba3b55d3c227 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 12:06:19 -0400 Subject: [PATCH 15/27] fix: normalize the upload mime type before the plan check isMimeTypeSupportedBySitePlan is an exact, case-sensitive match against a closed allowlist, but resolveMimeType passed the multipart part's raw Content-Type header through untouched. Two shapes that header legitimately takes were rejected as disallowed file types: - Parameters and casing. "image/jpeg; charset=binary" and "IMAGE/JPEG" miss the allowlist, so a valid photo failed with the localized "file type is not allowed" error instead of uploading. - A missing header. GutenbergKit's multipart parser defaults a part with no Content-Type to "text/plain" (RFC 7578) and selects the file part by its filename parameter rather than its type, so a real image can arrive labeled text/plain and was rejected outright. Strip parameters, lowercase, and treat text/plain as a placeholder alongside application/octet-stream so it falls back to the filename extension. Also guard the extension lookup: MimeTypeMap.getSingleton() is @NonNull on device but null under the unit test stubs, and widening the fallback to text/plain widened that latent NPE onto the common no-Content-Type path. An unresolvable lookup now degrades to the declared type rather than an empty string, which the plan check cannot reason about. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 34 +++++++++++- .../editor/GBKMediaUploadProcessorTest.kt | 52 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index d2954b176807..6f6cb98e8220 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -262,10 +262,31 @@ class GBKMediaUploadProcessor( } } + /** + * Normalizes the client-supplied mime type, falling back to the filename extension when it + * carries no usable information. + * + * The result feeds [MediaUtilsWrapper.isMimeTypeSupportedBySitePlan], which is an exact, + * case-sensitive match against a closed allowlist, so anything but a bare lowercase + * `type/subtype` is rejected outright. Two shapes reach us that the allowlist would miss: + * - Parameters and casing: `Content-Type` may legitimately carry parameters + * (`image/jpeg; charset=binary`) and its casing is not significant (RFC 9110 §8.3). + * - Missing header: GutenbergKit's multipart parser defaults a part with no `Content-Type` + * to `text/plain` (RFC 7578 §4.4), and it picks the file part by the presence of a + * `filename` parameter, not by content type — so a real image can arrive labeled + * `text/plain`. Treat that like the other placeholders and fall back to the extension. + */ private fun resolveMimeType(mimeType: String, filename: String): String { - if (mimeType.isNotBlank() && mimeType != MIME_OCTET_STREAM) return mimeType + val normalized = mimeType.substringBefore(';').trim().lowercase() + if (normalized.isNotBlank() && normalized !in PLACEHOLDER_MIME_TYPES) return normalized + val extension = filename.substringAfterLast('.', "").lowercase() - return MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension) ?: MIME_OCTET_STREAM + // Fall back to the declared type when the extension resolves to nothing: it is a + // placeholder, but a placeholder the plan check can still reject coherently, whereas an + // empty string is neither. getSingleton() is @NonNull on device but null under the unit + // test stubs, so it is treated as an unresolvable lookup rather than dereferenced. + val fromExtension = MimeTypeMap.getSingleton()?.getMimeTypeFromExtension(extension) + return fromExtension ?: normalized.ifBlank { MIME_OCTET_STREAM } } companion object { @@ -275,6 +296,15 @@ class GBKMediaUploadProcessor( private const val MIME_JPEG = "image/jpeg" private const val MIME_MP4 = "video/mp4" private const val MIME_OCTET_STREAM = "application/octet-stream" + private const val MIME_TEXT_PLAIN = "text/plain" + + /** + * Mime types that carry no usable type information for an upload, so [resolveMimeType] + * prefers the filename extension over them. `application/octet-stream` is the generic + * "unknown bytes" type; `text/plain` is the multipart default for a part that sent no + * `Content-Type` header at all. + */ + private val PLACEHOLDER_MIME_TYPES = setOf(MIME_OCTET_STREAM, MIME_TEXT_PLAIN) /** * Formats androidx ExifInterface can actually strip GPS from: saveAttributes() supports diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 3072b7354230..46c1a9b120d9 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -12,6 +12,7 @@ import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.doReturn +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -291,6 +292,57 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { assertThat(createProcessor().handlesFile("video/mp4", "movie.mp4")).isTrue() } + @Test + fun `mime type parameters are stripped before the plan check`() = test { + // Content-Type may legitimately carry parameters. isMimeTypeSupportedBySitePlan is an + // exact match against a closed allowlist, so an unnormalized value hard-fails a valid + // image. + val optimized = tempFolder.newFile("optimized.jpg") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(stagedFile, "image/jpeg; charset=binary", "photo.jpg") + + verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("image/jpeg")) + result as ProcessedProxyFile.Processed + assertThat(result.mimeType).isEqualTo("image/jpeg") + } + + @Test + fun `mime type casing is normalized before the plan check`() = test { + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(false) + + val result = createProcessor(wpComSite()).processFile(stagedFile, "IMAGE/JPEG", "photo.jpg") + + verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("image/jpeg")) + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + } + + @Test + fun `text plain is treated as a placeholder and resolved from the filename`() { + // GutenbergKit's multipart parser defaults a part with no Content-Type to text/plain + // (RFC 7578), and picks the file part by its filename parameter rather than its type — so + // a real image can arrive labeled text/plain and must not be rejected as a disallowed type. + // + // MimeTypeMap is a stub returning null under unit tests, so the extension lookup cannot + // resolve here; this asserts the surrounding contract instead — text/plain is not taken at + // face value, and an unresolvable lookup degrades to the declared type rather than "". + createProcessor().handlesFile("text/plain", "photo.jpg") + + verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("text/plain")) + } + + @Test + fun `blank mime type never resolves to an empty string`() { + // An empty resolved type would be meaningless to the plan check; a placeholder it can + // reject coherently is the safe floor. + createProcessor().handlesFile("", "mystery") + + verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("application/octet-stream")) + } + @Test fun `handlesFile claims disallowed types so processFile can reject them locally`() { // Declining would relay the file to WordPress instead, wasting a full upload and replacing From 2480d4319b1de975d16b670dd28016bed60ced76 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 12:08:15 -0400 Subject: [PATCH 16/27] fix: fall back to the original video when the transcode cannot start transcodeVideo documents that any failure resolves to null so the caller uploads the original, but composer.start() was unguarded. m4m throws IllegalStateException when it cannot set up the codec (codec unavailable, memory pressure, unsupported track config), and that exception escaped the coroutine through processFile into GutenbergKit's catch-all, which relayed it to the editor as a 500 carrying the raw, untranslated m4m message. The upload was lost where the legacy VideoOptimizer would have succeeded with the original file. Guard it exactly as VideoOptimizer.start() does. Also make the transcode mutex process-wide and the output filename collision proof, which are the same defect seen from two sides: - A new processor is constructed for every editor fragment and GutenbergKitActivity declares no launchMode, so instances stack. The per-instance mutex therefore did not serialize anything across editors, which is what its own KDoc says it exists to do. - generateTimeStampedFileName is only "wp-{currentTimeMillis}.mp4", so two transcodes starting in the same millisecond shared an output path. createTempFile takes uniqueness from the filesystem with no check-then-create race, matching the collision-avoiding naming used by both MediaUtils.getUniqueCacheFileForName and the iOS exporter. The transcode path depends on the static WPVideoUtils.getVideoOptimizationComposer and real m4m codecs, so it has no unit coverage here, as before. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 6f6cb98e8220..c1a8ee8e8705 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -13,7 +13,6 @@ import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.AppLog -import org.wordpress.android.util.MediaUtils import org.wordpress.android.util.MediaUtilsWrapper import org.wordpress.android.util.WPVideoUtils import org.wordpress.gutenberg.MediaUploadDelegate @@ -47,13 +46,6 @@ class GBKMediaUploadProcessor( private val appPrefsWrapper: AppPrefsWrapper, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : MediaUploadDelegate { - /** - * Serializes video transcodes. GutenbergKit's upload server handles requests concurrently, - * but parallel m4m hardware transcodes are memory/codec-heavy; the legacy pipeline - * effectively serialized them through the upload queue. - */ - private val transcodeMutex = Mutex() - /** * Metadata-only gate GutenbergKit consults before copying an upload to a temp file. Declining * makes it relay the original request body straight to WordPress, skipping a copy this @@ -146,7 +138,10 @@ class GBKMediaUploadProcessor( */ @Suppress("TooGenericExceptionCaught") private suspend fun transcodeVideo(input: File): File? = suspendCancellableCoroutine { continuation -> - val output = File(appContext.cacheDir, MediaUtils.generateTimeStampedFileName(MIME_MP4)) + // createTempFile rather than MediaUtils.generateTimeStampedFileName, which is only + // "wp-{currentTimeMillis}.mp4" and collides for two transcodes started in the same + // millisecond. Uniqueness comes from the filesystem, with no check-then-create race. + val output = File.createTempFile("wp-", ".mp4", appContext.cacheDir) val listener = object : IProgressListener { override fun onMediaStart() = Unit override fun onMediaProgress(progress: Float) = Unit @@ -197,7 +192,18 @@ class GBKMediaUploadProcessor( output.delete() } - composer.start() + // m4m throws IllegalStateException from start() when it cannot set up the codec (codec + // unavailable, memory pressure, unsupported track config). Without this guard the + // exception escapes the coroutine and GutenbergKit's catch-all turns it into a 500 whose + // raw, untranslated m4m message is shown to the user — losing an upload the legacy + // pipeline would have completed with the original file. Guard it as VideoOptimizer does. + try { + composer.start() + } catch (e: IllegalStateException) { + AppLog.e(AppLog.T.MEDIA, "GBKMediaUploadProcessor > failed to start composer", e) + output.delete() + if (continuation.isActive) continuation.resume(null) + } } @Suppress("ReturnCount") @@ -290,6 +296,18 @@ class GBKMediaUploadProcessor( } companion object { + /** + * Serializes video transcodes. GutenbergKit's upload server handles requests concurrently, + * but parallel m4m hardware transcodes are memory/codec-heavy; the legacy pipeline + * effectively serialized them through the upload queue. + * + * Process-wide rather than per-instance: a new processor is constructed for every editor + * fragment (see GutenbergKitActivity's SectionsPagerAdapter), and GutenbergKitActivity has + * no launchMode, so instances stack. A per-instance mutex would let two editors transcode + * in parallel — exactly what this exists to prevent. + */ + private val transcodeMutex = Mutex() + private const val MIME_IMAGE_PREFIX = "image/" private const val MIME_GIF = "image/gif" private const val MIME_PNG = "image/png" From a3aa9ea0e673979e5efb893552ef6266dfc604be Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 17 Aug 2026 12:27:34 -0400 Subject: [PATCH 17/27] fix: label a processed image from its encoded output, not its declared type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit processedImage chose the reported mime type and extension by testing the declared *input* mime against image/png, but ImageUtils chooses its encoder from the extension it derives for the *output*: resizeImageAndWriteToStream writes PNG only when that extension is literally "png", and everything else — HEIC and WebP included — becomes JPEG. Those two decisions can disagree. optimizeImage derives its extension via MediaUtils.getMediaFileName, which supplies one sniffed from the file's bytes whenever the name carries no extension of its own. An extensionless upload whose declared Content-Type disagrees with its content therefore produced JPEG bytes reported as image/png (or PNG bytes reported as image/jpeg), and WordPress stores that mislabel on the attachment permanently. Both the optimization and the rotation branch were affected, and image optimization is on by default. Read the format off the output file instead. ImageUtils names the output with the same extension it used to select the encoder, so the written filename is a faithful record of the encode decision, which the declared input mime is not. This observes what the encoder did rather than predicting it — the same single-source-of-truth property that makes the iOS processor robust, where the mime type is likewise derived from the exported file. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 24 +++++++++---- .../editor/GBKMediaUploadProcessorTest.kt | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index c1a8ee8e8705..f5117ead7be0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -217,7 +217,7 @@ class GBKMediaUploadProcessor( ?.takeIf { it != file.absolutePath } if (optimizedPath != null) { - return processedImage(File(optimizedPath), mimeType, filename) + return processedImage(File(optimizedPath), filename) } // With optimization off, WP.com rotates sideways-captured images server-side but @@ -228,7 +228,7 @@ class GBKMediaUploadProcessor( ?.path ?.takeIf { it != file.absolutePath } if (rotatedPath != null) { - return processedImage(File(rotatedPath), mimeType, filename) + return processedImage(File(rotatedPath), filename) } } @@ -250,10 +250,19 @@ class GBKMediaUploadProcessor( /** * Wraps an optimized/rotated image file, stripping GPS EXIF when enabled and correcting the * reported mime type and filename: ImageUtils re-encodes PNG to PNG and everything else - * (including HEIC/WebP) to JPEG bytes while keeping the original file extension, so the - * metadata sent to WordPress must reflect the actual output format. + * (including HEIC/WebP) to JPEG bytes, so the metadata sent to WordPress must reflect the + * actual output format. + * + * The format is read off the *output* file rather than predicted from the declared input mime + * type. ImageUtils picks its encoder from the extension it derives for the output + * (`resizeImageAndWriteToStream` writes PNG only when that extension is literally "png") and + * names the output file with the same extension, so the written name is a faithful record of + * the encode decision. The declared input mime is not: for an extensionless upload + * `MediaUtils.getMediaFileName` supplies an extension sniffed from the bytes, which can + * disagree with what the client declared. Labeling from the input therefore produced JPEG + * bytes tagged `image/png` (and the reverse) — a mislabel WordPress then stores permanently. */ - private fun processedImage(output: File, inputMimeType: String, filename: String): ProcessedProxyFile { + private fun processedImage(output: File, filename: String): ProcessedProxyFile { if (appPrefsWrapper.isStripImageLocation) { // getOptimizedMedia copies the original's EXIF (including GPS) onto its output, so // the strip must run on the output — matching the legacy strip-at-upload behavior. @@ -261,7 +270,7 @@ class GBKMediaUploadProcessor( } val basename = filename.substringBeforeLast('.') - return if (inputMimeType == MIME_PNG) { + return if (output.extension.lowercase() == EXTENSION_PNG) { ProcessedProxyFile.Processed(output, MIME_PNG, "$basename.png") } else { ProcessedProxyFile.Processed(output, MIME_JPEG, "$basename.jpg") @@ -316,6 +325,9 @@ class GBKMediaUploadProcessor( private const val MIME_OCTET_STREAM = "application/octet-stream" private const val MIME_TEXT_PLAIN = "text/plain" + /** The one extension ImageUtils treats as "encode as PNG"; everything else becomes JPEG. */ + private const val EXTENSION_PNG = "png" + /** * Mime types that carry no usable type information for an upload, so [resolveMimeType] * prefers the filename extension over them. `application/octet-stream` is the generic diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 46c1a9b120d9..70a28eb85887 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -181,6 +181,40 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { assertThat(result.filename).isEqualTo("art.png") } + @Test + fun `output format is read from the encoded file, not the declared mime type`() = test { + // ImageUtils picks its encoder from the extension it derives for the output, which for an + // extensionless upload comes from the file's sniffed bytes rather than the declared mime. + // Labeling from the declared type would tag these PNG bytes as image/jpeg, and WordPress + // would store that mislabel permanently. + val extensionless = tempFolder.newFile("screenshot") + val optimized = tempFolder.newFile("optimized-sniffed.png") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(extensionless.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(extensionless, "image/jpeg", "screenshot") + + result as ProcessedProxyFile.Processed + assertThat(result.mimeType).isEqualTo("image/png") + assertThat(result.filename).isEqualTo("screenshot.png") + } + + @Test + fun `jpeg output is labeled jpeg even when the input declared png`() = test { + val staged = tempFolder.newFile("mystery") + val optimized = tempFolder.newFile("optimized-sniffed.jpg") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(staged.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(staged, "image/png", "mystery") + + result as ProcessedProxyFile.Processed + assertThat(result.mimeType).isEqualTo("image/jpeg") + assertThat(result.filename).isEqualTo("mystery.jpg") + } + @Test fun `gif passes through untouched`() = test { val gifStaged = tempFolder.newFile("anim.gif") From bce749e1c982b32eb7f0d15d85234d307df5096d Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 18 Aug 2026 10:09:51 -0400 Subject: [PATCH 18/27] fix: key the video duration limit off the resolved mime type processVideo is entered when isVideoMimeType(resolvedMimeType) is true, but the duration guard re-derived videoness from the staged file. Wrapped in Uri.fromFile, contentResolver.getType() returns null (it only types content URIs), so isVideoFile collapsed to MediaUtils.isVideo -- an extension-only test over nine suffixes. GutenbergKit names the staged copy "{uuid}-{filename}" from the client-supplied filename and synthesizes no extension, and that name can legitimately lack one: Chromium takes File.name from OpenableColumns.DISPLAY_NAME on the content:// path, and a filename="" parameter reaches the delegate as an empty string. In those cases a declared video/mp4 skipped the check entirely, letting a free-plan site upload a video the legacy pipeline would have rejected. Split the shared implementation so videoness is supplied by the caller. The File overload passes isVideoMimeType(mimeType); the Uri overload keeps isVideoFile(uri), leaving AddLocalMediaToPostUseCase and MediaBrowserActivity unchanged -- they pass content URIs, where the resolver types correctly, which is why the legacy editor never had this gap. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 9 ++++--- .../android/util/MediaUtilsWrapper.kt | 27 +++++++++++++++---- .../editor/GBKMediaUploadProcessorTest.kt | 27 +++++++++++++++++-- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index f5117ead7be0..f7dd21b06f68 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -98,7 +98,7 @@ class GBKMediaUploadProcessor( when { // Never re-encode GIFs — it would flatten animation. Passthrough skips even a copy. resolvedMimeType == MIME_GIF -> ProcessedProxyFile.Original - mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> processVideo(file, filename) + mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> processVideo(file, resolvedMimeType, filename) resolvedMimeType.startsWith(MIME_IMAGE_PREFIX) -> processImage(file, resolvedMimeType, filename) // Non-media files (documents, archives, audio on paid plans) upload unchanged. else -> ProcessedProxyFile.Original @@ -106,8 +106,11 @@ class GBKMediaUploadProcessor( } @Suppress("ReturnCount") - private suspend fun processVideo(file: File, filename: String): ProcessedProxyFile { - if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, file)) { + private suspend fun processVideo(file: File, mimeType: String, filename: String): ProcessedProxyFile { + // Pass the resolved mime type rather than letting the check re-derive "is this a video" + // from the staged file: that path is an extension-only test, and GutenbergKit names the + // staged copy after the client-supplied filename, which need not carry one. + if (mediaUtilsWrapper.isProhibitedVideoDuration(appContext, site, file, mimeType)) { throw GBKMediaUploadException( appContext.getString(R.string.error_media_video_duration_exceeds_limit) ) diff --git a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt index 6c31562b887d..20af968049fa 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt @@ -64,11 +64,28 @@ class MediaUtilsWrapper @Inject constructor(private val appContext: Context) { fun isVideoFile(mediaUri: Uri): Boolean = isVideo(mediaUri) || isVideoMimeType(getMimeType(mediaUri)) - fun isProhibitedVideoDuration(context: Context, site: SiteModel, file: File): Boolean = - isProhibitedVideoDuration(context, site, Uri.fromFile(file)) - - fun isProhibitedVideoDuration(context: Context, site: SiteModel, uri: Uri): Boolean { - if (isVideoFile(uri) && site.hasFreePlan && !site.isActiveModuleEnabled("videopress")) { + /** + * Duration check for a file whose type is already known from its upload metadata. + * + * Callers must pass the mime type they resolved, because the [Uri] overload cannot recover it + * here: a `file://` URI makes [getMimeType] return null (ContentResolver only types content + * URIs), collapsing [isVideoFile] to [MediaUtils.isVideo] — an extension-only test. A video + * whose filename carries no recognized extension would then skip the check entirely and let a + * free site upload a video over the limit. + */ + fun isProhibitedVideoDuration(context: Context, site: SiteModel, file: File, mimeType: String): Boolean = + isProhibitedVideoDuration(context, site, Uri.fromFile(file), isVideoMimeType(mimeType)) + + fun isProhibitedVideoDuration(context: Context, site: SiteModel, uri: Uri): Boolean = + isProhibitedVideoDuration(context, site, uri, isVideoFile(uri)) + + private fun isProhibitedVideoDuration( + context: Context, + site: SiteModel, + uri: Uri, + isVideo: Boolean + ): Boolean { + if (isVideo && site.hasFreePlan && !site.isActiveModuleEnabled("videopress")) { val retriever = MediaMetadataRetriever() try { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 70a28eb85887..38b1b7bc7aad 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -242,7 +242,8 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { @Test fun `video exceeding duration limit throws with localized message`() = test { whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) - whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any())).thenReturn(true) + whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any(), any())) + .thenReturn(true) val videoStaged = tempFolder.newFile("movie.mp4") val thrown = runCatching { @@ -257,7 +258,8 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { @Test fun `video passes through when optimization disabled`() = test { whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) - whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any())).thenReturn(false) + whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any(), any())) + .thenReturn(false) whenever(appPrefsWrapper.isVideoOptimize).thenReturn(false) val videoStaged = tempFolder.newFile("movie.mp4") @@ -266,6 +268,27 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { assertThat(result).isEqualTo(ProcessedProxyFile.Original) } + @Test + fun `duration check receives the resolved mime type, not the staged path`() = test { + // The staged file is named after the client-supplied filename, which need not carry an + // extension. Deriving "is this a video" from that path is an extension-only test, so the + // duration limit must be keyed off the resolved mime type instead. + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + whenever(mediaUtilsWrapper.isProhibitedVideoDuration(any(), any(), any(), any())) + .thenReturn(false) + whenever(appPrefsWrapper.isVideoOptimize).thenReturn(false) + val extensionlessVideo = tempFolder.newFile("upload") + + createProcessor().processFile(extensionlessVideo, "video/mp4", "upload") + + verify(mediaUtilsWrapper).isProhibitedVideoDuration( + any(), + any(), + eq(extensionlessVideo), + eq("video/mp4") + ) + } + @Test fun `optimization returning the input path is treated as not optimized`() = test { // ImageUtils.optimizeImage returns the original path for skips/failures; wrapping it in From fb5f49274221c7b9fd4ec1ea3175edab6157089a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 18 Aug 2026 10:09:58 -0400 Subject: [PATCH 19/27] build: pin GutenbergKit to the v0.20.0-alpha.0 release Replaces the trunk snapshot with the tagged release and drops the TODO. The pinned commit 16aceb17 is an ancestor of the tag, and the only difference across the android/ module between them is the GutenbergKitVersion.kt string bump, so this is a no-op for behavior. Co-Authored-By: Claude Opus 5 (1M context) --- gradle/libs.versions.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c62bffac27d3..329649acb436 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -73,8 +73,7 @@ google-play-review = '2.0.2' google-services = '4.5.0' gravatar = '2.5.0' greenrobot-eventbus = '3.3.1' -# TODO: Swap to a tagged GutenbergKit release before merge (snapshot of GutenbergKit trunk) -gutenberg-kit = 'trunk-16aceb17716dbef9fb2289d316e27dc812a569f6' +gutenberg-kit = 'v0.20.0-alpha.0' gutenberg-mobile = 'v1.121.0' indexos-media-for-mobile = '43a9026f0973a2f0a74fa813132f6a16f7499c3a' jackson-databind = '2.12.7.1' From 383be177e0c564531ef546e2bb4c11e1cc74f532 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 12:04:04 -0400 Subject: [PATCH 20/27] fix: scope the upload plan check to free WordPress.com plans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delegate rejected any upload missing from the app's static MimeTypes table. That table is hand-maintained and has drifted from what WordPress accepts: it has no image/avif (core-supported since 6.5), no image/svg+xml, and no text types, and it maps self-hosted sites to the same document set as WP.com paid. Uploads that worked before this feature — a .txt or .csv on self-hosted, an AVIF anywhere, an SVG with the plugin enabled — were rejected with "This file type is not allowed". The check could only ever produce false rejections. GutenbergKit already validates uploads in the WebView against the site's real allowedMimeTypes from /wp-block-editor/v1/settings, so anything reaching processFile has passed the authoritative check. Narrow it to the one case the app can judge better than the server: audio and documents on a free WordPress.com plan, where the restriction is a plan entitlement and a localized message beats the server's untranslated error. Images and videos are left to GutenbergKit and the server. application/octet- stream is excluded because resolveMimeType emits it for unidentifiable uploads, and "we could not identify this file" should not be answered with "this file type is not allowed". The free-plan test uses SiteUtils.onFreePlan, matching what WPMediaUtils.getSitePlanForMimeTypes uses to select WP_COM_FREE, so the gate cannot disagree with the allowlist it guards. Co-Authored-By: Claude Opus 5 (1M context) --- .../android/ui/posts/GutenbergKitActivity.kt | 3 + .../posts/editor/GBKMediaUploadProcessor.kt | 60 ++++++--- .../android/util/MediaUtilsWrapper.kt | 6 + .../editor/GBKMediaUploadProcessorTest.kt | 117 ++++++++++++++---- 4 files changed, 148 insertions(+), 38 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt index fd85f47a9e52..401f46e767d7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt @@ -196,6 +196,7 @@ import org.wordpress.android.util.DisplayUtils import org.wordpress.android.util.FluxCUtils import org.wordpress.android.util.MediaUtils import org.wordpress.android.util.MediaUtilsWrapper +import org.wordpress.android.util.SiteUtilsWrapper import org.wordpress.android.util.NetworkUtils import org.wordpress.android.util.ReblogUtils import org.wordpress.android.util.ShortcutUtils @@ -391,6 +392,7 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene @Inject lateinit var gutenbergKitNetworkLogger: GutenbergKitNetworkLogger @Inject lateinit var gutenbergKitSettingsBuilder: GutenbergKitSettingsBuilder @Inject lateinit var mediaUtilsWrapper: MediaUtilsWrapper + @Inject lateinit var siteUtilsWrapper: SiteUtilsWrapper @Inject lateinit var appPrefsWrapper: AppPrefsWrapper private lateinit var editPostNavigationViewModel: EditPostNavigationViewModel private lateinit var editPostSettingsViewModel: EditPostSettingsViewModel @@ -2273,6 +2275,7 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene appContext = applicationContext, mediaUtilsWrapper = mediaUtilsWrapper, appPrefsWrapper = appPrefsWrapper, + siteUtilsWrapper = siteUtilsWrapper, ) ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index f7dd21b06f68..fa2f26320330 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -14,6 +14,7 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.AppLog import org.wordpress.android.util.MediaUtilsWrapper +import org.wordpress.android.util.SiteUtilsWrapper import org.wordpress.android.util.WPVideoUtils import org.wordpress.gutenberg.MediaUploadDelegate import org.wordpress.gutenberg.ProcessedProxyFile @@ -44,6 +45,7 @@ class GBKMediaUploadProcessor( private val appContext: Context, private val mediaUtilsWrapper: MediaUtilsWrapper, private val appPrefsWrapper: AppPrefsWrapper, + private val siteUtilsWrapper: SiteUtilsWrapper, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : MediaUploadDelegate { /** @@ -53,17 +55,18 @@ class GBKMediaUploadProcessor( * and non-media, so today those pay a full byte-for-byte copy only to be passed through. * * This is an optimization hint, never the enforcement point. It sees only the client-supplied - * mime type and filename, which can disagree with the file's actual bytes, so the plan check - * inside [processFile] stays authoritative — the copy here is a fast path, not a replacement. + * mime type and filename, which can disagree with the file's actual bytes, so the free-plan + * check inside [processFile] stays authoritative — the copy here is a fast path, not a + * replacement. */ @Suppress("ReturnCount") override fun handlesFile(mimeType: String, filename: String): Boolean { val resolvedMimeType = resolveMimeType(mimeType, filename) - // Claim disallowed types so processFile still runs and throws the localized rejection. + // Claim plan-rejected types so processFile still runs and throws the localized rejection. // Declining would forward them to WordPress instead, spending a full upload on a file the // site's plan won't accept and surfacing the server's untranslated error in place of ours. - if (!mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType)) return true + if (isRejectedByFreePlan(resolvedMimeType)) return true return when { // Never re-encoded; processFile always returns Original. @@ -83,15 +86,7 @@ class GBKMediaUploadProcessor( ): ProcessedProxyFile = withContext(ioDispatcher) { val resolvedMimeType = resolveMimeType(mimeType, filename) - // Fallback plan check. GutenbergKit's editor validates uploads in the WebView against the - // site's allowedMimeTypes (from /wp-block-editor/v1/settings) before the request reaches - // this delegate, so for most disallowed types the editor rejects with its own localized - // message first. Those settings are cached on disk and reused on later opens, so GB - // validates against whatever mime list was cached — not necessarily the site's current - // one. This check still fires when GB's list is absent (cache miss where editor settings - // resolve to undefined) or when the app's static MimeTypes table is stricter than the - // server's list — in which case it can over-reject a type the server would accept. - if (!mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType)) { + if (isRejectedByFreePlan(resolvedMimeType)) { throw GBKMediaUploadException(appContext.getString(R.string.error_media_file_type_not_allowed)) } @@ -105,6 +100,39 @@ class GBKMediaUploadProcessor( } } + /** + * Rejects the one upload class the app can judge better than the server: audio and documents + * on a free WordPress.com plan, where the restriction is a plan entitlement rather than a + * format question, so a localized message beats the server's untranslated error. + * + * Deliberately narrow. [MediaUtilsWrapper.isMimeTypeSupportedBySitePlan] matches against a + * closed, hand-maintained table ([org.wordpress.android.fluxc.utils.MimeTypes]) that has + * drifted from what WordPress accepts — it has no `image/avif` (core-supported since 6.5), no + * `image/svg+xml`, and no text types at all, and it maps self-hosted to the same document set + * as WP.com paid. Applying it to every upload therefore rejects files the server would store. + * GutenbergKit already validates against the site's real `allowedMimeTypes` from + * `/wp-block-editor/v1/settings` before this delegate runs, so images and videos are left to + * that check and to the server, which are both authoritative where this table is not. + * + * The free-plan test mirrors [WPMediaUtils.getSitePlanForMimeTypes], which selects + * `WP_COM_FREE` from [SiteUtilsWrapper.onFreePlan] — using `hasFreePlan` here instead would + * let the gate and the allowlist it guards disagree. + * + * [MIME_OCTET_STREAM] is excluded because [resolveMimeType] emits it for uploads whose type + * could not be resolved at all; it counts as an `application` type but means "unknown bytes", + * and rejecting it would answer "we could not identify this file" with "this file type is not + * allowed". + */ + private fun isRejectedByFreePlan(resolvedMimeType: String): Boolean { + if (!site.isWPCom || !siteUtilsWrapper.onFreePlan(site)) return false + + val isPlanRestrictedType = resolvedMimeType != MIME_OCTET_STREAM && + (mediaUtilsWrapper.isAudioMimeType(resolvedMimeType) || + mediaUtilsWrapper.isApplicationMimeType(resolvedMimeType)) + + return isPlanRestrictedType && !mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(site, resolvedMimeType) + } + @Suppress("ReturnCount") private suspend fun processVideo(file: File, mimeType: String, filename: String): ProcessedProxyFile { // Pass the resolved mime type rather than letting the check re-derive "is this a video" @@ -284,9 +312,9 @@ class GBKMediaUploadProcessor( * Normalizes the client-supplied mime type, falling back to the filename extension when it * carries no usable information. * - * The result feeds [MediaUtilsWrapper.isMimeTypeSupportedBySitePlan], which is an exact, - * case-sensitive match against a closed allowlist, so anything but a bare lowercase - * `type/subtype` is rejected outright. Two shapes reach us that the allowlist would miss: + * The result feeds [isRejectedByFreePlan] and the type routing below. The plan check it + * performs is an exact, case-sensitive match against a closed allowlist, so anything but a + * bare lowercase `type/subtype` is rejected outright. Two shapes reach us that it would miss: * - Parameters and casing: `Content-Type` may legitimately carry parameters * (`image/jpeg; charset=binary`) and its casing is not significant (RFC 9110 §8.3). * - Missing header: GutenbergKit's multipart parser defaults a part with no `Content-Type` diff --git a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt index 20af968049fa..fa09667c825b 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/MediaUtilsWrapper.kt @@ -36,6 +36,12 @@ class MediaUtilsWrapper @Inject constructor(private val appContext: Context) { fun isVideoMimeType(mimeType: String?): Boolean = org.wordpress.android.fluxc.utils.MediaUtils.isVideoMimeType(mimeType) + fun isAudioMimeType(mimeType: String?): Boolean = + org.wordpress.android.fluxc.utils.MediaUtils.isAudioMimeType(mimeType) + + fun isApplicationMimeType(mimeType: String?): Boolean = + org.wordpress.android.fluxc.utils.MediaUtils.isApplicationMimeType(mimeType) + fun stripImageLocation(imagePath: String) = org.wordpress.android.fluxc.utils.MediaUtils.stripLocation(imagePath) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 38b1b7bc7aad..5b0cd2f5c710 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -22,6 +22,7 @@ import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.MediaUtilsWrapper +import org.wordpress.android.util.SiteUtilsWrapper import org.wordpress.gutenberg.ProcessedProxyFile import java.io.File @@ -34,6 +35,7 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { private lateinit var appContext: Context private lateinit var mediaUtilsWrapper: MediaUtilsWrapper private lateinit var appPrefsWrapper: AppPrefsWrapper + private lateinit var siteUtilsWrapper: SiteUtilsWrapper private lateinit var stagedFile: File @Before @@ -42,10 +44,12 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { on { getString(R.string.error_media_file_type_not_allowed) } doReturn FILE_TYPE_ERROR on { getString(R.string.error_media_video_duration_exceeds_limit) } doReturn VIDEO_LIMIT_ERROR } - mediaUtilsWrapper = mock { - on { isMimeTypeSupportedBySitePlan(anyOrNull(), any()) } doReturn true - } + mediaUtilsWrapper = mock() appPrefsWrapper = mock() + // Default to a paid plan: the free-plan rejection is opt-in per test. + siteUtilsWrapper = mock { + on { onFreePlan(any()) } doReturn false + } stagedFile = tempFolder.newFile("photo.jpg").apply { writeText("staged-bytes") } } @@ -54,6 +58,7 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { appContext = appContext, mediaUtilsWrapper = mediaUtilsWrapper, appPrefsWrapper = appPrefsWrapper, + siteUtilsWrapper = siteUtilsWrapper, ioDispatcher = testDispatcher() ) @@ -61,6 +66,11 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { private fun selfHostedSite() = SiteModel().apply { setIsWPCom(false) } + /** Puts the site on a free WP.com plan, where audio/document uploads are plan-restricted. */ + private fun onFreePlan() { + whenever(siteUtilsWrapper.onFreePlan(any())).thenReturn(true) + } + @Test fun `image is optimized when optimization produces a new file`() = test { val optimized = tempFolder.newFile("optimized.jpg") @@ -226,7 +236,9 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { } @Test - fun `disallowed file type throws with localized message`() = test { + fun `document disallowed by a free plan throws with localized message`() = test { + onFreePlan() + whenever(mediaUtilsWrapper.isApplicationMimeType("application/zip")).thenReturn(true) whenever(mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(anyOrNull(), any())).thenReturn(false) val zipStaged = tempFolder.newFile("archive.zip") @@ -239,6 +251,63 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { .hasMessage(FILE_TYPE_ERROR) } + @Test + fun `audio disallowed by a free plan throws with localized message`() = test { + onFreePlan() + whenever(mediaUtilsWrapper.isAudioMimeType("audio/mpeg")).thenReturn(true) + whenever(mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(anyOrNull(), any())).thenReturn(false) + val audioStaged = tempFolder.newFile("song.mp3") + + val thrown = runCatching { + createProcessor().processFile(audioStaged, "audio/mpeg", "song.mp3") + }.exceptionOrNull() + + assertThat(thrown) + .isInstanceOf(GBKMediaUploadException::class.java) + .hasMessage(FILE_TYPE_ERROR) + } + + @Test + fun `document missing from the allowlist uploads on a paid plan`() = test { + // Paid and self-hosted sites are not plan-restricted, so the stale MimeTypes table must + // not reject for them — the server is authoritative. The allowlist is left unstubbed + // deliberately: reaching it at all would be the bug. + val zipStaged = tempFolder.newFile("archive.zip") + + val result = createProcessor().processFile(zipStaged, "application/zip", "archive.zip") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) + } + + @Test + fun `image missing from the allowlist uploads even on a free plan`() = test { + // AVIF is core-supported since WP 6.5 but absent from the app's MimeTypes table. Images + // are never plan-restricted, so the table must not be consulted for them at all. + onFreePlan() + whenever(mediaUtilsWrapper.getOptimizedMedia(any(), any())).thenReturn(null) + val avifStaged = tempFolder.newFile("photo.avif") + + val result = createProcessor().processFile(avifStaged, "image/avif", "photo.avif") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) + } + + @Test + fun `unresolvable type is not rejected as a plan-restricted document`() = test { + // resolveMimeType emits application/octet-stream for "unknown bytes". It would classify as + // an application type, so the exclusion has to short-circuit ahead of that classification — + // isApplicationMimeType is deliberately left unstubbed to pin that ordering. + onFreePlan() + val unknownStaged = tempFolder.newFile("mystery.bin") + + val result = createProcessor().processFile(unknownStaged, "application/octet-stream", "mystery.bin") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) + } + @Test fun `video exceeding duration limit throws with localized message`() = test { whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) @@ -350,10 +419,9 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { } @Test - fun `mime type parameters are stripped before the plan check`() = test { - // Content-Type may legitimately carry parameters. isMimeTypeSupportedBySitePlan is an - // exact match against a closed allowlist, so an unnormalized value hard-fails a valid - // image. + fun `mime type parameters are stripped before the type is routed`() = test { + // Content-Type may legitimately carry parameters. An unnormalized value would miss the + // image/ prefix test and fall through to the non-media passthrough. val optimized = tempFolder.newFile("optimized.jpg") val optimizedUri = fileUri(optimized) whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) @@ -361,50 +429,55 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { val result = createProcessor().processFile(stagedFile, "image/jpeg; charset=binary", "photo.jpg") - verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("image/jpeg")) result as ProcessedProxyFile.Processed assertThat(result.mimeType).isEqualTo("image/jpeg") } @Test - fun `mime type casing is normalized before the plan check`() = test { + fun `mime type casing is normalized before the type is routed`() = test { + // An uppercase type must still route to the image path: getOptimizedMedia being consulted + // is what proves the normalization happened. whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(null) whenever(appPrefsWrapper.isStripImageLocation).thenReturn(false) val result = createProcessor(wpComSite()).processFile(stagedFile, "IMAGE/JPEG", "photo.jpg") - verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("image/jpeg")) + verify(mediaUtilsWrapper).getOptimizedMedia(stagedFile.absolutePath, false) assertThat(result).isEqualTo(ProcessedProxyFile.Original) } @Test - fun `text plain is treated as a placeholder and resolved from the filename`() { + fun `text plain is treated as a placeholder and resolved from the filename`() = test { // GutenbergKit's multipart parser defaults a part with no Content-Type to text/plain // (RFC 7578), and picks the file part by its filename parameter rather than its type — so // a real image can arrive labeled text/plain and must not be rejected as a disallowed type. // // MimeTypeMap is a stub returning null under unit tests, so the extension lookup cannot - // resolve here; this asserts the surrounding contract instead — text/plain is not taken at - // face value, and an unresolvable lookup degrades to the declared type rather than "". - createProcessor().handlesFile("text/plain", "photo.jpg") + // resolve here; this asserts the surrounding contract instead — a free-plan site does not + // reject text/plain, which is neither audio nor an application type. + onFreePlan() + val staged = tempFolder.newFile("note.txt") + + val result = createProcessor().processFile(staged, "text/plain", "note.txt") - verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("text/plain")) + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) } @Test fun `blank mime type never resolves to an empty string`() { - // An empty resolved type would be meaningless to the plan check; a placeholder it can - // reject coherently is the safe floor. - createProcessor().handlesFile("", "mystery") - - verify(mediaUtilsWrapper).isMimeTypeSupportedBySitePlan(anyOrNull(), eq("application/octet-stream")) + // An empty resolved type would be meaningless to the type routing; a placeholder is the + // safe floor. octet-stream routes to neither image nor video, so the file is not claimed. + assertThat(createProcessor().handlesFile("", "mystery")).isFalse() } @Test - fun `handlesFile claims disallowed types so processFile can reject them locally`() { + fun `handlesFile claims plan-rejected types so processFile can reject them locally`() { // Declining would relay the file to WordPress instead, wasting a full upload and replacing // our localized message with the server's. Pairs with the processFile rejection test above, // which uses the same mime type. + onFreePlan() + whenever(mediaUtilsWrapper.isApplicationMimeType("application/zip")).thenReturn(true) whenever(mediaUtilsWrapper.isMimeTypeSupportedBySitePlan(anyOrNull(), any())).thenReturn(false) assertThat(createProcessor().handlesFile("application/zip", "archive.zip")).isTrue() From 7f3da1f0b58bbfbf411534d9aaa0d504b6b2b3a8 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 12:08:50 -0400 Subject: [PATCH 21/27] perf: claim uploads only when processing would read the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handlesFile claimed every image and video unconditionally. Claiming makes GutenbergKit write a full byte-for-byte copy of the upload to a temp file before calling processFile, so the gate is only worth paying when processing will actually read that file. It often will not. processVideo returns Original immediately when video optimization is off, and the duration check only measures on a free plan without VideoPress — so on a paid or self-hosted site with optimization off, a common configuration since optimization is opt-in, every video upload wrote a second full copy of the file to the cache dir for nothing. For a 1 GB video that is a gigabyte of pointless I/O and a plausible ENOSPC. processImage has the same shape: with optimization off, strip off, and the site on WP.com, it returns Original without touching the file. Gate both branches on the decisions the two methods actually make. The image gate keeps the self-hosted term so the issue #5737 rotation fallback still gets its file, and only counts the location strip for formats androidx ExifInterface can rewrite — a HEIC with strip enabled reads no file either. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 46 +++++++++-- .../android/ui/prefs/AppPrefsWrapper.kt | 3 + .../editor/GBKMediaUploadProcessorTest.kt | 82 ++++++++++++++++++- 3 files changed, 124 insertions(+), 7 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index fa2f26320330..9c8d67aaf34c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -51,8 +51,12 @@ class GBKMediaUploadProcessor( /** * Metadata-only gate GutenbergKit consults before copying an upload to a temp file. Declining * makes it relay the original request body straight to WordPress, skipping a copy this - * delegate would not have used: [processFile] returns [ProcessedProxyFile.Original] for GIFs - * and non-media, so today those pay a full byte-for-byte copy only to be passed through. + * delegate would not have used — GIFs, non-media, and any media whose processing is switched + * off would otherwise pay a full byte-for-byte copy only to be passed through. + * + * The media branches mirror the decisions [processVideo] and [processImage] actually make + * (see [needsVideoFile] / [needsImageFile]) rather than claiming every image and video, so a + * site with optimization off does not copy uploads it will never touch. * * This is an optimization hint, never the enforcement point. It sees only the client-supplied * mime type and filename, which can disagree with the file's actual bytes, so the free-plan @@ -71,14 +75,43 @@ class GBKMediaUploadProcessor( return when { // Never re-encoded; processFile always returns Original. resolvedMimeType == MIME_GIF -> false - // Both the duration check and the optional transcode need the file itself. - mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> true - resolvedMimeType.startsWith(MIME_IMAGE_PREFIX) -> true + mediaUtilsWrapper.isVideoMimeType(resolvedMimeType) -> needsVideoFile() + resolvedMimeType.startsWith(MIME_IMAGE_PREFIX) -> needsImageFile(resolvedMimeType) // Non-media files (documents, archives, audio on paid plans) upload unchanged. else -> false } } + /** + * Whether [processVideo] would actually read the staged file, mirroring its two exits: + * the duration check (which only measures on a free plan without VideoPress — see + * [MediaUtilsWrapper.isProhibitedVideoDuration]) and the transcode. + * + * With video optimization off on a paid or self-hosted site — a common configuration, since + * optimization is opt-in — the method returns [ProcessedProxyFile.Original] without touching + * the file, so claiming it would make GutenbergKit write a full byte-for-byte copy of the + * upload to the cache dir for nothing. For a multi-gigabyte video that is gigabytes of I/O + * and a plausible ENOSPC. + */ + private fun needsVideoFile(): Boolean { + val durationCheckApplies = site.hasFreePlan && !site.isActiveModuleEnabled(VIDEOPRESS_MODULE) + return durationCheckApplies || appPrefsWrapper.isVideoOptimize + } + + /** + * Whether [processImage] would actually read the staged file, mirroring its three exits: + * optimization ([WPMediaUtils.getOptimizedMedia] short-circuits when the pref is off), + * the self-hosted rotation fallback for issue #5737, and the EXIF location strip (which only + * copies for the formats androidx ExifInterface can rewrite — see [EXIF_MIME_TYPES]). + * + * All three must be inapplicable to decline. Dropping the rotation term would regress #5737 + * parity on self-hosted sites, where the server does not rotate for us. + */ + private fun needsImageFile(resolvedMimeType: String): Boolean { + val stripsLocation = appPrefsWrapper.isStripImageLocation && resolvedMimeType in EXIF_MIME_TYPES + return appPrefsWrapper.isImageOptimize || !site.isWPCom || stripsLocation + } + override suspend fun processFile( file: File, mimeType: String, @@ -348,6 +381,9 @@ class GBKMediaUploadProcessor( */ private val transcodeMutex = Mutex() + /** Site module that lifts the free-plan video duration limit. */ + private const val VIDEOPRESS_MODULE = "videopress" + private const val MIME_IMAGE_PREFIX = "image/" private const val MIME_GIF = "image/gif" private const val MIME_PNG = "image/png" diff --git a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt index 708e80fa5b3e..46e4323281e4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt @@ -49,6 +49,9 @@ class AppPrefsWrapper @Inject constructor(val buildConfigWrapper: BuildConfigWra get() = AppPrefs.isAztecEditorEnabled() set(enabled) = AppPrefs.setAztecEditorEnabled(enabled) + val isImageOptimize: Boolean + get() = AppPrefs.isImageOptimize() + val isVideoOptimize: Boolean get() = AppPrefs.isVideoOptimize() diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 5b0cd2f5c710..f63380b2dc5d 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -71,6 +71,16 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { whenever(siteUtilsWrapper.onFreePlan(any())).thenReturn(true) } + /** + * A site subject to the free-plan video duration limit. Note this is [SiteModel.hasFreePlan] + * (the API's `plan.is_free`), which gates the duration check, and is distinct from + * [SiteUtilsWrapper.onFreePlan] (a plan-id match) used for the mime allowlist. + */ + private fun freePlanSite() = SiteModel().apply { + setIsWPCom(true) + setHasFreePlan(true) + } + @Test fun `image is optimized when optimization produces a new file`() = test { val optimized = tempFolder.newFile("optimized.jpg") @@ -407,17 +417,76 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { } @Test - fun `handlesFile claims images`() { + fun `handlesFile claims images when optimization is on`() { + whenever(appPrefsWrapper.isImageOptimize).thenReturn(true) + + assertThat(createProcessor().handlesFile("image/jpeg", "photo.jpg")).isTrue() + } + + @Test + fun `handlesFile claims images when the location strip applies`() { + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + assertThat(createProcessor().handlesFile("image/jpeg", "photo.jpg")).isTrue() } @Test - fun `handlesFile claims videos`() { + fun `handlesFile claims images on self-hosted for the rotation fallback`() { + // Issue #5737: self-hosted sites are not rotated server-side, so processImage still needs + // the file even with every optimization pref off. + assertThat(createProcessor(selfHostedSite()).handlesFile("image/jpeg", "photo.jpg")).isTrue() + } + + @Test + fun `handlesFile declines images when nothing would touch the file`() { + // WP.com, optimization off, strip off: processImage returns Original without reading the + // file, so claiming it would cost a full copy for nothing. + assertThat(createProcessor(wpComSite()).handlesFile("image/jpeg", "photo.jpg")).isFalse() + } + + @Test + fun `handlesFile declines images whose format cannot be exif-stripped`() { + // androidx ExifInterface cannot rewrite HEIC, so the strip is a no-op for it and the file + // is never read — see EXIF_MIME_TYPES. + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + + assertThat(createProcessor(wpComSite()).handlesFile("image/heic", "photo.heic")).isFalse() + } + + @Test + fun `handlesFile claims videos when optimization is on`() { whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + whenever(appPrefsWrapper.isVideoOptimize).thenReturn(true) assertThat(createProcessor().handlesFile("video/mp4", "movie.mp4")).isTrue() } + @Test + fun `handlesFile claims videos on a free plan for the duration check`() { + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + + assertThat(createProcessor(freePlanSite()).handlesFile("video/mp4", "movie.mp4")).isTrue() + } + + @Test + fun `handlesFile declines videos when nothing would touch the file`() { + // Paid plan with video optimization off — a common configuration, since optimization is + // opt-in. Claiming here copies the whole video to cache for a guaranteed passthrough. + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + + assertThat(createProcessor(wpComSite()).handlesFile("video/mp4", "movie.mp4")).isFalse() + } + + @Test + fun `handlesFile declines videos on a free plan with VideoPress enabled`() { + // VideoPress lifts the duration limit, so the check does not measure and nothing else + // reads the file with optimization off. + whenever(mediaUtilsWrapper.isVideoMimeType("video/mp4")).thenReturn(true) + val site = freePlanSite().apply { activeModules = "videopress" } + + assertThat(createProcessor(site).handlesFile("video/mp4", "movie.mp4")).isFalse() + } + @Test fun `mime type parameters are stripped before the type is routed`() = test { // Content-Type may legitimately carry parameters. An unnormalized value would miss the @@ -483,6 +552,15 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { assertThat(createProcessor().handlesFile("application/zip", "archive.zip")).isTrue() } + @Test + fun `handlesFile declines documents that are not plan-rejected`() { + // On a paid plan there is nothing for processFile to say about a document, so claiming it + // would cost a full byte-for-byte copy for a guaranteed passthrough. The plan test + // short-circuits before the type is even classified. + assertThat(createProcessor().handlesFile("application/zip", "archive.zip")).isFalse() + verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) + } + private fun fileUri(file: File): android.net.Uri = mock { on { path } doReturn file.absolutePath } From 78b057702428327b1b5e276014b387d8bfeddf9f Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 12:09:01 -0400 Subject: [PATCH 22/27] fix: warn when the media upload delegate arrives after the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setMediaUploadDelegate only stores a field; onCreateView is the sole place it reaches GutenbergView, because GutenbergKit captures the delegate when the page begins loading and throws from its setter afterward. Every other hook here pushes into the live view via gutenbergView?.setX and therefore survives a late call, but this one cannot — arriving late is a silent no-op, and uploads quietly fall back to GutenbergKit's unprocessed WebView path. Log it so the failure is visible rather than invisible. Verified on device across both deferred setup paths (private WP.com and Atomic cookie fetch, with rotation during load): the delegate reaches the view before onCreateView in every case and the warning does not fire, so this is insurance against a timing we could not reproduce, not a known break. Co-Authored-By: Claude Opus 5 (1M context) --- .../ui/posts/editor/GutenbergKitEditorFragment.kt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt index 94cc5f64f630..320efc446b12 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GutenbergKitEditorFragment.kt @@ -567,8 +567,21 @@ class GutenbergKitEditorFragment : GutenbergKitEditorFragmentBase() { * which is the only place the delegate reaches the view: GutenbergKit captures it when the * editor page begins loading and throws from its setter afterward, so pushing it into a live * view here would crash. The field is the single source of truth. + * + * Unlike the other hooks here ([setNetworkRequestListener], [setImageLoader]), this one cannot + * push into an already-created view, so arriving late is a silent no-op: uploads fall back to + * GutenbergKit's unprocessed WebView path with no error. That can only happen if the view was + * created before the delegate was assigned — e.g. a configuration change restoring the fragment + * ahead of a deferred setupViewPager() callback. Log it rather than let it pass unnoticed. */ fun setMediaUploadDelegate(delegate: MediaUploadDelegate) { + if (gutenbergView != null) { + AppLog.w( + AppLog.T.MEDIA, + "GutenbergKitEditorFragment: media upload delegate set after the view was created" + + " - uploads will bypass the app's media settings for this session" + ) + } mediaUploadDelegate = delegate } From b10438d6a64058603bc7e38b6154759c83ba066a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 12:40:13 -0400 Subject: [PATCH 23/27] feat: track video optimization analytics for GutenbergKit transcodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VideoOptimizer emits MEDIA_VIDEO_OPTIMIZED, MEDIA_VIDEO_CANT_OPTIMIZE, and MEDIA_VIDEO_OPTIMIZE_ERROR, but transcodeVideo emitted none, so GutenbergKit video optimization was invisible in telemetry. Image optimization is unaffected — it inherits MEDIA_PHOTO_OPTIMIZED and MEDIA_PHOTO_OPTIMIZE_ERROR from inside getOptimizedMedia for free. With the rollout measuring parity against the legacy editor, that asymmetry is worth closing. Mirror VideoOptimizer's events and property shapes, including the input_video_/output_video_ prefixes, saved_megabytes, elapsed_time_ms, was_npe_detected, and optimizer_lib, so the two pipelines are directly comparable. Output properties are attached only on success, where the file still exists and its size is meaningful. Note the composer.start() IllegalStateException path deliberately emits nothing, matching VideoOptimizer, which also only logs there. Co-Authored-By: Claude Opus 5 (1M context) --- .../android/ui/posts/GutenbergKitActivity.kt | 4 ++ .../posts/editor/GBKMediaUploadProcessor.kt | 60 +++++++++++++++++++ .../editor/GBKMediaUploadProcessorTest.kt | 8 +++ 3 files changed, 72 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt index 401f46e767d7..e790cc281de3 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergKitActivity.kt @@ -209,6 +209,7 @@ import org.wordpress.android.util.WPMediaUtils import org.wordpress.android.util.WPPermissionUtils import org.wordpress.android.util.WPUrlUtils import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper +import org.wordpress.android.util.analytics.AnalyticsUtilsWrapper import org.wordpress.android.util.analytics.AnalyticsUtils import org.wordpress.android.util.analytics.AnalyticsUtils.BlockEditorEnabledSource import org.wordpress.android.util.config.ContactSupportFeatureConfig @@ -361,6 +362,7 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene @Inject lateinit var reblogUtils: ReblogUtils @Inject lateinit var analyticsTrackerWrapper: AnalyticsTrackerWrapper + @Inject lateinit var analyticsUtilsWrapper: AnalyticsUtilsWrapper @Inject lateinit var publishPostImmediatelyUseCase: PublishPostImmediatelyUseCase @@ -2276,6 +2278,8 @@ class GutenbergKitActivity : BaseAppCompatActivity(), EditorImageSettingsListene mediaUtilsWrapper = mediaUtilsWrapper, appPrefsWrapper = appPrefsWrapper, siteUtilsWrapper = siteUtilsWrapper, + analyticsTrackerWrapper = analyticsTrackerWrapper, + analyticsUtilsWrapper = analyticsUtilsWrapper, ) ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 9c8d67aaf34c..2a765f4ebef4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -13,9 +13,12 @@ import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.AppLog +import org.wordpress.android.analytics.AnalyticsTracker import org.wordpress.android.util.MediaUtilsWrapper import org.wordpress.android.util.SiteUtilsWrapper import org.wordpress.android.util.WPVideoUtils +import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper +import org.wordpress.android.util.analytics.AnalyticsUtilsWrapper import org.wordpress.gutenberg.MediaUploadDelegate import org.wordpress.gutenberg.ProcessedProxyFile import java.io.File @@ -46,6 +49,8 @@ class GBKMediaUploadProcessor( private val mediaUtilsWrapper: MediaUtilsWrapper, private val appPrefsWrapper: AppPrefsWrapper, private val siteUtilsWrapper: SiteUtilsWrapper, + private val analyticsTrackerWrapper: AnalyticsTrackerWrapper, + private val analyticsUtilsWrapper: AnalyticsUtilsWrapper, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : MediaUploadDelegate { /** @@ -206,6 +211,7 @@ class GBKMediaUploadProcessor( // "wp-{currentTimeMillis}.mp4" and collides for two transcodes started in the same // millisecond. Uniqueness comes from the filesystem, with no check-then-create race. val output = File.createTempFile("wp-", ".mp4", appContext.cacheDir) + val startTimeMs = System.currentTimeMillis() val listener = object : IProgressListener { override fun onMediaStart() = Unit override fun onMediaProgress(progress: Float) = Unit @@ -216,16 +222,19 @@ class GBKMediaUploadProcessor( override fun onMediaStop() = Unit override fun onMediaDone() { + trackTranscodeFinished(input, output, startTimeMs, null) if (continuation.isActive) continuation.resume(output) } override fun onError(exception: Exception) { AppLog.e(AppLog.T.MEDIA, "GBKMediaUploadProcessor > video transcode failed", exception) + trackTranscodeFinished(input, output, startTimeMs, exception) output.delete() if (continuation.isActive) continuation.resume(null) } } + var wasNpeDetected = false val composer = try { WPVideoUtils.getVideoOptimizationComposer( appContext, @@ -238,10 +247,12 @@ class GBKMediaUploadProcessor( } catch (npe: NullPointerException) { // m4m throws NPEs on some malformed inputs; the legacy pipeline guards this too. AppLog.w(AppLog.T.MEDIA, "GBKMediaUploadProcessor > NPE getting composer: ${npe.message}") + wasNpeDetected = true null } if (composer == null) { + trackCantOptimize(input, wasNpeDetected) output.delete() continuation.resume(null) return@suspendCancellableCoroutine @@ -270,6 +281,50 @@ class GBKMediaUploadProcessor( } } + /** + * Mirrors [org.wordpress.android.ui.uploads.VideoOptimizer]'s null-composer event so a + * GutenbergKit transcode that never starts is as visible in telemetry as a legacy one. + */ + private fun trackCantOptimize(input: File, wasNpeDetected: Boolean) { + val properties = analyticsUtilsWrapper.getMediaProperties(true, null, input.absolutePath) + properties["was_npe_detected"] = wasNpeDetected + properties[PROPERTY_OPTIMIZER_LIB] = OPTIMIZER_LIB_M4M + analyticsTrackerWrapper.track(AnalyticsTracker.Stat.MEDIA_VIDEO_CANT_OPTIMIZE, properties) + } + + /** + * Mirrors [org.wordpress.android.ui.uploads.VideoOptimizer.trackVideoProcessingEvents], + * including its `input_video_`/`output_video_` property prefixes, so GutenbergKit transcodes + * are comparable with legacy ones while the rollout measures parity. + * + * Output properties are only attached on success: on failure the output file is deleted + * moments later and its size would be meaningless. + */ + private fun trackTranscodeFinished(input: File, output: File, startTimeMs: Long, exception: Exception?) { + val properties = mutableMapOf() + analyticsUtilsWrapper.getMediaProperties(true, null, input.absolutePath) + .forEach { (key, value) -> properties["input_video_$key"] = value } + + if (exception == null) { + analyticsUtilsWrapper.getMediaProperties(true, null, output.absolutePath) + .forEach { (key, value) -> properties["output_video_$key"] = value } + properties["saved_megabytes"] = ((input.length() - output.length()) / BYTES_PER_MEGABYTE).toString() + } else { + properties["exception_name"] = exception.javaClass.canonicalName + properties["exception_message"] = exception.message + } + + properties["elapsed_time_ms"] = System.currentTimeMillis() - startTimeMs + properties[PROPERTY_OPTIMIZER_LIB] = OPTIMIZER_LIB_M4M + + val stat = if (exception == null) { + AnalyticsTracker.Stat.MEDIA_VIDEO_OPTIMIZED + } else { + AnalyticsTracker.Stat.MEDIA_VIDEO_OPTIMIZE_ERROR + } + analyticsTrackerWrapper.track(stat, properties) + } + @Suppress("ReturnCount") private fun processImage(file: File, mimeType: String, filename: String): ProcessedProxyFile { // getOptimizedMedia returns null when optimization is disabled or a no-op. It can also @@ -384,6 +439,11 @@ class GBKMediaUploadProcessor( /** Site module that lifts the free-plan video duration limit. */ private const val VIDEOPRESS_MODULE = "videopress" + /** Matches VideoOptimizer's analytics so GutenbergKit and legacy transcodes compare. */ + private const val PROPERTY_OPTIMIZER_LIB = "optimizer_lib" + private const val OPTIMIZER_LIB_M4M = "m4m" + private const val BYTES_PER_MEGABYTE = 1024 * 1024 + private const val MIME_IMAGE_PREFIX = "image/" private const val MIME_GIF = "image/gif" private const val MIME_PNG = "image/png" diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index f63380b2dc5d..dd11bc4a38d1 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -23,6 +23,8 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.MediaUtilsWrapper import org.wordpress.android.util.SiteUtilsWrapper +import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper +import org.wordpress.android.util.analytics.AnalyticsUtilsWrapper import org.wordpress.gutenberg.ProcessedProxyFile import java.io.File @@ -36,6 +38,8 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { private lateinit var mediaUtilsWrapper: MediaUtilsWrapper private lateinit var appPrefsWrapper: AppPrefsWrapper private lateinit var siteUtilsWrapper: SiteUtilsWrapper + private lateinit var analyticsTrackerWrapper: AnalyticsTrackerWrapper + private lateinit var analyticsUtilsWrapper: AnalyticsUtilsWrapper private lateinit var stagedFile: File @Before @@ -50,6 +54,8 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { siteUtilsWrapper = mock { on { onFreePlan(any()) } doReturn false } + analyticsTrackerWrapper = mock() + analyticsUtilsWrapper = mock() stagedFile = tempFolder.newFile("photo.jpg").apply { writeText("staged-bytes") } } @@ -59,6 +65,8 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { mediaUtilsWrapper = mediaUtilsWrapper, appPrefsWrapper = appPrefsWrapper, siteUtilsWrapper = siteUtilsWrapper, + analyticsTrackerWrapper = analyticsTrackerWrapper, + analyticsUtilsWrapper = analyticsUtilsWrapper, ioDispatcher = testDispatcher() ) From 3e64468febf6bacad1f8807dfc24fbc6d75397de Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 12:59:39 -0400 Subject: [PATCH 24/27] fix: delete the strip-location temp file when the copy fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit processImage created a temp file in the cache dir and then copied the staged upload into it. If the copy threw — a full disk being the likely cause — the just-created file was abandoned there: GutenbergKit only deletes the files it is handed back, and this one never gets returned. Delete it on failure and rethrow, so the disk-full case does not also leak. Also drop the test's duplicate @RunWith(MockitoJUnitRunner::class), which BaseUnitTest already carries and JUnit inherits. Its @ExperimentalCoroutinesApi is kept: Kotlin's opt-in requirement is not inherited by subclasses, so removing that one fails the build under -Werror. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 8 +++++- .../editor/GBKMediaUploadProcessorTest.kt | 25 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 2a765f4ebef4..c59ab351e7dd 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -356,7 +356,13 @@ class GBKMediaUploadProcessor( // request body byte-for-byte, so stripping EXIF from the staged file in place would // silently upload the un-stripped bytes. val copy = File.createTempFile("gbk-media", ".${file.extension}", appContext.cacheDir) - file.copyTo(copy, overwrite = true) + // A failed copy (a full disk being the likely cause) would otherwise leave the + // just-created temp file behind in the cache dir: GutenbergKit only deletes the files + // it is handed back, and this one never gets returned. + runCatching { file.copyTo(copy, overwrite = true) }.onFailure { + copy.delete() + throw it + } mediaUtilsWrapper.stripImageLocation(copy.absolutePath) return ProcessedProxyFile.Processed(copy, mimeType, filename) } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index dd11bc4a38d1..2e6d562be420 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -7,8 +7,6 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder -import org.junit.runner.RunWith -import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.doReturn @@ -28,8 +26,8 @@ import org.wordpress.android.util.analytics.AnalyticsUtilsWrapper import org.wordpress.gutenberg.ProcessedProxyFile import java.io.File +// BaseUnitTest carries this too, but Kotlin's opt-in requirement is not inherited by subclasses. @ExperimentalCoroutinesApi -@RunWith(MockitoJUnitRunner::class) class GBKMediaUploadProcessorTest : BaseUnitTest() { @get:Rule val tempFolder = TemporaryFolder() @@ -41,12 +39,16 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { private lateinit var analyticsTrackerWrapper: AnalyticsTrackerWrapper private lateinit var analyticsUtilsWrapper: AnalyticsUtilsWrapper private lateinit var stagedFile: File + private lateinit var cacheDir: File @Before fun setUp() { + // A real directory so tests can assert on the temp files the processor writes there. + cacheDir = tempFolder.newFolder("cache") appContext = mock { on { getString(R.string.error_media_file_type_not_allowed) } doReturn FILE_TYPE_ERROR on { getString(R.string.error_media_video_duration_exceeds_limit) } doReturn VIDEO_LIMIT_ERROR + on { getCacheDir() } doReturn cacheDir } mediaUtilsWrapper = mock() appPrefsWrapper = mock() @@ -135,6 +137,23 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { result.file.delete() } + @Test + fun `failed strip copy does not leak the temp file`() = test { + // GutenbergKit only deletes files handed back to it, so a temp file abandoned mid-copy + // (a full disk being the likely cause) would sit in the cache dir indefinitely. + val missing = File(tempFolder.root, "vanished.jpg") + whenever(mediaUtilsWrapper.getOptimizedMedia(missing.absolutePath, false)).thenReturn(null) + whenever(appPrefsWrapper.isStripImageLocation).thenReturn(true) + val cacheFilesBefore = cacheDir.listFiles()?.size ?: 0 + + val thrown = runCatching { + createProcessor(wpComSite()).processFile(missing, "image/jpeg", "vanished.jpg") + }.exceptionOrNull() + + assertThat(thrown).isNotNull() + assertThat(cacheDir.listFiles()?.size ?: 0).isEqualTo(cacheFilesBefore) + } + @Test fun `gps is stripped from the optimized output when strip enabled`() = test { val optimized = tempFolder.newFile("optimized.jpg") From 08602b596d15351eaf71343354f18493762aba8e Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 13:45:55 -0400 Subject: [PATCH 25/27] fix: build media URIs from files instead of parsing them as strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getOptimizedMedia and fixOrientationIssue returned Uri.parse(path) on a bare filesystem path. Uri.parse reads everything after a '#' as a fragment and after a '?' as a query, so for a file named "IMG_#1.jpg" the resulting getPath() is truncated to a path that does not exist. The upload then sends nothing or fails outright. The exposure is new: legacy callers get their paths from MediaStore, whereas GutenbergKit names the staged file after the client-supplied multipart filename, which is far less constrained. Uri.fromFile encodes the path rather than parsing it, and getRealPathFromURI already handles the file:// scheme these now carry — its no-scheme branch only worked by accident. Consuming a file:// Uri means getPath() is percent-encoded, so the processor decodes before touching the filesystem, via a wrapper method because Uri.decode is a stub returning null under unit tests. An existence check backstops both: an unresolvable path now degrades to a clean passthrough of the original upload instead of a File that is not there. Co-Authored-By: Claude Opus 5 (1M context) --- .../posts/editor/GBKMediaUploadProcessor.kt | 48 +++++++++++++++---- .../wordpress/android/util/WPMediaUtils.java | 10 +++- .../editor/GBKMediaUploadProcessorTest.kt | 31 ++++++++++++ 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index c59ab351e7dd..5d861c6beb8d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -1,6 +1,7 @@ package org.wordpress.android.ui.posts.editor import android.content.Context +import android.net.Uri import android.webkit.MimeTypeMap import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers @@ -331,23 +332,21 @@ class GBKMediaUploadProcessor( // return the *input* path unchanged (GIF-like skips, decode failures inside // ImageUtils.optimizeImage) — treat that as "not optimized" too, otherwise the original // file would be mislabeled with a corrected JPEG mime type below. - val optimizedPath = mediaUtilsWrapper.getOptimizedMedia(file.absolutePath, false) - ?.path - ?.takeIf { it != file.absolutePath } + val optimized = mediaUtilsWrapper.getOptimizedMedia(file.absolutePath, false) + .toDistinctExistingFile(file) - if (optimizedPath != null) { - return processedImage(File(optimizedPath), filename) + if (optimized != null) { + return processedImage(optimized, filename) } // With optimization off, WP.com rotates sideways-captured images server-side but // self-hosted sites don't, so rotate physically (legacy parity — see issue #5737). // Returns null when no rotation is needed. if (!site.isWPCom) { - val rotatedPath = mediaUtilsWrapper.fixOrientationIssue(file.absolutePath, false) - ?.path - ?.takeIf { it != file.absolutePath } - if (rotatedPath != null) { - return processedImage(File(rotatedPath), filename) + val rotated = mediaUtilsWrapper.fixOrientationIssue(file.absolutePath, false) + .toDistinctExistingFile(file) + if (rotated != null) { + return processedImage(rotated, filename) } } @@ -372,6 +371,35 @@ class GBKMediaUploadProcessor( return ProcessedProxyFile.Original } + /** + * Resolves an image-processing result to a usable output file, or null to fall back to the + * original upload. + * + * Both [WPMediaUtils.getOptimizedMedia] and [WPMediaUtils.fixOrientationIssue] return the + * *input* path unchanged in their no-op cases (GIF-like skips, decode failures inside + * ImageUtils, no rotation needed), which must be read as "not processed" — otherwise the + * original file would be relabeled with a corrected JPEG mime type by [processedImage]. + * + * The existence check is the backstop: GutenbergKit names the staged file after the + * client-supplied multipart filename, which is far less constrained than the MediaStore paths + * the legacy callers pass in, so a path that cannot be resolved degrades to a clean + * passthrough instead of an upload of a file that is not there. + */ + private fun Uri?.toDistinctExistingFile(input: File): File? { + val resolvedPath = this?.path ?: return null + if (resolvedPath == input.absolutePath) return null + + val output = File(resolvedPath) + if (!output.exists()) { + AppLog.w( + AppLog.T.MEDIA, + "GBKMediaUploadProcessor > processed image path does not exist, using the original" + ) + return null + } + return output + } + /** * Wraps an optimized/rotated image file, stripping GPS EXIF when enabled and correcting the * reported mime type and filename: ImageUtils re-encodes PNG to PNG and everything else diff --git a/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java b/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java index e37725a067da..9b882e8741bb 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java @@ -92,7 +92,11 @@ public static Uri getOptimizedMedia(Context context, String path, boolean isVide ExifUtils.writeExifData(exifData, optimizedPath); AnalyticsTracker.track(AnalyticsTracker.Stat.MEDIA_PHOTO_OPTIMIZED); - return Uri.parse(optimizedPath); + // fromFile, not parse: these are filesystem paths, and Uri.parse reads everything + // after a '#' as a fragment and after a '?' as a query, so a filename containing + // either (e.g. "IMG_#1.jpg") yields a Uri whose getPath() is truncated to a file that + // does not exist. fromFile encodes the path instead of parsing it. + return Uri.fromFile(new File(optimizedPath)); } return null; } @@ -104,7 +108,9 @@ public static Uri fixOrientationIssue(Context context, String path, boolean isVi String rotatedPath = ImageUtils.rotateImageIfNecessary(context, path); if (rotatedPath != null) { - return Uri.parse(rotatedPath); + // See getOptimizedMedia above: fromFile encodes a filesystem path, Uri.parse would + // truncate it at a '#' or '?' in the filename. + return Uri.fromFile(new File(rotatedPath)); } return null; diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt index 2e6d562be420..d93aa6bf8d66 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessorTest.kt @@ -137,6 +137,36 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { result.file.delete() } + @Test + fun `optimized path that does not exist falls back to the original`() = test { + // GutenbergKit names the staged file after the client-supplied multipart filename, so a + // path that fails to resolve is reachable here in a way it never was for the MediaStore + // paths the legacy callers pass in. Uploading a File that is not there would send nothing. + val missing = mock { + on { path } doReturn File(tempFolder.root, "never-written.jpg").absolutePath + } + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)).thenReturn(missing) + + val result = createProcessor().processFile(stagedFile, "image/jpeg", "photo.jpg") + + assertThat(result).isEqualTo(ProcessedProxyFile.Original) + } + + @Test + fun `optimized path containing a percent sign is used verbatim`() = test { + // Uri.getPath is already decoded. Decoding it again reads "%_d" as an escape sequence and + // mangles the path, so a file whose name contains a literal '%' would fail to resolve. + val optimized = tempFolder.newFile("100%_done.jpg") + val optimizedUri = fileUri(optimized) + whenever(mediaUtilsWrapper.getOptimizedMedia(stagedFile.absolutePath, false)) + .thenReturn(optimizedUri) + + val result = createProcessor().processFile(stagedFile, "image/jpeg", "100%_done.jpg") + + result as ProcessedProxyFile.Processed + assertThat(result.file.absolutePath).isEqualTo(optimized.absolutePath) + } + @Test fun `failed strip copy does not leak the temp file`() = test { // GutenbergKit only deletes files handed back to it, so a temp file abandoned mid-copy @@ -588,6 +618,7 @@ class GBKMediaUploadProcessorTest : BaseUnitTest() { verify(mediaUtilsWrapper, never()).isMimeTypeSupportedBySitePlan(anyOrNull(), any()) } + /** A Uri standing in for [android.net.Uri.fromFile], whose path resolves to the real file. */ private fun fileUri(file: File): android.net.Uri = mock { on { path } doReturn file.absolutePath } From 5294b1952b45fe53c5262a3392ccdd4852a33a1b Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 15:03:21 -0400 Subject: [PATCH 26/27] build: pin wordpress-utils to the PR build with the path decode fix The previous snapshot predates WordPress-Utils-Android#156's second commit, which decodes percent-encoded paths before the EXIF orientation fast path so a "file://" argument resolves instead of falling through to the failing MediaStore query. Still a PR build; the TODO to restore a tagged release stands. Co-Authored-By: Claude Opus 5 (1M context) --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 329649acb436..8fa3252e5883 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -103,7 +103,7 @@ wordpress-lint = '2.2.0' wordpress-persistent-edittext = '1.0.2' wordpress-rs = '0.6.0' # TODO: Restore to a tagged release once WordPress-Utils-Android#156 merges (PR-build snapshot) -wordpress-utils = '156-2e542df55715dff22c21def058bea551ee9569d0' +wordpress-utils = '156-f375205024780f54c4b6e4e834ae1bbace69322a' automattic-ucrop = '2.2.11' zendesk = '5.5.3' turbine = '1.2.1' From 3ab55ca870f9e8b4bcf01d97e1450a9515587308 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 19 Aug 2026 16:03:00 -0400 Subject: [PATCH 27/27] fix: satisfy detekt in GBKMediaUploadProcessor LongParameterList on the constructor and ReturnCount on toDistinctExistingFile. Both are suppressed rather than restructured: the constructor takes injected wrappers but is built per-editor with a site, so it cannot carry @Inject (which the rule exempts), and the guard clauses read more clearly than the expression form that would satisfy the return limit. Co-Authored-By: Claude Opus 5 (1M context) --- .../android/ui/posts/editor/GBKMediaUploadProcessor.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt index 5d861c6beb8d..7870807d6580 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/GBKMediaUploadProcessor.kt @@ -44,6 +44,7 @@ import kotlin.coroutines.resume * - Thrown exceptions are relayed to the editor as an error notice showing the exception message, * so messages must be localized and user-facing. */ +@Suppress("LongParameterList") class GBKMediaUploadProcessor( private val site: SiteModel, private val appContext: Context, @@ -385,6 +386,7 @@ class GBKMediaUploadProcessor( * the legacy callers pass in, so a path that cannot be resolved degrades to a clean * passthrough instead of an upload of a file that is not there. */ + @Suppress("ReturnCount") private fun Uri?.toDistinctExistingFile(input: File): File? { val resolvedPath = this?.path ?: return null if (resolvedPath == input.absolutePath) return null