Skip to content
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.

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

[image_picker_android] Adjust file extension in FileUtils when it does not match its mime type #3409

Merged
4 changes: 4 additions & 0 deletions packages/image_picker/image_picker_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.8.6+2

* Fixes case when file extension returned from the OS does not match its real mime type.

## 0.8.6+1

* Refactors code in preparation for adopting Pigeon.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ class FileUtils {
* <p>Each file is placed in its own directory to avoid conflicts according to the following
* scheme: {cacheDir}/{randomUuid}/{fileName}
*
* <p>File extension is changed to match MIME type of the file, if known. Otherwise, the extension
* is left unchanged.
*
* <p>If the original file name is unknown, a predefined "image_picker" filename is used and the
* file extension is deduced from the mime type (with fallback to ".jpg" in case of failure).
*/
Expand All @@ -57,9 +60,14 @@ String getPathFromUri(final Context context, final Uri uri) {
// just clear the picked files after the app startup.
targetDirectory.deleteOnExit();
String fileName = getImageName(context, uri);
String extension = getImageExtension(context, uri);

if (fileName == null) {
Log.w("FileUtils", "Cannot get file name for " + uri);
fileName = "image_picker" + getImageExtension(context, uri);
if (extension == null) extension = ".jpg";
fileName = "image_picker" + extension;
} else if (extension != null) {
fileName = getBaseName(fileName) + extension;
}
File file = new File(targetDirectory, fileName);
try (OutputStream outputStream = new FileOutputStream(file)) {
Expand All @@ -74,7 +82,7 @@ String getPathFromUri(final Context context, final Uri uri) {
}
}

/** @return extension of image with dot, or default .jpg if it none. */
/** @return extension of image with dot, or null if it's empty. */
private static String getImageExtension(Context context, Uri uriImage) {
String extension;

Expand All @@ -88,12 +96,11 @@ private static String getImageExtension(Context context, Uri uriImage) {
Uri.fromFile(new File(uriImage.getPath())).toString());
}
} catch (Exception e) {
extension = null;
return null;
}

if (extension == null || extension.isEmpty()) {
//default extension for matches the previous behavior of the plugin
extension = "jpg";
return null;
}

return "." + extension;
Expand Down Expand Up @@ -121,4 +128,9 @@ private static void copy(InputStream in, OutputStream out) throws IOException {
}
out.flush();
}

private static String getBaseName(String fileName) {
// Basename is everything before the last '.'.
return fileName.substring(0, fileName.lastIndexOf('.'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import android.database.MatrixCursor;
import android.net.Uri;
import android.provider.MediaStore;
import android.webkit.MimeTypeMap;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.test.core.app.ApplicationProvider;
Expand All @@ -29,6 +30,7 @@
import org.robolectric.Robolectric;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.shadows.ShadowContentResolver;
import org.robolectric.shadows.ShadowMimeTypeMap;

@RunWith(RobolectricTestRunner.class)
public class FileUtilTest {
Expand All @@ -42,6 +44,10 @@ public void before() {
context = ApplicationProvider.getApplicationContext();
shadowContentResolver = shadowOf(context.getContentResolver());
fileUtils = new FileUtils();
ShadowMimeTypeMap mimeTypeMap = shadowOf(MimeTypeMap.getSingleton());
mimeTypeMap.addExtensionMimeTypMapping("jpg", "image/jpeg");
mimeTypeMap.addExtensionMimeTypMapping("png", "image/png");
mimeTypeMap.addExtensionMimeTypMapping("webp", "image/webp");
}

@Test
Expand Down Expand Up @@ -74,15 +80,38 @@ public void FileUtil_getImageExtension() throws IOException {

@Test
public void FileUtil_getImageName() throws IOException {
Uri uri = Uri.parse("content://dummy/dummy.png");
Uri uri = MockContentProvider.PNG_URI;
Robolectric.buildContentProvider(MockContentProvider.class).create("dummy");
shadowContentResolver.registerInputStream(
uri, new ByteArrayInputStream("imageStream".getBytes(UTF_8)));
String path = fileUtils.getPathFromUri(context, uri);
assertTrue(path.endsWith("a.b.png"));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add another test for the case where a file's basename contains .s.

Copy link
Contributor

Choose a reason for hiding this comment

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

You created a new codepath in this PR for the case where the filename is non-null but the extension is null; that case needs a test as well.

@Test
public void FileUtil_getImageName_mismatchedType() throws IOException {
Uri uri = MockContentProvider.WEBP_URI;
Robolectric.buildContentProvider(MockContentProvider.class).create("dummy");
shadowContentResolver.registerInputStream(
uri, new ByteArrayInputStream("imageStream".getBytes(UTF_8)));
String path = fileUtils.getPathFromUri(context, uri);
assertTrue(path.endsWith("dummy.png"));
assertTrue(path.endsWith("c.d.webp"));
}

@Test
public void FileUtil_getImageName_unknownType() throws IOException {
Uri uri = MockContentProvider.UNKNOWN_URI;
Robolectric.buildContentProvider(MockContentProvider.class).create("dummy");
shadowContentResolver.registerInputStream(
uri, new ByteArrayInputStream("imageStream".getBytes(UTF_8)));
String path = fileUtils.getPathFromUri(context, uri);
assertTrue(path.endsWith("e.f.g"));
}

private static class MockContentProvider extends ContentProvider {
public static final Uri PNG_URI = Uri.parse("content://dummy/a.b.png");
public static final Uri WEBP_URI = Uri.parse("content://dummy/c.d.png");
public static final Uri UNKNOWN_URI = Uri.parse("content://dummy/e.f.g");

@Override
public boolean onCreate() {
Expand All @@ -98,14 +127,16 @@ public Cursor query(
@Nullable String[] selectionArgs,
@Nullable String sortOrder) {
MatrixCursor cursor = new MatrixCursor(new String[] {MediaStore.MediaColumns.DISPLAY_NAME});
cursor.addRow(new Object[] {"dummy.png"});
cursor.addRow(new Object[] {uri.getLastPathSegment()});
return cursor;
}

@Nullable
@Override
public String getType(@NonNull Uri uri) {
return "image/png";
if (uri.equals(PNG_URI)) return "image/png";
if (uri.equals(WEBP_URI)) return "image/webp";
return null;
}

@Nullable
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Android implementation of the image_picker plugin.
repository: https://github.com/flutter/packages/tree/main/packages/image_picker/image_picker_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22

version: 0.8.6+1
version: 0.8.6+2

environment:
sdk: ">=2.17.0 <3.0.0"
Expand Down