Return correct offsets for getAttributeRange()

Currently, the offset value returned from getAttributeRange() does
not take into account the offset to the EXIF data within the image
file. This CL fixes the code to return the correct offsets.

The existing test code could not detect this issue because it does
not explicitly seek to the given offset and retrieve/compare the
data. This CL also adds test code to do this. Make tag information
is also added because all test files have this information, while only
a select few have thumbnail/GPS tag information.

This CL also fixes a bug where the thumbnail offset was not being
updated if the data was passed with StripOffsets tag.

Bug: 143118731, Bug: 143663411
Test: atest ExifInterfaceTest
Change-Id: Ib5e33335cfafa81e3584e96d49a764ad873835a9

1 files changed, 73 insertions, 64 deletions

diff --git a/media/java/android/media/ExifInterface.java b/media/java/android/media/ExifInterface.java
index 5ac6f7f1b03f..ce4aac9bdb7f 100644
--- a/media/java/android/media/ExifInterface.java
+++ b/media/java/android/media/ExifInterface.java
@@ -1443,7 +1443,9 @@ public class ExifInterface {
     private ByteOrder mExifByteOrder = ByteOrder.BIG_ENDIAN;
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     private boolean mHasThumbnail;
-    // The following values used for indicating a thumbnail position.
+    private boolean mHasThumbnailStrips;
+    private boolean mAreThumbnailStripsConsecutive;
+    // Used to indicate the position of the thumbnail (includes offset to EXIF data segment).
     private int mThumbnailOffset;
     private int mThumbnailLength;
     private byte[] mThumbnailBytes;
@@ -1574,7 +1576,8 @@ public class ExifInterface {
         mFilename = null;
 
         if (shouldBeExifDataOnly) {
-            if (!isExifDataOnly(inputStream)) {
+            inputStream = new BufferedInputStream(inputStream, SIGNATURE_CHECK_SIZE);
+            if (!isExifDataOnly((BufferedInputStream) inputStream)) {
                 Log.w(TAG, "Given data does not follow the structure of an Exif-only data.");
                 return;
             }
@@ -2262,10 +2265,12 @@ public class ExifInterface {
 
     /**
      * Returns the offset and length of thumbnail inside the image file, or
-     * {@code null} if there is no thumbnail.
+     * {@code null} if either there is no thumbnail or the thumbnail bytes are stored
+     * non-consecutively.
      *
      * @return two-element array, the offset in the first value, and length in
-     *         the second, or {@code null} if no thumbnail was found.
+     *         the second, or {@code null} if no thumbnail was found or the thumbnail strips are
+     *         not placed consecutively.
      * @throws IllegalStateException if {@link #saveAttributes()} has been
      *             called since the underlying file was initially parsed, since
      *             that means offsets may have changed.
@@ -2277,13 +2282,12 @@ public class ExifInterface {
         }
 
         if (mHasThumbnail) {
-            if (mIsExifDataOnly) {
-                return new long[] { mThumbnailOffset + mExifOffset, mThumbnailLength };
+            if (mHasThumbnailStrips && !mAreThumbnailStripsConsecutive) {
+                return null;
             }
             return new long[] { mThumbnailOffset, mThumbnailLength };
-        } else {
-            return null;
         }
+        return null;
     }
 
     /**
@@ -2741,20 +2745,17 @@ public class ExifInterface {
         return true;
     }
 
-    private static boolean isExifDataOnly(InputStream inputStream) {
-        try {
-            byte[] signatureCheckBytes = new byte[IDENTIFIER_EXIF_APP1.length];
-            inputStream.read(signatureCheckBytes);
-            if (Arrays.equals(signatureCheckBytes, IDENTIFIER_EXIF_APP1)) {
-                return true;
-            }
-        } catch (IOException e) {
-            if (DEBUG) {
-                Log.w(TAG,
-                        "Encountered error while checking whether input stream is Exif data only");
+    private static boolean isExifDataOnly(BufferedInputStream in) throws IOException {
+        in.mark(IDENTIFIER_EXIF_APP1.length);
+        byte[] signatureCheckBytes = new byte[IDENTIFIER_EXIF_APP1.length];
+        in.read(signatureCheckBytes);
+        in.reset();
+        for (int i = 0; i < IDENTIFIER_EXIF_APP1.length; i++) {
+            if (signatureCheckBytes[i] != IDENTIFIER_EXIF_APP1[i]) {
+                return false;
             }
         }
-        return false;
+        return true;
     }
 
     /**
@@ -2828,10 +2829,9 @@ public class ExifInterface {
                         final long offset = start + IDENTIFIER_EXIF_APP1.length;
                         final byte[] value = Arrays.copyOfRange(bytes,
                                 IDENTIFIER_EXIF_APP1.length, bytes.length);
-                        readExifSegment(value, imageType);
-
                         // Save offset values for handleThumbnailFromJfif() function
                         mExifOffset = (int) offset;
+                        readExifSegment(value, imageType);
                     } else if (ArrayUtils.startsWith(bytes, IDENTIFIER_XMP_APP1)) {
                         // See XMP Specification Part 3: Storage in Files, 1.1.3 JPEG, Table 6
                         final long offset = start + IDENTIFIER_XMP_APP1.length;
@@ -3136,6 +3136,8 @@ public class ExifInterface {
                 if (in.read(bytes) != length) {
                     throw new IOException("Can't read exif");
                 }
+                // Save offset values for handling thumbnail and attribute offsets.
+                mExifOffset = offset;
                 readExifSegment(bytes, IFD_TYPE_PRIMARY);
             }
 
@@ -3148,13 +3150,13 @@ public class ExifInterface {
     }
 
     private void getStandaloneAttributes(ByteOrderedDataInputStream in) throws IOException {
+        in.skipBytes(IDENTIFIER_EXIF_APP1.length);
         // TODO: Need to handle potential OutOfMemoryError
         byte[] data = new byte[in.available()];
         in.readFully(data);
-        readExifSegment(data, IFD_TYPE_PRIMARY);
-
-        // Save offset values for handleThumbnailFromJfif() function
+        // Save offset values for handling thumbnail and attribute offsets.
         mExifOffset = IDENTIFIER_EXIF_APP1.length;
+        readExifSegment(data, IFD_TYPE_PRIMARY);
     }
 
     /**
@@ -3335,13 +3337,11 @@ public class ExifInterface {
                                 + "\n recorded CRC value: " + dataCrcValue + ", calculated CRC "
                                 + "value: " + crc.getValue());
                     }
-
+                    // Save offset values for handleThumbnailFromJfif() function
+                    mExifOffset = bytesRead;
                     readExifSegment(data, IFD_TYPE_PRIMARY);
 
                     validateImages();
-
-                    // Save offset values for handleThumbnailFromJfif() function
-                    mExifOffset = bytesRead;
                     break;
                 } else {
                     // Skip to next chunk
@@ -3396,6 +3396,8 @@ public class ExifInterface {
                         throw new IOException("Failed to read given length for given PNG chunk "
                                 + "type: " + byteArrayToHexString(code));
                     }
+                    // Save offset values for handling thumbnail and attribute offsets.
+                    mExifOffset = bytesRead;
                     readExifSegment(payload, IFD_TYPE_PRIMARY);
                     break;
                 } else {
@@ -3416,8 +3418,6 @@ public class ExifInterface {
                     bytesRead += skipped;
                 }
             }
-            // Save offset values for handleThumbnailFromJfif() function
-            mExifOffset = bytesRead;
         } catch (EOFException e) {
             // Should not reach here. Will only reach here if the file is corrupted or
             // does not follow the WebP specifications
@@ -3448,7 +3448,7 @@ public class ExifInterface {
         // Write EXIF APP1 segment
         dataOutputStream.writeByte(MARKER);
         dataOutputStream.writeByte(MARKER_APP1);
-        writeExifSegment(dataOutputStream, 6);
+        writeExifSegment(dataOutputStream);
 
         byte[] bytes = new byte[4096];
 
@@ -3565,7 +3565,7 @@ public class ExifInterface {
                     new ByteOrderedDataOutputStream(exifByteArrayOutputStream,
                             ByteOrder.BIG_ENDIAN);
             // Store Exif data in separate byte array
-            writeExifSegment(exifDataOutputStream, 0);
+            writeExifSegment(exifDataOutputStream);
             byte[] exifBytes =
                     ((ByteArrayOutputStream) exifDataOutputStream.mOutputStream).toByteArray();
             // Write EXIF chunk data
@@ -3835,7 +3835,7 @@ public class ExifInterface {
                 continue;
             }
 
-            final int bytesOffset = dataInputStream.peek();
+            final int bytesOffset = dataInputStream.peek() + mExifOffset;
             final byte[] bytes = new byte[(int) byteCount];
             dataInputStream.readFully(bytes);
             ExifAttribute attribute = new ExifAttribute(dataFormat, numberOfComponents,
@@ -3965,29 +3965,17 @@ public class ExifInterface {
             int thumbnailOffset = jpegInterchangeFormatAttribute.getIntValue(mExifByteOrder);
             int thumbnailLength = jpegInterchangeFormatLengthAttribute.getIntValue(mExifByteOrder);
 
-            switch (mMimeType) {
-                case IMAGE_TYPE_JPEG:
-                case IMAGE_TYPE_RAF:
-                case IMAGE_TYPE_RW2:
-                case IMAGE_TYPE_PNG:
-                case IMAGE_TYPE_WEBP:
-                    thumbnailOffset += mExifOffset;
-                    break;
-                case IMAGE_TYPE_ORF:
-                    // Update offset value since RAF files have IFD data preceding MakerNote data.
-                    thumbnailOffset += mOrfMakerNoteOffset;
-                    break;
+            if (mMimeType == IMAGE_TYPE_ORF) {
+                // Update offset value since RAF files have IFD data preceding MakerNote data.
+                thumbnailOffset += mOrfMakerNoteOffset;
             }
             // The following code limits the size of thumbnail size not to overflow EXIF data area.
             thumbnailLength = Math.min(thumbnailLength, in.getLength() - thumbnailOffset);
 
-            if (DEBUG) {
-                Log.d(TAG, "Setting thumbnail attributes with offset: " + thumbnailOffset
-                        + ", length: " + thumbnailLength);
-            }
             if (thumbnailOffset > 0 && thumbnailLength > 0) {
                 mHasThumbnail = true;
-                mThumbnailOffset = thumbnailOffset;
+                // Need to add mExifOffset, which is the offset to the EXIF data segment
+                mThumbnailOffset = thumbnailOffset + mExifOffset;
                 mThumbnailLength = thumbnailLength;
                 mThumbnailCompression = DATA_JPEG;
 
@@ -3995,12 +3983,16 @@ public class ExifInterface {
                         && mSeekableFileDescriptor == null) {
                     // TODO: Need to handle potential OutOfMemoryError
                     // Save the thumbnail in memory if the input doesn't support reading again.
-                    byte[] thumbnailBytes = new byte[thumbnailLength];
-                    in.seek(thumbnailOffset);
+                    byte[] thumbnailBytes = new byte[mThumbnailLength];
+                    in.seek(mThumbnailOffset);
                     in.readFully(thumbnailBytes);
                     mThumbnailBytes = thumbnailBytes;
                 }
             }
+            if (DEBUG) {
+                Log.d(TAG, "Setting thumbnail attributes with offset: " + thumbnailOffset
+                        + ", length: " + thumbnailLength);
+            }
         }
     }
 
@@ -4018,12 +4010,16 @@ public class ExifInterface {
             long[] stripByteCounts =
                     convertToLongArray(stripByteCountsAttribute.getValue(mExifByteOrder));
 
-            if (stripOffsets == null) {
-                Log.w(TAG, "stripOffsets should not be null.");
+            if (stripOffsets == null || stripOffsets.length == 0) {
+                Log.w(TAG, "stripOffsets should not be null or have zero length.");
+                return;
+            }
+            if (stripByteCounts == null || stripByteCounts.length == 0) {
+                Log.w(TAG, "stripByteCounts should not be null or have zero length.");
                 return;
             }
-            if (stripByteCounts == null) {
-                Log.w(TAG, "stripByteCounts should not be null.");
+            if (stripOffsets.length != stripByteCounts.length) {
+                Log.w(TAG, "stripOffsets and stripByteCounts should have same length.");
                 return;
             }
 
@@ -4034,10 +4030,18 @@ public class ExifInterface {
 
             int bytesRead = 0;
             int bytesAdded = 0;
+            mHasThumbnail = mHasThumbnailStrips = mAreThumbnailStripsConsecutive = true;
             for (int i = 0; i < stripOffsets.length; i++) {
                 int stripOffset = (int) stripOffsets[i];
                 int stripByteCount = (int) stripByteCounts[i];
 
+                // Check if strips are consecutive
+                // TODO: Add test for non-consecutive thumbnail image
+                if (i < stripOffsets.length - 1
+                        && stripOffset + stripByteCount != stripOffsets[i + 1]) {
+                    mAreThumbnailStripsConsecutive = false;
+                }
+
                 // Skip to offset
                 int skipBytes = stripOffset - bytesRead;
                 if (skipBytes < 0) {
@@ -4057,10 +4061,13 @@ public class ExifInterface {
                         stripBytes.length);
                 bytesAdded += stripBytes.length;
             }
-
-            mHasThumbnail = true;
             mThumbnailBytes = totalStripBytes;
-            mThumbnailLength = totalStripBytes.length;
+
+            if (mAreThumbnailStripsConsecutive) {
+                // Need to add mExifOffset, which is the offset to the EXIF data segment
+                mThumbnailOffset = (int) stripOffsets[0] + mExifOffset;
+                mThumbnailLength = totalStripBytes.length;
+            }
         }
     }
 
@@ -4218,8 +4225,7 @@ public class ExifInterface {
     }
 
     // Writes an Exif segment into the given output stream.
-    private int writeExifSegment(ByteOrderedDataOutputStream dataOutputStream,
-            int exifOffsetFromBeginning) throws IOException {
+    private int writeExifSegment(ByteOrderedDataOutputStream dataOutputStream) throws IOException {
         // The following variables are for calculating each IFD tag group size in bytes.
         int[] ifdOffsets = new int[EXIF_TAGS.length];
         int[] ifdDataSizes = new int[EXIF_TAGS.length];
@@ -4278,7 +4284,9 @@ public class ExifInterface {
         }
 
         // Calculate IFD offsets.
-        int position = 8; // 8 bytes are for TIFF headers
+        // 8 bytes are for TIFF headers: 2 bytes (byte order) + 2 bytes (identifier) + 4 bytes
+        // (offset of IFDs)
+        int position = 8;
         for (int ifdType = 0; ifdType < EXIF_TAGS.length; ++ifdType) {
             if (!mAttributes[ifdType].isEmpty()) {
                 ifdOffsets[ifdType] = position;
@@ -4289,7 +4297,8 @@ public class ExifInterface {
             int thumbnailOffset = position;
             mAttributes[IFD_TYPE_THUMBNAIL].put(JPEG_INTERCHANGE_FORMAT_TAG.name,
                     ExifAttribute.createULong(thumbnailOffset, mExifByteOrder));
-            mThumbnailOffset = exifOffsetFromBeginning + thumbnailOffset;
+            // Need to add mExifOffset, which is the offset to the EXIF data segment
+            mThumbnailOffset = thumbnailOffset + mExifOffset;
             position += mThumbnailLength;
         }