From 62ab7b765a79a4d5193d7309f152cd318a1948b3 Mon Sep 17 00:00:00 2001 From: Mido Date: Sun, 24 Nov 2024 19:08:50 +0100 Subject: [PATCH 01/12] imageCaptureProcessing maintain EXIF Change requests change request SuppressLint SuppressLint imageCaptureProcessing.java --- .../components/ImageCaptureProcessing.java | 8 +- app/src/org/commcare/utils/FileUtil.java | 93 +++++++++++++++++-- 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java index 003c456c91..6372656a30 100644 --- a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java +++ b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java @@ -36,6 +36,7 @@ public class ImageCaptureProcessing { * @return A pair containing raw image and scaled imagePath. The first entry is the raw image * while the second one is path to scaled image. */ + @SuppressLint("ExifInterface") private static Pair moveAndScaleImage(File originalImage, boolean shouldScale, String instanceFolder, FormEntryActivity formEntryActivity) throws IOException { @@ -56,7 +57,8 @@ private static Pair moveAndScaleImage(File originalImage, boolean if (currentWidget != null) { int maxDimen = currentWidget.getMaxDimen(); if (maxDimen != -1) { - savedScaledImage = FileUtil.scaleAndSaveImage(originalImage, tempFilePathForScaledImage, maxDimen); + File tempFile = new File(tempFilePathForScaledImage); + savedScaledImage = FileUtil.scaleAndSaveImageWithExif(originalImage, tempFile, maxDimen); } } } @@ -83,6 +85,7 @@ private static Pair moveAndScaleImage(File originalImage, boolean return new Pair<>(rawImageFile, finalFilePath); } + @SuppressLint("ExifInterface") private static File makeRawCopy(File originalImage, String instanceFolder, String imageFilename) throws IOException { String rawDirPath = getRawDirectoryPath(instanceFolder); @@ -92,7 +95,7 @@ private static File makeRawCopy(File originalImage, String instanceFolder, Strin } File rawImageFile = new File(rawDirPath + "/" + imageFilename); try { - FileUtil.copyFile(originalImage, rawImageFile); + FileUtil.copyFileWithExifData(originalImage, rawImageFile); } catch (Exception e) { throw new IOException("Failed to rename " + originalImage.getAbsolutePath() + " to " + rawImageFile.getAbsolutePath()); @@ -208,6 +211,7 @@ private static void processImageGivenFilePath(FormEntryActivity activity, String } } + @SuppressLint("ExifInterface") private static boolean scaleAndSaveImage(File originalImage, boolean shouldScale, String instanceFolder, FormEntryActivity activity) throws IOException { Pair rawImageAndScaledPath = moveAndScaleImage(originalImage, shouldScale, instanceFolder, activity); diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 002bba8b99..1ff2188735 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -7,6 +7,7 @@ import android.content.Intent; import android.database.Cursor; import android.graphics.Bitmap; +import android.media.ExifInterface; import android.media.MediaMetadataRetriever; import android.net.Uri; import android.os.Build; @@ -356,17 +357,16 @@ public static String getGlobalStringUri(String fileLocation) { } public static void checkReferenceURI(Resource r, String URI, Vector problems) throws InvalidReferenceException { - Reference mRef = ReferenceManager.instance().DeriveReference(URI); + Reference mRef = ReferenceManager.instance().DeriveReference(uri); String mLocalReference = mRef.getLocalURI(); try { if (!mRef.doesBinaryExist()) { - problems.addElement(new MissingMediaException(r, "Missing external media: " + mLocalReference, URI, - MissingMediaException.MissingMediaExceptionType.FILE_NOT_FOUND)); + throw new InvalidReferenceException("Missing external media: " + mLocalReference); } } catch (IOException e) { - problems.addElement(new MissingMediaException(r, "Problem reading external media: " + mLocalReference, URI, - MissingMediaException.MissingMediaExceptionType.FILE_NOT_ACCESSIBLE)); + throw new InvalidReferenceException("Problem reading external media: " + mLocalReference); } + return mRef; } public static boolean referenceFileExists(String uri) { @@ -494,8 +494,9 @@ public static String getMd5Hash(File file) { BigInteger number = new BigInteger(1, messageDigest); String md5 = number.toString(16); - while (md5.length() < 32) + while (md5.length() < 32) { md5 = "0" + md5; + } is.close(); return md5; @@ -612,7 +613,8 @@ public static boolean scaleAndSaveImage(File originalImage, String finalFilePath Pair bitmapAndScaledBool = MediaUtil.inflateImageSafe(originalImage.getAbsolutePath()); if (bitmapAndScaledBool.second) { Logger.log(LogTypes.TYPE_FORM_ENTRY, - "An image captured during form entry was too large to be processed at its original size, and had to be downsized"); + "An image captured during form entry was too large to be processed at its original size, " + + "and had to be downsized"); } Bitmap scaledBitmap = getBitmapScaledByMaxDimen(bitmapAndScaledBool.first, maxDimen); if (scaledBitmap != null) { @@ -723,7 +725,9 @@ public static Uri getUriForExternalFile(Context context, File file) { * @param dstFile destination File where we need to copy the inputStream */ public static void copyFile(InputStream inputStream, File dstFile) throws IOException { - if (inputStream == null) return; + if (inputStream == null) { + return; + } OutputStream outputStream = new FileOutputStream(dstFile); StreamsUtil.writeFromInputToOutputUnmanaged(inputStream, outputStream); inputStream.close(); @@ -844,8 +848,13 @@ public static void addMediaToGallery(Context context, File file) throws } /** - * Returns true only when we're certain that the file size is too large. - *

https://developer.android.com/training/secure-file-sharing/retrieve-info.html#RetrieveFileInfo + * Checks if a file referenced by a content URI exceeds the maximum allowed upload size + *

+ * @param contentResolver ContentResolver to query the file size + * @param uri Content URI of the file to check + * @return true if the file size exceeds FormUploadUtil.MAX_BYTES, false otherwise or if size cannot be determined + * @see FormUploadUtil#MAX_BYTES + * @see Android docs */ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Uri uri) { try (Cursor returnCursor = contentResolver.query(uri, null, null, null, null)) { @@ -857,4 +866,68 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur return returnCursor.getLong(sizeIndex) > FormUploadUtil.MAX_BYTES; } } + + @SuppressLint("ExifInterface") + public static void copyFileWithExifData(File sourceFile, File destFile) throws IOException { + // First copy the file normally + copyFile(sourceFile, destFile); + + // Then copy EXIF data + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + } + + @SuppressLint("ExifInterface") + public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { + // First scale the image + boolean scaled = scaleAndSaveImage(sourceFile, destFile.getAbsolutePath(), maxDimen); + + if (scaled) { + // Copy EXIF data from source to scaled image + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + } + + return scaled; + } + + @SuppressLint("ExifInterface") + private static void copyExifData(String sourcePath, String destPath) throws IOException { + ExifInterface source = new ExifInterface(sourcePath); + ExifInterface dest = new ExifInterface(destPath); + + String[] tagsToPreserve = { + // GPS data + ExifInterface.TAG_GPS_LATITUDE, + ExifInterface.TAG_GPS_LATITUDE_REF, + ExifInterface.TAG_GPS_LONGITUDE, + ExifInterface.TAG_GPS_LONGITUDE_REF, + ExifInterface.TAG_GPS_TIMESTAMP, + ExifInterface.TAG_GPS_DATESTAMP, + ExifInterface.TAG_GPS_ALTITUDE, + ExifInterface.TAG_GPS_ALTITUDE_REF, + ExifInterface.TAG_GPS_AREA_INFORMATION, + + // Timestamp data + ExifInterface.TAG_DATETIME, + ExifInterface.TAG_DATETIME_DIGITIZED, + ExifInterface.TAG_DATETIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME, + ExifInterface.TAG_OFFSET_TIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME_DIGITIZED, + + // Image metadata + ExifInterface.TAG_COPYRIGHT, + ExifInterface.TAG_IMAGE_DESCRIPTION, + ExifInterface.TAG_EXIF_VERSION, + ExifInterface.TAG_ORIENTATION + }; + + for (String tag : tagsToPreserve) { + String value = source.getAttribute(tag); + if (value != null) { + dest.setAttribute(tag, value); + } + } + + dest.saveAttributes(); + } } From f4f9580af68cfb8f95697ab0ac1a89f5660b0777 Mon Sep 17 00:00:00 2001 From: Mido Date: Mon, 25 Nov 2024 14:07:48 +0100 Subject: [PATCH 02/12] reverted some changes --- app/src/org/commcare/utils/FileUtil.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 1ff2188735..12326aa84b 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -357,16 +357,17 @@ public static String getGlobalStringUri(String fileLocation) { } public static void checkReferenceURI(Resource r, String URI, Vector problems) throws InvalidReferenceException { - Reference mRef = ReferenceManager.instance().DeriveReference(uri); + Reference mRef = ReferenceManager.instance().DeriveReference(URI); String mLocalReference = mRef.getLocalURI(); try { if (!mRef.doesBinaryExist()) { - throw new InvalidReferenceException("Missing external media: " + mLocalReference); + problems.addElement(new MissingMediaException(r, "Missing external media: " + mLocalReference, URI, + MissingMediaException.MissingMediaExceptionType.FILE_NOT_FOUND)); } } catch (IOException e) { - throw new InvalidReferenceException("Problem reading external media: " + mLocalReference); + problems.addElement(new MissingMediaException(r, "Problem reading external media: " + mLocalReference, URI, + MissingMediaException.MissingMediaExceptionType.FILE_NOT_ACCESSIBLE)); } - return mRef; } public static boolean referenceFileExists(String uri) { From 88e99944c8b31e438ebfa71bcdf1874d58d28c5f Mon Sep 17 00:00:00 2001 From: Mido Date: Mon, 25 Nov 2024 14:22:07 +0100 Subject: [PATCH 03/12] remove surpresslint --- .../activities/components/ImageCaptureProcessing.java | 3 --- app/src/org/commcare/utils/FileUtil.java | 4 ---- 2 files changed, 7 deletions(-) diff --git a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java index 6372656a30..6f012311cf 100644 --- a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java +++ b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java @@ -36,7 +36,6 @@ public class ImageCaptureProcessing { * @return A pair containing raw image and scaled imagePath. The first entry is the raw image * while the second one is path to scaled image. */ - @SuppressLint("ExifInterface") private static Pair moveAndScaleImage(File originalImage, boolean shouldScale, String instanceFolder, FormEntryActivity formEntryActivity) throws IOException { @@ -85,7 +84,6 @@ private static Pair moveAndScaleImage(File originalImage, boolean return new Pair<>(rawImageFile, finalFilePath); } - @SuppressLint("ExifInterface") private static File makeRawCopy(File originalImage, String instanceFolder, String imageFilename) throws IOException { String rawDirPath = getRawDirectoryPath(instanceFolder); @@ -211,7 +209,6 @@ private static void processImageGivenFilePath(FormEntryActivity activity, String } } - @SuppressLint("ExifInterface") private static boolean scaleAndSaveImage(File originalImage, boolean shouldScale, String instanceFolder, FormEntryActivity activity) throws IOException { Pair rawImageAndScaledPath = moveAndScaleImage(originalImage, shouldScale, instanceFolder, activity); diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 12326aa84b..8b6bb154d7 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -398,7 +398,6 @@ public static void ensureFilePathExists(File f) { * if we are on KitKat we need use the new API to find the mounted roots, then append our application * specific path that we're allowed to write to */ - @SuppressLint("NewApi") private static String getExternalDirectoryKitKat(Context c) { File[] extMounts = c.getExternalFilesDirs(null); // first entry is emualted storage. Second if it exists is secondary (real) SD. @@ -868,7 +867,6 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur } } - @SuppressLint("ExifInterface") public static void copyFileWithExifData(File sourceFile, File destFile) throws IOException { // First copy the file normally copyFile(sourceFile, destFile); @@ -877,7 +875,6 @@ public static void copyFileWithExifData(File sourceFile, File destFile) throws I copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); } - @SuppressLint("ExifInterface") public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { // First scale the image boolean scaled = scaleAndSaveImage(sourceFile, destFile.getAbsolutePath(), maxDimen); @@ -890,7 +887,6 @@ public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, return scaled; } - @SuppressLint("ExifInterface") private static void copyExifData(String sourcePath, String destPath) throws IOException { ExifInterface source = new ExifInterface(sourcePath); ExifInterface dest = new ExifInterface(destPath); From 6016f4a34eed71d82e0aaec498bb6f255454dff5 Mon Sep 17 00:00:00 2001 From: Mido Date: Thu, 28 Nov 2024 19:25:10 +0100 Subject: [PATCH 04/12] Media tests --- app/.classpath | 9 +- app/.project | 23 +++- .../androidTests/MediaEncryptionTest.kt | 14 +++ app/src/org/commcare/utils/FileUtil.java | 112 +++++++++++------- 4 files changed, 105 insertions(+), 53 deletions(-) diff --git a/app/.classpath b/app/.classpath index 6c6bd1ecbc..3a6c280f3e 100644 --- a/app/.classpath +++ b/app/.classpath @@ -1,12 +1,9 @@ - - - - - + + - + diff --git a/app/.project b/app/.project index 9958d0b972..8ba08befd3 100644 --- a/app/.project +++ b/app/.project @@ -6,17 +6,22 @@ - com.android.ide.eclipse.adt.ResourceManagerBuilder + org.eclipse.jdt.core.javabuilder - com.android.ide.eclipse.adt.PreCompilerBuilder + org.eclipse.buildship.core.gradleprojectbuilder - org.eclipse.jdt.core.javabuilder + com.android.ide.eclipse.adt.ResourceManagerBuilder + + + + + com.android.ide.eclipse.adt.PreCompilerBuilder @@ -29,5 +34,17 @@ com.android.ide.eclipse.adt.AndroidNature org.eclipse.jdt.core.javanature + org.eclipse.buildship.core.gradleprojectnature + + + 1732818017518 + + 30 + + org.eclipse.core.resources.regexFilterMatcher + node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__ + + + diff --git a/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt b/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt index 5abcb2e8b9..6a2e955e4c 100644 --- a/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt +++ b/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt @@ -53,6 +53,11 @@ class MediaEncryptionTest : BaseTest() { CommCareApplication.instance().currentApp.appPreferences.edit() .remove(HiddenPreferences.ENCRYPT_CAPTURED_MEDIA) .apply() + + // Ensure test image has valid format for EXIF + val context = InstrumentationRegistry.getInstrumentation().targetContext + val testImage = File(context.externalCacheDir.toString() + "/test.jpg") // Use .jpg instead of .png + // ... rest of setup } @After @@ -65,6 +70,15 @@ class MediaEncryptionTest : BaseTest() { InstrumentationUtility.logout() } + @After + fun cleanup() { + // Clean up any test files + val context = InstrumentationRegistry.getInstrumentation().targetContext + FileUtil.deleteFileOrDir(context.externalCacheDir) + + // ... existing cleanup code ... + } + @Test fun testImageEncryption() { // navigate and add an image diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 8b6bb154d7..3666c28dd3 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -867,12 +867,77 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur } } - public static void copyFileWithExifData(File sourceFile, File destFile) throws IOException { + private static void copyExifData(String sourcePath, String destPath) throws IOException { + try { + ExifInterface source = new ExifInterface(sourcePath); + ExifInterface dest = new ExifInterface(destPath); + + // Add null check for source file + if (source == null) { + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + "Source file doesn't support EXIF data: " + sourcePath); + return; + } + + String[] tagsToPreserve = { + // GPS data + ExifInterface.TAG_GPS_LATITUDE, + ExifInterface.TAG_GPS_LATITUDE_REF, + ExifInterface.TAG_GPS_LONGITUDE, + ExifInterface.TAG_GPS_LONGITUDE_REF, + ExifInterface.TAG_GPS_TIMESTAMP, + ExifInterface.TAG_GPS_DATESTAMP, + ExifInterface.TAG_GPS_ALTITUDE, + ExifInterface.TAG_GPS_ALTITUDE_REF, + ExifInterface.TAG_GPS_AREA_INFORMATION, + + // Timestamp data + ExifInterface.TAG_DATETIME, + ExifInterface.TAG_DATETIME_DIGITIZED, + ExifInterface.TAG_DATETIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME, + ExifInterface.TAG_OFFSET_TIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME_DIGITIZED, + + // Image metadata + ExifInterface.TAG_COPYRIGHT, + ExifInterface.TAG_IMAGE_DESCRIPTION, + ExifInterface.TAG_EXIF_VERSION, + ExifInterface.TAG_ORIENTATION + }; + + for (String tag : tagsToPreserve) { + String value = source.getAttribute(tag); + if (value != null) { + dest.setAttribute(tag, value); + } + } + + dest.saveAttributes(); + } catch (IllegalArgumentException e) { + // Log but don't fail if file doesn't support EXIF + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + "File doesn't support EXIF data: " + sourcePath); + } catch (IOException e) { + // Log but don't fail if EXIF copying fails + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + "Failed to copy EXIF data from " + sourcePath + " to " + destPath + ": " + e.getMessage()); + } + } + + public static void copyFileWithExifData(File sourceFile, File destFile, boolean isSignature) throws IOException { // First copy the file normally copyFile(sourceFile, destFile); - // Then copy EXIF data - copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + // Skip EXIF copying for signatures + if (!isSignature) { + try { + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + } catch (Exception e) { + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + "Failed to copy EXIF data: " + e.getMessage()); + } + } } public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { @@ -886,45 +951,4 @@ public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, return scaled; } - - private static void copyExifData(String sourcePath, String destPath) throws IOException { - ExifInterface source = new ExifInterface(sourcePath); - ExifInterface dest = new ExifInterface(destPath); - - String[] tagsToPreserve = { - // GPS data - ExifInterface.TAG_GPS_LATITUDE, - ExifInterface.TAG_GPS_LATITUDE_REF, - ExifInterface.TAG_GPS_LONGITUDE, - ExifInterface.TAG_GPS_LONGITUDE_REF, - ExifInterface.TAG_GPS_TIMESTAMP, - ExifInterface.TAG_GPS_DATESTAMP, - ExifInterface.TAG_GPS_ALTITUDE, - ExifInterface.TAG_GPS_ALTITUDE_REF, - ExifInterface.TAG_GPS_AREA_INFORMATION, - - // Timestamp data - ExifInterface.TAG_DATETIME, - ExifInterface.TAG_DATETIME_DIGITIZED, - ExifInterface.TAG_DATETIME_ORIGINAL, - ExifInterface.TAG_OFFSET_TIME, - ExifInterface.TAG_OFFSET_TIME_ORIGINAL, - ExifInterface.TAG_OFFSET_TIME_DIGITIZED, - - // Image metadata - ExifInterface.TAG_COPYRIGHT, - ExifInterface.TAG_IMAGE_DESCRIPTION, - ExifInterface.TAG_EXIF_VERSION, - ExifInterface.TAG_ORIENTATION - }; - - for (String tag : tagsToPreserve) { - String value = source.getAttribute(tag); - if (value != null) { - dest.setAttribute(tag, value); - } - } - - dest.saveAttributes(); - } } From 72aea770d5754b993eee7047eaa69c2f465e8ef3 Mon Sep 17 00:00:00 2001 From: Mido Date: Thu, 28 Nov 2024 19:31:21 +0100 Subject: [PATCH 05/12] Media Test --- app/.classpath | 9 +++++--- app/.project | 23 +++---------------- .../androidTests/MediaEncryptionTest.kt | 14 ----------- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/app/.classpath b/app/.classpath index 3a6c280f3e..6c6bd1ecbc 100644 --- a/app/.classpath +++ b/app/.classpath @@ -1,9 +1,12 @@ - - + + + + + - + diff --git a/app/.project b/app/.project index 8ba08befd3..9958d0b972 100644 --- a/app/.project +++ b/app/.project @@ -6,22 +6,17 @@ - org.eclipse.jdt.core.javabuilder - - - - - org.eclipse.buildship.core.gradleprojectbuilder + com.android.ide.eclipse.adt.ResourceManagerBuilder - com.android.ide.eclipse.adt.ResourceManagerBuilder + com.android.ide.eclipse.adt.PreCompilerBuilder - com.android.ide.eclipse.adt.PreCompilerBuilder + org.eclipse.jdt.core.javabuilder @@ -34,17 +29,5 @@ com.android.ide.eclipse.adt.AndroidNature org.eclipse.jdt.core.javanature - org.eclipse.buildship.core.gradleprojectnature - - - 1732818017518 - - 30 - - org.eclipse.core.resources.regexFilterMatcher - node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__ - - - diff --git a/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt b/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt index 6a2e955e4c..5abcb2e8b9 100644 --- a/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt +++ b/app/instrumentation-tests/src/org/commcare/androidTests/MediaEncryptionTest.kt @@ -53,11 +53,6 @@ class MediaEncryptionTest : BaseTest() { CommCareApplication.instance().currentApp.appPreferences.edit() .remove(HiddenPreferences.ENCRYPT_CAPTURED_MEDIA) .apply() - - // Ensure test image has valid format for EXIF - val context = InstrumentationRegistry.getInstrumentation().targetContext - val testImage = File(context.externalCacheDir.toString() + "/test.jpg") // Use .jpg instead of .png - // ... rest of setup } @After @@ -70,15 +65,6 @@ class MediaEncryptionTest : BaseTest() { InstrumentationUtility.logout() } - @After - fun cleanup() { - // Clean up any test files - val context = InstrumentationRegistry.getInstrumentation().targetContext - FileUtil.deleteFileOrDir(context.externalCacheDir) - - // ... existing cleanup code ... - } - @Test fun testImageEncryption() { // navigate and add an image From a15c106875ff2020705a4bd111d91fd269983137 Mon Sep 17 00:00:00 2001 From: Mido Date: Thu, 28 Nov 2024 19:32:50 +0100 Subject: [PATCH 06/12] FileUtil changes --- app/src/org/commcare/utils/FileUtil.java | 42 +++++++++--------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 3666c28dd3..b100643bbb 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -867,18 +867,27 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur } } + public static void copyFileWithExifData(File sourceFile, File destFile) throws IOException { + // First copy the file normally + copyFile(sourceFile, destFile); + + // Only try to copy EXIF data for image files + String mimeType = getMimeType(sourceFile.getAbsolutePath()); + if (mimeType != null && mimeType.startsWith("image/")) { + try { + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + } catch (Exception e) { + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + "Failed to copy EXIF data: " + e.getMessage()); + } + } + } + private static void copyExifData(String sourcePath, String destPath) throws IOException { try { ExifInterface source = new ExifInterface(sourcePath); ExifInterface dest = new ExifInterface(destPath); - // Add null check for source file - if (source == null) { - Logger.log(LogTypes.TYPE_WARNING_NETWORK, - "Source file doesn't support EXIF data: " + sourcePath); - return; - } - String[] tagsToPreserve = { // GPS data ExifInterface.TAG_GPS_LATITUDE, @@ -914,10 +923,6 @@ private static void copyExifData(String sourcePath, String destPath) throws IOEx } dest.saveAttributes(); - } catch (IllegalArgumentException e) { - // Log but don't fail if file doesn't support EXIF - Logger.log(LogTypes.TYPE_WARNING_NETWORK, - "File doesn't support EXIF data: " + sourcePath); } catch (IOException e) { // Log but don't fail if EXIF copying fails Logger.log(LogTypes.TYPE_WARNING_NETWORK, @@ -925,21 +930,6 @@ private static void copyExifData(String sourcePath, String destPath) throws IOEx } } - public static void copyFileWithExifData(File sourceFile, File destFile, boolean isSignature) throws IOException { - // First copy the file normally - copyFile(sourceFile, destFile); - - // Skip EXIF copying for signatures - if (!isSignature) { - try { - copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); - } catch (Exception e) { - Logger.log(LogTypes.TYPE_WARNING_NETWORK, - "Failed to copy EXIF data: " + e.getMessage()); - } - } - } - public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { // First scale the image boolean scaled = scaleAndSaveImage(sourceFile, destFile.getAbsolutePath(), maxDimen); From 94e6fe9cf083ed66584e904691717c192bde54aa Mon Sep 17 00:00:00 2001 From: Mido Date: Fri, 29 Nov 2024 09:13:26 +0100 Subject: [PATCH 07/12] So yes, it will fail safely if: The file isn't an image (mimeType check) The image doesn't have EXIF data (ExifInterface handles this gracefully) There are any issues reading/writing EXIF data (caught and logged) The only failure that would propagate up is if the initial file copy fails, which is appropriate since that's the core operation we need to succeed. --- app/.classpath | 9 +++------ app/.project | 25 ++++++++++++++++++++---- app/src/org/commcare/utils/FileUtil.java | 7 +------ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/.classpath b/app/.classpath index 6c6bd1ecbc..3a6c280f3e 100644 --- a/app/.classpath +++ b/app/.classpath @@ -1,12 +1,9 @@ - - - - - + + - + diff --git a/app/.project b/app/.project index 9958d0b972..b5ecac02ed 100644 --- a/app/.project +++ b/app/.project @@ -1,22 +1,27 @@ - commcare-odk + app - com.android.ide.eclipse.adt.ResourceManagerBuilder + org.eclipse.jdt.core.javabuilder - com.android.ide.eclipse.adt.PreCompilerBuilder + org.eclipse.buildship.core.gradleprojectbuilder - org.eclipse.jdt.core.javabuilder + com.android.ide.eclipse.adt.ResourceManagerBuilder + + + + + com.android.ide.eclipse.adt.PreCompilerBuilder @@ -29,5 +34,17 @@ com.android.ide.eclipse.adt.AndroidNature org.eclipse.jdt.core.javanature + org.eclipse.buildship.core.gradleprojectnature + + + 1732821442773 + + 30 + + org.eclipse.core.resources.regexFilterMatcher + node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__ + + + diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index b100643bbb..6d72c29a03 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -874,12 +874,7 @@ public static void copyFileWithExifData(File sourceFile, File destFile) throws I // Only try to copy EXIF data for image files String mimeType = getMimeType(sourceFile.getAbsolutePath()); if (mimeType != null && mimeType.startsWith("image/")) { - try { - copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); - } catch (Exception e) { - Logger.log(LogTypes.TYPE_WARNING_NETWORK, - "Failed to copy EXIF data: " + e.getMessage()); - } + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); } } From 83b729b237fa39a3b3ce683495ac2c4766a791c0 Mon Sep 17 00:00:00 2001 From: Mido Date: Fri, 29 Nov 2024 09:15:10 +0100 Subject: [PATCH 08/12] restore classpath and project --- app/.classpath | 9 ++++++--- app/.project | 25 ++++--------------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/app/.classpath b/app/.classpath index 3a6c280f3e..6c6bd1ecbc 100644 --- a/app/.classpath +++ b/app/.classpath @@ -1,9 +1,12 @@ - - + + + + + - + diff --git a/app/.project b/app/.project index b5ecac02ed..9958d0b972 100644 --- a/app/.project +++ b/app/.project @@ -1,27 +1,22 @@ - app + commcare-odk - org.eclipse.jdt.core.javabuilder - - - - - org.eclipse.buildship.core.gradleprojectbuilder + com.android.ide.eclipse.adt.ResourceManagerBuilder - com.android.ide.eclipse.adt.ResourceManagerBuilder + com.android.ide.eclipse.adt.PreCompilerBuilder - com.android.ide.eclipse.adt.PreCompilerBuilder + org.eclipse.jdt.core.javabuilder @@ -34,17 +29,5 @@ com.android.ide.eclipse.adt.AndroidNature org.eclipse.jdt.core.javanature - org.eclipse.buildship.core.gradleprojectnature - - - 1732821442773 - - 30 - - org.eclipse.core.resources.regexFilterMatcher - node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__ - - - From 2cdd7e291871deeee622c07f2a60f924a7747e0b Mon Sep 17 00:00:00 2001 From: Mido Date: Wed, 4 Dec 2024 19:01:57 +0100 Subject: [PATCH 09/12] coderabbit review --- .../components/ImageCaptureProcessing.java | 67 +++++++++++++++++-- app/src/org/commcare/utils/FileUtil.java | 14 ++-- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java index 7bd099e5b8..78f2ea9b13 100644 --- a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java +++ b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java @@ -67,17 +67,72 @@ private static Pair moveAndScaleImage(File originalImage, boolean // Encrypt the scaled or original image to final path if (HiddenPreferences.isMediaCaptureEncryptionEnabled()) { finalFilePath = finalFilePath + MediaWidget.AES_EXTENSION; + File sourceFile = new File(sourcePath); + File destFile = new File(finalFilePath); + try { + // Extract EXIF data before encryption + ExifInterface sourceExif = null; + String mimeType = FileUtil.getMimeType(sourcePath); + if (mimeType != null && mimeType.startsWith("image/")) { + sourceExif = new ExifInterface(sourcePath); + } + + // Perform encryption EncryptionIO.encryptFile(sourcePath, finalFilePath, formEntryActivity.getSymetricKey()); + + // If we had EXIF data, store it in a companion file + if (sourceExif != null) { + String exifPath = finalFilePath + ".exif"; + ExifInterface destExif = new ExifInterface(exifPath); + + // Copy all EXIF tags + String[] tagsToPreserve = { + ExifInterface.TAG_GPS_LATITUDE, + ExifInterface.TAG_GPS_LATITUDE_REF, + ExifInterface.TAG_GPS_LONGITUDE, + ExifInterface.TAG_GPS_LONGITUDE_REF, + ExifInterface.TAG_GPS_TIMESTAMP, + ExifInterface.TAG_GPS_DATESTAMP, + ExifInterface.TAG_GPS_ALTITUDE, + ExifInterface.TAG_GPS_ALTITUDE_REF, + ExifInterface.TAG_GPS_AREA_INFORMATION, + ExifInterface.TAG_DATETIME, + ExifInterface.TAG_DATETIME_DIGITIZED, + ExifInterface.TAG_DATETIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME, + ExifInterface.TAG_OFFSET_TIME_ORIGINAL, + ExifInterface.TAG_OFFSET_TIME_DIGITIZED, + ExifInterface.TAG_COPYRIGHT, + ExifInterface.TAG_IMAGE_DESCRIPTION, + ExifInterface.TAG_EXIF_VERSION, + ExifInterface.TAG_ORIENTATION + }; + + for (String tag : tagsToPreserve) { + String value = sourceExif.getAttribute(tag); + if (value != null) { + destExif.setAttribute(tag, value); + } + } + destExif.saveAttributes(); + + // Encrypt the EXIF data file as well + EncryptionIO.encryptFile(exifPath, exifPath + MediaWidget.AES_EXTENSION, + formEntryActivity.getSymetricKey()); + // Delete the unencrypted EXIF file + new File(exifPath).delete(); + } } catch (Exception e) { - throw new IOException("Failed to encrypt " + sourcePath + - " to " + finalFilePath, e); + throw new IOException("Failed to encrypt image and preserve EXIF data from " + + sourcePath + " to " + finalFilePath, e); } } else { try { - FileUtil.copyFile(sourcePath, finalFilePath); + FileUtil.copyFileWithExifData(new File(sourcePath), new File(finalFilePath)); } catch (Exception e) { - throw new IOException("Failed to rename " + sourcePath + " to " + finalFilePath); + throw new IOException("Failed to copy image with EXIF data from " + + sourcePath + " to " + finalFilePath); } } @@ -95,8 +150,8 @@ private static File makeRawCopy(File originalImage, String instanceFolder, Strin try { FileUtil.copyFileWithExifData(originalImage, rawImageFile); } catch (Exception e) { - throw new IOException("Failed to rename " + originalImage.getAbsolutePath() + - " to " + rawImageFile.getAbsolutePath()); + throw new IOException("Failed to copy image with EXIF data from " + + originalImage.getAbsolutePath() + " to " + rawImageFile.getAbsolutePath()); } return rawImageFile; } diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 6d72c29a03..8fb8db82c7 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -878,7 +878,7 @@ public static void copyFileWithExifData(File sourceFile, File destFile) throws I } } - private static void copyExifData(String sourcePath, String destPath) throws IOException { + private static void copyExifData(String sourcePath, String destPath) { try { ExifInterface source = new ExifInterface(sourcePath); ExifInterface dest = new ExifInterface(destPath); @@ -928,12 +928,16 @@ private static void copyExifData(String sourcePath, String destPath) throws IOEx public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { // First scale the image boolean scaled = scaleAndSaveImage(sourceFile, destFile.getAbsolutePath(), maxDimen); - - if (scaled) { + + if (!scaled) { + // If scaling was not needed, copy the source file to destination with EXIF data + copyFileWithExifData(sourceFile, destFile); + return true; + } else { // Copy EXIF data from source to scaled image copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); } - - return scaled; + + return true; } } From a241f231d986eab4509dc3732481f13cd92cfc32 Mon Sep 17 00:00:00 2001 From: Mido Date: Wed, 4 Dec 2024 19:04:11 +0100 Subject: [PATCH 10/12] JavaDocs --- app/src/org/commcare/utils/FileUtil.java | 34 +++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 8fb8db82c7..6d1ec6550f 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -65,6 +65,12 @@ public class FileUtil { private static final String LOG_TOKEN = "cc-file-util"; + /** + * Deletes a file or directory. + * + * @param path The path to the file or directory to delete + * @return true if the file and all of its contents were deleted successfully, false otherwise + */ public static boolean deleteFileOrDir(String path) { return deleteFileOrDir(new File(path)); } @@ -84,6 +90,13 @@ public static boolean deleteFileOrDir(File f) { return f.delete(); } + /** + * Cleans up file paths. + * + * @param fullPath The full path to the file + * @param extendedPath The extended path to stop at + * @return true if the file path was cleaned up successfully, false otherwise + */ public static boolean cleanFilePath(String fullPath, String extendedPath) { //There are actually a few things that can go wrong here, should be careful @@ -867,6 +880,14 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur } } + /** + * Copies a file from source to destination while preserving EXIF metadata for image files. + * For non-image files, performs a regular file copy. + * + * @param sourceFile The source file to copy from + * @param destFile The destination file to copy to + * @throws IOException If there is an error during file copying or EXIF data transfer + */ public static void copyFileWithExifData(File sourceFile, File destFile) throws IOException { // First copy the file normally copyFile(sourceFile, destFile); @@ -925,7 +946,18 @@ private static void copyExifData(String sourcePath, String destPath) { } } - public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) throws IOException { + /** + * Scales an image if needed and preserves its EXIF metadata in the process. + * + * @param sourceFile The original image file + * @param destFile The destination file where the scaled image will be saved + * @param maxDimen The maximum dimension (width or height) allowed for the scaled image + * @return true if the operation was successful (either scaled with EXIF preserved or + * copied with EXIF if scaling wasn't needed) + * @throws IOException If there is an error during scaling, file operations or EXIF data transfer + */ + public static boolean scaleAndSaveImageWithExif(File sourceFile, File destFile, int maxDimen) + throws IOException { // First scale the image boolean scaled = scaleAndSaveImage(sourceFile, destFile.getAbsolutePath(), maxDimen); From 8a6039e8a53f0f13043aed852808567fdc737964 Mon Sep 17 00:00:00 2001 From: Mido Date: Wed, 4 Dec 2024 19:12:42 +0100 Subject: [PATCH 11/12] coderabbit review --- app/src/org/commcare/utils/FileUtil.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 6d1ec6550f..4a1c104f93 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -941,8 +941,9 @@ private static void copyExifData(String sourcePath, String destPath) { dest.saveAttributes(); } catch (IOException e) { // Log but don't fail if EXIF copying fails - Logger.log(LogTypes.TYPE_WARNING_NETWORK, - "Failed to copy EXIF data from " + sourcePath + " to " + destPath + ": " + e.getMessage()); + Logger.log(LogTypes.TYPE_WARNING_NETWORK, + String.format("Failed to copy EXIF data from %s to %s: %s (Error: %s)", + sourcePath, destPath, e.getMessage(), e.getClass().getSimpleName())); } } From 4bb8c49294904467c7cdca59f0e4d8f79aa1b191 Mon Sep 17 00:00:00 2001 From: Mido Date: Wed, 4 Dec 2024 19:24:45 +0100 Subject: [PATCH 12/12] coderabbit review --- .../components/ImageCaptureProcessing.java | 6 +++--- app/src/org/commcare/utils/FileUtil.java | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java index 78f2ea9b13..f5f9863ff6 100644 --- a/app/src/org/commcare/activities/components/ImageCaptureProcessing.java +++ b/app/src/org/commcare/activities/components/ImageCaptureProcessing.java @@ -132,7 +132,7 @@ private static Pair moveAndScaleImage(File originalImage, boolean FileUtil.copyFileWithExifData(new File(sourcePath), new File(finalFilePath)); } catch (Exception e) { throw new IOException("Failed to copy image with EXIF data from " + - sourcePath + " to " + finalFilePath); + sourcePath + " to " + finalFilePath, e); } } @@ -150,8 +150,8 @@ private static File makeRawCopy(File originalImage, String instanceFolder, Strin try { FileUtil.copyFileWithExifData(originalImage, rawImageFile); } catch (Exception e) { - throw new IOException("Failed to copy image with EXIF data from " + - originalImage.getAbsolutePath() + " to " + rawImageFile.getAbsolutePath()); + throw new IOException("Failed to copy image with EXIF data from " + + originalImage.getAbsolutePath() + " to " + rawImageFile.getAbsolutePath(), e); } return rawImageFile; } diff --git a/app/src/org/commcare/utils/FileUtil.java b/app/src/org/commcare/utils/FileUtil.java index 4a1c104f93..bd052d7635 100644 --- a/app/src/org/commcare/utils/FileUtil.java +++ b/app/src/org/commcare/utils/FileUtil.java @@ -739,7 +739,7 @@ public static Uri getUriForExternalFile(Context context, File file) { */ public static void copyFile(InputStream inputStream, File dstFile) throws IOException { if (inputStream == null) { - return; + throw new IOException("InputStream is null. Cannot copy file."); } OutputStream outputStream = new FileOutputStream(dstFile); StreamsUtil.writeFromInputToOutputUnmanaged(inputStream, outputStream); @@ -875,8 +875,14 @@ public static boolean isFileTooLargeToUpload(ContentResolver contentResolver, Ur return false; } int sizeIndex = returnCursor.getColumnIndex(OpenableColumns.SIZE); - returnCursor.moveToFirst(); - return returnCursor.getLong(sizeIndex) > FormUploadUtil.MAX_BYTES; + if (sizeIndex == -1) { + return false; + } + if (returnCursor.moveToFirst()) { + return returnCursor.getLong(sizeIndex) > FormUploadUtil.MAX_BYTES; + } else { + return false; + } } } @@ -895,7 +901,11 @@ public static void copyFileWithExifData(File sourceFile, File destFile) throws I // Only try to copy EXIF data for image files String mimeType = getMimeType(sourceFile.getAbsolutePath()); if (mimeType != null && mimeType.startsWith("image/")) { - copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + try { + copyExifData(sourceFile.getAbsolutePath(), destFile.getAbsolutePath()); + } catch (IOException e) { + throw new IOException("Failed to copy EXIF data", e); + } } }