Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e3e6fa1e authored by Rakesh Kumar's avatar Rakesh Kumar
Browse files

Prevent ExifInterface incorrectly copying a WebP padding byte 

When skipping over the EXIF chunk in the input file (and writing a new
EXIF chunk to the output based on the internal Exif data of the class),
it's important to skip both the chunk and any trailing padding byte (if
the chunk size is odd).

Without this fix the output of saveAttributes may be invalid (not
decodable with BitmapFactory.decodeFile).

Test: Added tests to ExifInterfaceTest to check the output of
saveAttributes is always parsable with BitmapFactory.

Bug: 253622642

Change-Id: I6b7c4612b830dbf95a12c42eac5d6301dfb15104
parent 81c4fd80

media/java/android/media/ExifInterface.java

+5 −0
Original line number Diff line number Diff line
@@ -3750,6 +3750,11 @@ public class ExifInterface { 
                // Skip input stream to the end of the EXIF chunk

                totalInputStream.skipBytes(WEBP_CHUNK_TYPE_BYTE_LENGTH);

                int exifChunkLength = totalInputStream.readInt();

                // RIFF chunks have a single padding byte at the end if the declared chunk size is

                // odd.

                if (exifChunkLength % 2 != 0) {

                    exifChunkLength++;

                }

                totalInputStream.skipBytes(exifChunkLength);



                // Write new EXIF chunk to output stream

media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/unit/ExifInterfaceTest.java

+60 −2
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.graphics.Bitmap; 
import android.graphics.BitmapFactory;

import android.media.ExifInterface;

import android.os.Environment;

import android.os.FileUtils;

import android.test.AndroidTestCase;

import android.util.Log;

import android.system.ErrnoException;
@@ -37,6 +38,8 @@ import java.io.FileInputStream; 
import java.io.FileOutputStream;

import java.io.InputStream;

import java.io.IOException;

import java.io.OutputStream;

import java.util.Objects;



import libcore.io.IoUtils;

import libcore.io.Streams;
@@ -353,20 +356,23 @@ public class ExifInterfaceTest extends AndroidTestCase { 
        }

    }



    private void testSaveAttributes_withFileName(File imageFile, ExpectedValue expectedValue)

    private void testSaveAttributes_withFileName(File srcFile, ExpectedValue expectedValue)

            throws IOException {

        File imageFile = clone(srcFile);

        String verboseTag = imageFile.getName();



        ExifInterface exifInterface = new ExifInterface(imageFile.getAbsolutePath());

        exifInterface.saveAttributes();

        exifInterface = new ExifInterface(imageFile.getAbsolutePath());

        compareWithExpectedValue(exifInterface, expectedValue, verboseTag);

        assertBitmapsEquivalent(srcFile, imageFile);

        assertSecondSaveProducesSameSizeFile(imageFile);



        // Test for modifying one attribute.

        exifInterface = new ExifInterface(imageFile.getAbsolutePath());

        String backupValue = exifInterface.getAttribute(ExifInterface.TAG_MAKE);

        exifInterface.setAttribute(ExifInterface.TAG_MAKE, "abc");

        exifInterface.saveAttributes();

        exifInterface = new ExifInterface(imageFile.getAbsolutePath());

        assertEquals("abc", exifInterface.getAttribute(ExifInterface.TAG_MAKE));

        // Restore the backup value.

        exifInterface.setAttribute(ExifInterface.TAG_MAKE, backupValue);
@@ -481,4 +487,56 @@ public class ExifInterfaceTest extends AndroidTestCase { 
        // Test if it is possible to parse the volantis generated JPEG smoothly.

        testExifInterfaceForJpeg(VOLANTIS_JPEG, R.array.volantis_jpg);

    }



    /**

     * Asserts that {@code expectedImageFile} and {@code actualImageFile} can be decoded by

     * {@link BitmapFactory} and the results have the same width, height and MIME type.

     *

     * <p>This does not check the image itself for similarity/equality.

     */

    private void assertBitmapsEquivalent(File expectedImageFile, File actualImageFile) {

        BitmapFactory.Options expectedOptions = new BitmapFactory.Options();

        Bitmap expectedBitmap = Objects.requireNonNull(

                BitmapFactory.decodeFile(expectedImageFile.getAbsolutePath(), expectedOptions));

        BitmapFactory.Options actualOptions = new BitmapFactory.Options();

        Bitmap actualBitmap = Objects.requireNonNull(

                BitmapFactory.decodeFile(actualImageFile.getAbsolutePath(), actualOptions));



        assertEquals(expectedOptions.outWidth, actualOptions.outWidth);

        assertEquals(expectedOptions.outHeight, actualOptions.outHeight);

        assertEquals(expectedOptions.outMimeType, actualOptions.outMimeType);

        assertEquals(expectedBitmap.getWidth(), actualBitmap.getWidth());

        assertEquals(expectedBitmap.getHeight(), actualBitmap.getHeight());

    }



    /**

     * Asserts that saving the file the second time (without modifying any attributes) produces

     * exactly the same length file as the first save. The first save (with no modifications) is

     * expected to (possibly) change the file length because {@link ExifInterface} may move/reformat

     * the Exif block within the file, but the second save should not make further modifications.

     */

    private void assertSecondSaveProducesSameSizeFile(File imageFileAfterOneSave)

            throws IOException {

        File imageFileAfterTwoSaves = clone(imageFileAfterOneSave);

        ExifInterface exifInterface = new ExifInterface(imageFileAfterTwoSaves.getAbsolutePath());

        exifInterface.saveAttributes();

        if (imageFileAfterOneSave.getAbsolutePath().endsWith(".png")

                || imageFileAfterOneSave.getAbsolutePath().endsWith(".webp")) {

            // PNG and (some) WebP files are (surprisingly) modified between the first and second

            // save (b/249097443), so we check the difference between second and third save instead.

            File imageFileAfterThreeSaves = clone(imageFileAfterTwoSaves);

            exifInterface = new ExifInterface(imageFileAfterThreeSaves.getAbsolutePath());

            exifInterface.saveAttributes();

            assertEquals(imageFileAfterTwoSaves.length(), imageFileAfterThreeSaves.length());

        } else {

            assertEquals(imageFileAfterOneSave.length(), imageFileAfterTwoSaves.length());

        }

    }



    private static File clone(File original) throws IOException {

        final File cloned =

                File.createTempFile("tmp_", +System.nanoTime() + "_" + original.getName());

        FileUtils.copyFileOrThrow(original, cloned);

        return cloned;

    }

}