Skip to content

[Android] MediaPicker: Stop overwriting picked source files #36572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kubaflo merged 3 commits into from
Aug 9, 2026
Merged

[Android] MediaPicker: Stop overwriting picked source files #36572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
937544b
[Android] MediaPicker no longer overwrites the picked source file
mattleibow Jul 14, 2026
aed7544
[Android] Compose picker rotation/compression in a single pass
mattleibow Jul 15, 2026
9b0d5d4
[Android] Clean up MAUI-owned picker cache input after processing
mattleibow Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/Essentials/src/FileSystem/FileSystemUtils.android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,40 @@ public static Java.IO.File GetTemporaryFile(Java.IO.File root, string fileName)
return tmpFile;
}

// Determines whether a path is a temporary file that MAUI itself created via
// GetTemporaryFile - i.e. one that lives under the "<cache>/<EssentialsFolderHash>/"
// folder that only GetTemporaryFile ever creates. This is a reliable ownership marker:
// files picked from the gallery, an SD card, another app's shared storage, or resolved
// to a raw physical path are never located under that folder, so callers can safely
// clean up an owned temporary input without any risk of touching a user-owned source.
internal static bool IsMauiOwnedTemporaryFile(string path)
{
if (string.IsNullOrEmpty(path))
return false;

try
{
var canonicalPath = new Java.IO.File(path).CanonicalPath;

foreach (var root in new[] { Application.Context.CacheDir, Application.Context.ExternalCacheDir })

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

💡 Suggestion — Android Platform / Performance: each call allocates a Java.IO.File[], crosses the JNI bridge for Application.Context twice, and leaks three undisposed Java.IO.File peers (new Java.IO.File(path) plus one per root) whose CanonicalPath also hits JNI.

Per android.instructions.md, cache Context in a local before repeated use. This is not a hot path (once per picked image), so it is not a correctness problem, but the same result can be had without the array or the extra JNI hops — e.g. hoist var context = Application.Context;, compute the two candidate roots into strings, and compare against the single canonicalized input path. Wrapping the Java.IO.File instances in using would also release the peers deterministically.

{
if (root is null)
continue;

var ownedRoot = new Java.IO.File(root, EssentialsFolderHash).CanonicalPath + Java.IO.File.Separator;
if (canonicalPath.StartsWith(ownedRoot, StringComparison.Ordinal))
return true;
}
}
catch
{
// If the path cannot be canonicalized, err on the side of caution and never treat
// it as owned - callers use this to decide whether to delete, so false is the safe default.
}

return false;
}

public static string EnsurePhysicalPath(AndroidUri uri, bool requireExtendedAccess = true)
{
// if this is a file, use that
Expand Down
319 changes: 116 additions & 203 deletions src/Essentials/src/MediaPicker/MediaPicker.android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,73 +25,14 @@ partial class MediaPickerImplementation : IMediaPicker
public bool IsCaptureSupported
=> Application.Context?.PackageManager?.HasSystemFeature(PackageManager.FeatureCameraAny) ?? false;

static async Task RotateImageInPlace(string filePath, MediaPickerOptions options)
{
await using var inputStream = File.OpenRead(filePath);
var fileName = System.IO.Path.GetFileName(filePath);
await using var rotatedStream = await ImageProcessor.RotateImageAsync(inputStream, fileName);
rotatedStream.Position = 0;
inputStream.Dispose(); // explicit close before delete
try
{ File.Delete(filePath); }
catch { }
await using var outputStream = File.Create(filePath);
await rotatedStream.CopyToAsync(outputStream);
}

internal static async Task<string> ProcessPhotoAsync(string imagePath, MediaPickerOptions options)
{
// Apply rotation if needed for photos
if (imagePath is not null && ImageProcessor.IsRotationNeeded(options))
{
await RotateImageInPlace(imagePath, options);
}

// Apply compression/resizing if needed for photos
if (imagePath is not null && ImageProcessor.IsProcessingNeeded(options?.MaximumWidth, options?.MaximumHeight, options?.CompressionQuality ?? 100))
{
imagePath = await CompressImageIfNeeded(imagePath, options);
}

return imagePath;
}

internal static async Task<string> ProcessPhotoPreservingSourceAsync(string imagePath, PersistedPhotoProcessingOptions options)
{
if (imagePath is null)
{
return null;
}
internal static Task<string> ProcessPhotoAsync(string imagePath, MediaPickerOptions options)
=> ProcessImage(imagePath, options);

var originalImagePath = imagePath;
string rotatedImagePath = null;

// Recovery-sensitive MediaPicker paths must leave the original file intact until the
// active recovery record has been cleared or promoted.
if (options.RotateImage)
{
var rotatedPath = await RotateImageToNewFileAsync(imagePath);
if (!string.Equals(rotatedPath, imagePath, StringComparison.Ordinal))
{
rotatedImagePath = rotatedPath;
}

imagePath = rotatedPath;
}

if (ImageProcessor.IsProcessingNeeded(options.MaximumWidth, options.MaximumHeight, options.CompressionQuality))
{
var compressedImagePath = await CompressImageIfNeeded(imagePath, options, preserveSource: true);
if (ShouldDeleteIntermediateFile(rotatedImagePath, originalImagePath, compressedImagePath))
{
TryDeleteFile(rotatedImagePath);
}

imagePath = compressedImagePath;
}

return imagePath;
}
// Recovery-sensitive MediaPicker paths must leave the original file intact until the
// active recovery record has been cleared or promoted, so they opt out of the
// MAUI-owned input cleanup that ProcessImage otherwise performs.
internal static Task<string> ProcessPhotoPreservingSourceAsync(string imagePath, PersistedPhotoProcessingOptions options)
=> ProcessImage(imagePath, options, preserveSource: true);

internal static PersistedPhotoProcessingOptions GetPhotoProcessingOptions(MediaPickerOptions options)
=> new(
Expand Down Expand Up @@ -279,15 +220,8 @@ void OnResult(Intent intent)
{
if (photo)
{
// Apply rotation if needed
if (ImageProcessor.IsRotationNeeded(options))
await RotateImageInPlace(path, options);

// Apply compression/resizing if needed
if (ImageProcessor.IsProcessingNeeded(options?.MaximumWidth, options?.MaximumHeight, options?.CompressionQuality ?? 100))
{
path = await CompressImageIfNeeded(path, options);
}
// Apply rotation and/or compression if needed
path = await ProcessImage(path, options);
}

return new FileResult(path);
Expand Down Expand Up @@ -428,123 +362,6 @@ void OnCreate(Intent intent)
return captureFile.AbsolutePath;
}

static async Task<string> RotateImageToNewFileAsync(string imagePath)
{
if (string.IsNullOrEmpty(imagePath))
{
return imagePath;
}

if (!File.Exists(imagePath))
{
return imagePath;
}

await using var inputStream = File.OpenRead(imagePath);
var inputFileName = System.IO.Path.GetFileName(imagePath);
await using var rotatedStream = await ImageProcessor.RotateImageAsync(inputStream, inputFileName);
rotatedStream.Position = 0;

var outputExtension = System.IO.Path.GetExtension(imagePath);
if (string.IsNullOrEmpty(outputExtension))
{
outputExtension = FileExtensions.Jpg;
}

var outputFile = FileSystemUtils.GetTemporaryFile(Application.Context.CacheDir, Guid.NewGuid().ToString("N") + outputExtension);
await using var outputStream = File.Create(outputFile.AbsolutePath);
await rotatedStream.CopyToAsync(outputStream);

return outputFile.AbsolutePath;
}

static bool ShouldDeleteIntermediateFile(string intermediatePath, string originalPath, string finalPath)
=> !string.IsNullOrEmpty(intermediatePath) &&
!string.Equals(intermediatePath, originalPath, StringComparison.Ordinal) &&
!string.Equals(intermediatePath, finalPath, StringComparison.Ordinal);

static void TryDeleteFile(string filePath)
{
try
{ File.Delete(filePath); }
catch { }
}

static Task<string> CompressImageIfNeeded(string imagePath, MediaPickerOptions options, bool preserveSource = false)
=> CompressImageIfNeeded(imagePath, GetPhotoProcessingOptions(options), preserveSource);

static async Task<string> CompressImageIfNeeded(string imagePath, PersistedPhotoProcessingOptions options, bool preserveSource = false)
{
if (!ImageProcessor.IsProcessingNeeded(options.MaximumWidth, options.MaximumHeight, options.CompressionQuality) || string.IsNullOrEmpty(imagePath))
return imagePath;

try
{
var originalFile = new Java.IO.File(imagePath);
if (!originalFile.Exists())
{
return imagePath;
}

// Use ImageProcessor for unified image processing
using var inputStream = File.OpenRead(imagePath);
var inputFileName = System.IO.Path.GetFileName(imagePath);
using var processedStream = await ImageProcessor.ProcessImageAsync(
inputStream,
options.MaximumWidth,
options.MaximumHeight,
options.CompressionQuality,
inputFileName,
options.RotateImage,
options.PreserveMetaData);

if (processedStream != null)
{
// Determine the correct output extension based on the processed format
processedStream.Position = 0;
var outputExtension = ImageProcessor.DetermineOutputExtension(processedStream, options.CompressionQuality, inputFileName);
var originalExtension = System.IO.Path.GetExtension(imagePath);

// If format changed (e.g., PNG -> JPEG), use new extension
string outputPath = imagePath;
if (!string.Equals(outputExtension, originalExtension, StringComparison.OrdinalIgnoreCase))
{
outputPath = System.IO.Path.ChangeExtension(imagePath, outputExtension);
}

if (preserveSource)
{
var outputFile = FileSystemUtils.GetTemporaryFile(Application.Context.CacheDir, Guid.NewGuid().ToString("N") + outputExtension);
outputPath = outputFile.AbsolutePath;
}
else
{
// Delete original file first
try
{ originalFile.Delete(); }
catch { }
}

// Write processed image to output path with correct extension
using var outputStream = File.Create(outputPath);
processedStream.Position = 0;
await processedStream.CopyToAsync(outputStream);

return outputPath;
}

// If ImageProcessor returns null (e.g., on .NET Standard), ImageProcessor.IsProcessingNeeded would have returned false,
// so we shouldn't reach this point. Return original path as fallback.
return imagePath;
}
catch
{
// If processing fails, return original path
}

return imagePath;
}

async Task<string> CaptureVideoAsync(Intent captureIntent)
{
// On Android 12 (API 31-32), the camera app creates the video in MediaStore as a pending item.
Expand Down Expand Up @@ -761,17 +578,8 @@ void OnResult(Intent resultIntent)

foreach (var path in tempResultList)
{
string processedPath = path;

// Apply rotation if needed
if (ImageProcessor.IsRotationNeeded(options))
await RotateImageInPlace(processedPath, options);

// Apply compression/resizing if needed
if (ImageProcessor.IsProcessingNeeded(options?.MaximumWidth, options?.MaximumHeight, options?.CompressionQuality ?? 100))
{
processedPath = await CompressImageIfNeeded(processedPath, options);
}
// Apply rotation and/or compression if needed
var processedPath = await ProcessImage(path, options);

resultList.Add(new FileResult(processedPath));
}
Expand All @@ -784,5 +592,110 @@ void OnResult(Intent resultIntent)
return [];
}
}

internal static Task<string> ProcessImage(string imagePath, MediaPickerOptions options)
=> ProcessImage(imagePath, GetPhotoProcessingOptions(options));

internal static async Task<string> ProcessImage(string imagePath, PersistedPhotoProcessingOptions options, bool preserveSource = false)

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[minor] Public API Surface (internal naming) — ProcessImage is async Task<string> but lacks the Async suffix used by every neighbouring member (ProcessPhotoAsync, ProcessPhotoPreservingSourceAsync, CapturePhotoAsync). It is internal so this is not a shipped-API concern, but it is also the seam the new device tests bind to, so the name will be sticky. Suggest ProcessImageAsync for both overloads.

{
if (string.IsNullOrEmpty(imagePath) || !File.Exists(imagePath))
return imagePath;

var needsRotation = options.RotateImage;
var needsProcessing = ImageProcessor.IsProcessingNeeded(options.MaximumWidth, options.MaximumHeight, options.CompressionQuality);

// Nothing to do - return the picked path untouched.
if (!needsRotation && !needsProcessing)
return imagePath;

try
{
var inputFileName = System.IO.Path.GetFileName(imagePath);

// Compose the transforms without mutating the input: rotation and compression are
// chained through in-memory streams and only the final result is written - once - to
// a new MAUI-owned cache file (GetTemporaryFile preserves the original filename, see
// #33258). External/user-owned sources (the gallery original, an SD card, another
// app's storage) are therefore never deleted or overwritten. If the input was itself
// a MAUI-owned temporary cache file (a camera capture, or a content:// URI that
// EnsurePhysicalPath copied into our cache), it is removed after the output is written
// (below) so we don't leave an orphaned duplicate behind - unless the caller opted
// into preserving it because a recovery record still points at it.
Stream currentStream = File.OpenRead(imagePath);
try
{
if (needsRotation)
{
var rotatedStream = await ImageProcessor.RotateImageAsync(currentStream, inputFileName);

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[minor] Performance-Critical Path / memory — The rotated MemoryStream (a full in-memory copy of the encoded image, produced by ImageProcessor.RotateImageAsync) is now kept alive as the input to ProcessImageAsync and only disposed at line 648, i.e. it coexists with ProcessImageAsync's own byte[] copy, the decoded Bitmap and the output MemoryStream. The removed flow round-tripped the rotated image through disk and re-opened it as a FileStream, so the encoded copy was not resident during decode/encode. Concrete scenario: RotateImage = true + CompressionQuality = 50 on a 12 MP camera JPEG on a low-RAM device raises peak managed memory by roughly one full encoded-image buffer, in a code path where Android OutOfMemoryError is already a known MediaPicker failure mode. Not a blocker, but worth a comment or an explicit disposal of the rotated buffer once ProcessImageAsync has consumed it.

currentStream.Dispose();
currentStream = rotatedStream;
currentStream.Position = 0;
}

if (needsProcessing)
{
var processedStream = await ImageProcessor.ProcessImageAsync(
currentStream,
options.MaximumWidth,
options.MaximumHeight,
options.CompressionQuality,
inputFileName,
false, // rotation, if any, has already been applied above
options.PreserveMetaData);

if (processedStream is not null)

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — When ImageProcessor.ProcessImageAsync returns null the code silently falls through: currentStream is still the unprocessed (only-rotated, or raw file) stream, it is written to a brand-new cache file, and — because preserveSource is false in the pick/capture flows — the MAUI-owned input is then deleted at line 677-684. The removed CompressImageIfNeeded explicitly handled this case by returning imagePath untouched ("If ImageProcessor returns null ... Return original path as fallback"). Concrete scenario: any future/partial-platform path where ProcessImageAsync returns null, the caller receives a file that was never resized/compressed while the original owned temp copy has already been deleted, so the untouched original is no longer recoverable. Suggest else { return imagePath; } (or skip the write/delete) when processedStream is null and needsProcessing was requested.

{
currentStream.Dispose();
currentStream = processedStream;
currentStream.Position = 0;
}
}

// Preserve the original filename (see #33258), but honor a format change
// (e.g. PNG -> JPEG) by swapping the extension.
var outputExtension = ImageProcessor.DetermineOutputExtension(currentStream, options.CompressionQuality, inputFileName);

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — Rotation-only picks can be written out under a wrong extension, a regression versus the old in-place behavior.

Concrete scenario: PickPhotoAsync(new MediaPickerOptions { RotateImage = true }) on a HEIC/WEBP/GIF/BMP source.

  • needsProcessing is false (default CompressionQuality = 100, no max dimensions), so only RotateImageAsync runs.
  • ImageProcessor.RotateImageAsync returns new MemoryStream(bytes) — the original, unmodified bytes — whenever EXIF orientation is 1 (the common case) or the bitmap fails to decode/compress (ImageProcessor.android.cs lines 41, 49, 57, 95, 103).
  • ImageProcessor.DetectImageFormat only recognizes PNG and JPEG magic numbers (ImageProcessor.shared.cs L285-295) and returns null for those bytes.
  • DetermineOutputExtension then falls through to the quality heuristic qualityPercent >= 95 ? ".png" : ".jpg" (L258). With quality 100 this yields ".png", so line 659-661 renames IMG_1234.heic → IMG_1234.png and line 664-671 writes the HEIC bytes into a file named .png.

Previously RotateImageInPlace wrote back to the same path, so the extension always survived. Consumers that dispatch on FileResult.FileName/ContentType (or Image.Source = ImageSource.FromFile(...)) now get a mislabeled file. A .jpeg source is likewise silently renamed to .jpg.

Suggested fix: only change the extension when DetectImageFormat actually returned a format (i.e. when the bytes were genuinely re-encoded), and otherwise keep inputFileName unchanged.

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

⚠️ Warning — Logic and Correctness / Image Handling: the rotate-only path now renames the output extension, and can mislabel the file.

When needsRotation == true and needsProcessing == false (RotateImage = true, CompressionQuality = 100, no max width/height), currentStream here is whatever ImageProcessor.RotateImageAsync returned. That method returns the original bytes unchanged (new MemoryStream(bytes)) whenever EXIF orientation is 1, the bitmap fails to decode, or the re-compress fails (ImageProcessor.android.cs lines 41, 50, 57, 95, 104). DetermineOutputExtension then runs DetectImageFormat, which only recognizes PNG and JPEG magic numbers and returns null for anything else, so it falls through to the quality heuristic — and with CompressionQuality == 100 (>= 95) it returns ".png".

Concrete scenario: user picks a modern-device IMG_1234.heic (or .webp) that is already upright (orientation 1) with new MediaPickerOptions { RotateImage = true }. The bytes are copied through untouched, but line 661 renames the output to IMG_1234.png while the content is still HEIC/WebP. The returned FileResult.FullPath now has an extension that does not match its content, which breaks consumers that dispatch on extension or MIME type derived from it.

This exposure is new to this PR: before the change, DetermineOutputExtension was only reached from CompressImageIfNeeded, where the stream was always a MAUI Graphics ImageFormat.Png/ImageFormat.Jpeg encode, so detection always succeeded. Rotation was in-place and never renamed. Consider only applying the extension swap when DetectImageFormat positively identified a format (or skipping the rename when the rotation pipeline returned the source bytes verbatim).

var originalExtension = System.IO.Path.GetExtension(inputFileName);
var outputFileName = inputFileName;
if (!string.Equals(outputExtension, originalExtension, StringComparison.OrdinalIgnoreCase))
{
outputFileName = System.IO.Path.ChangeExtension(inputFileName, outputExtension);
}

var outputFile = FileSystemUtils.GetTemporaryFile(Application.Context.CacheDir, outputFileName);
Comment thread
mattleibow marked this conversation as resolved.

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Android Platform / Cross-Platform Consistency — The terminal output is always written to Application.Context.CacheDir (internal storage), but the input may have been resolved into external cache: FileSystemUtils.EnsurePhysicalPath picks Application.Context.ExternalCacheDir when WRITE_EXTERNAL_STORAGE is declared (FileSystemUtils.android.cs:278-282). Before this PR the compression step rewrote the file in place, so a large picked image stayed on the same volume. Now every processed image is copied onto internal storage, and the owned external-cache input is deleted (line 677-684), silently migrating multi-MB photos to the internal partition — a real problem for multi-pick of many large photos on devices with a small data partition. Consider deriving the output root from the input (external-cache input → external cache), mirroring the root selection EnsurePhysicalPath already performs.

var outputPath = outputFile.AbsolutePath;

using (var outputStream = File.Create(outputPath))
{
currentStream.Position = 0;
await currentStream.CopyToAsync(outputStream);
}

// The output is now fully written and closed. If the input was a MAUI-owned
// temporary cache file, delete it so we don't accumulate an orphaned duplicate.
// External/user-owned sources are never under our cache folder, so they are left
// untouched. Deleting only after the output is written avoids any data-loss window.
if (!preserveSource &&

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Memory Leak Prevention / cache growth — Deleting only the input file leaves its containing directory behind forever.

FileSystemUtils.GetTemporaryFile creates <cache>/2203693cc04e0be7f4f024d5f9499e13/<new-guid>/<name> for every call (FileSystemUtils.android.cs L43-46). This PR now allocates one such GUID directory per processed image (line 664) in addition to the one the input already occupied, and the File.Delete(imagePath) here removes the file but not its parent GUID directory. Java.IO.File.DeleteOnExit() is effectively a no-op on Android (the process is killed, not exited normally), so these empty directories accumulate for the app's lifetime.

Before this change the non-preserving path overwrote in place and created zero new directories; now each pick/capture leaves at least one empty directory behind. Consider deleting the parent directory when it is empty and confirmed under the owned root (reuse IsMauiOwnedTemporaryFile for the containment check).

FileSystemUtils.IsMauiOwnedTemporaryFile(imagePath) &&
!string.Equals(imagePath, outputPath, StringComparison.Ordinal))
{
try
{ File.Delete(imagePath); }

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention / disk hygiene — File.Delete(imagePath) removes only the file, not the unique GUID directory GetTemporaryFile created for it (FileSystemUtils.android.cs:44-47 makes <cache>/<hash>/<guid>/ per call). Combined with line 664, every processed image now (a) creates one new GUID directory and (b) leaves the input's now-empty GUID directory behind forever — DeleteOnExit is a JVM shutdown hook that Android processes almost never run. Concrete scenario: PickPhotosAsync with 50 photos and CompressionQuality = 50 leaves 50 empty stale directories plus 50 new ones under <cache>/2203693cc04e0be7f4f024d5f9499e13/, repeated on every pick. Before the PR the non-recovery flows overwrote in place and created no new directory. Suggest deleting the input's parent directory when it is an owned GetTemporaryFile wrapper directory (i.e. it is empty after the delete).

catch { }
}

return outputPath;
}
finally
{
currentStream.Dispose();
}
}
catch

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Null Safety and Defensive Coding — A failure after File.Create(outputPath) (line 667) leaves a truncated/zero-byte orphan in the cache.

If CopyToAsync throws (disk full, IOException, cancellation) the blanket catch swallows it and returns imagePath, which is correct for the caller, but outputPath has already been created and is never cleaned up. Because the output now lives in its own GUID directory it will never be overwritten or reclaimed. Track outputPath outside the try and best-effort delete it (and its directory) in the catch.

@MauiBot MauiBot Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Null Safety / Defensive Coding — This blanket catch { } is now much wider than what it replaced. (1) Rotation failures previously propagated (RotateImageInPlace had no catch); they are now swallowed, so a picked photo can silently come back un-rotated with no diagnostic. The recovery code in this same feature uses Trace.WriteLine for exactly this situation (MediaPickerRecoveryManager.android.cs:675) — add a catch (Exception ex) with Trace.WriteLine. (2) If the failure happens inside the File.Create/CopyToAsync block (lines 667-671, e.g. IOException on a full cache partition), a truncated output file is left at outputPath in the cache while the method returns imagePath; nothing ever deletes it. Suggest tracking outputPath outside the inner try and best-effort deleting a partially written output in the catch.

{
// If processing fails, leave the picked source untouched and return its path.
}

return imagePath;
}
}
}
Loading
Loading