Wrong data returned [264729367]

Fixed

Bug

Status Update

No update yet.

Description

sc...@gmail.com

created issue #1

Jan 8, 2023 10:14AM

Component used: androidx.exifinterface:exifinterface
Version used: 1.3.5
Devices/Android versions reproduced on: Android Emulator API30

When using ExifIntreface on the attached picture, an exifinterface instance is returned, but further reading EXIF attributes yield wrong result, like getAttributeInt(ExifInterface.TAG_IMAGE_WIDTH, 0) returns 160, getAttributeInt(ExifInterface.TAG_IMAGE_LENGTH, 0) returns 120, and ExifIntreface.dateTimeOriginal returns null.

211014041-273c2a1d-74db-42fa-b050-67ded71be015.jpg

3.3 MB

View

Download

Comments

sc...@gmail.com <sc...@gmail.com> #2Feb 11, 2023 02:50PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ib...@google.com <ib...@google.com> #3Feb 22, 2023 04:40PM

I can reproduce the problem. Thanks for the pointer that the reported dimensions are that of the embedded thumbnail.

exiftool reports that the 160x120 dimensions are in the IFD0 directory (i.e. the root one):

$ exiftool -a -G0:1  211014041-273c2a1d-74db-42fa-b050-67ded71be015.jpg | grep -i width
[File]          Image Width                     : 4310
[EXIF:IFD0]     Image Width                     : 160
[EXIF:ExifIFD]  Exif Image Width                : 4310
[EXIF:InteropIFD] Related Image Width           : 4310

Tracing excecution through ExifInterface when reading this file, it finds 24 entries in the first IFD but it stops reading them after the 20th (index=19). This is because this seek() call throws an EOFException:

java.io.EOFException: Reached EOF while skipping 182211 bytes.
	at androidx.exifinterface.media.ExifInterface$ByteOrderedDataInputStream.skipFully(ExifInterface.java:7935)
	at androidx.exifinterface.media.ExifInterface$SeekableByteOrderedDataInputStream.seek(ExifInterface.java:7757)
	at androidx.exifinterface.media.ExifInterface.readImageFileDirectory(ExifInterface.java:6964)
	at androidx.exifinterface.media.ExifInterface.readExifSegment(ExifInterface.java:6713)
	at androidx.exifinterface.media.ExifInterface.getJpegAttributes(ExifInterface.java:5617)
	at androidx.exifinterface.media.ExifInterface.loadAttributes(ExifInterface.java:4597)
	at androidx.exifinterface.media.ExifInterface.initForFilename(ExifInterface.java:5282)
	at androidx.exifinterface.media.ExifInterface.<init>(ExifInterface.java:3933)

The 20th tag has tag_number=330 (i.e. sub IFD pointer), and after redirecting to position 0x185 in the stream (0x1A3 in the file) we read 4 bytes (ulong, the format of sub IFD pointers): 0x0002C94C = 182604

exiftool agrees this is an invalid offset:

$ exiftool -v3 211014041-273c2a1d-74db-42fa-b050-67ded71be015.jpg
  | 19) SubIFD (SubDirectory) -->
  |     - Tag 0x014a (8 bytes, int32u[2]):
  |         01a3: 4c c9 02 00 c4 c9 02 00                         [L.......]
  | Warning = Bad SubIFD SubDirectory start (directory end is 182612 but EXIF size is o[snip]

It looks like the exif isn't valid, and that's causing ExifInterface to stop parsing early, after it's read the thumbnail width/height but before it's encountered the full image width/height (which should replace the thumbnail ones once they're found).

I've experimented with catching this EOFException and trying to continue with the 'next' tag. We already have precedent for this sort of "ignore obviously invalid (negative) offsets and carry on", e.g. just below. With this change the correct height and width are returned.

Message last modified on Feb 22, 2023 04:41PM

ap...@google.com <ap...@google.com> #4Mar 2, 2023 04:33PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3d6c1ad6b9d8b10710ccfe08c0268aec5458dcb9
Author: Ian Baker <ibaker@google.com>
Date: Wed Feb 22 16:46:34 2023

Continue parsing after trying to follow an invalid IFD offset

This means we're now able to parse the GPS info out of
jpeg_with_exif_invalid_offset.jpg, because we just skip the invalid
ExifIFD offset instead of stopping parsing completely due to following
the offset and encountering an EOFException.

Test: ./gradlew :exifinterface:exifinterface:connectedAndroidTest
Bug: 264729367
Change-Id: I715a4de9ae14e2fd6576b025a36e16763d6960a6

M exifinterface/exifinterface/src/androidTest/java/androidx/exifinterface/media/ExifInterfaceTest.java
M exifinterface/exifinterface/src/androidTest/res/values/arrays.xml
M exifinterface/exifinterface/src/main/java/androidx/exifinterface/media/ExifInterface.java

https://android-review.googlesource.com/2460713

ap...@google.com <ap...@google.com> #5Mar 2, 2023 04:33PM

Project: platform/frameworks/support
Branch: androidx-main

commit ff398f939baaa4e6fec4eaefbe2b27767c056028
Author: Ian Baker <ibaker@google.com>
Date: Thu Feb 23 14:43:05 2023

Add test for invalid IFD offset in ExifInterface

jpeg_with_exif_invalid_offset.jpg was created by modifying the offset
of the ExifIFD from 0x00000008 to 0x00abcd08 (i.e. well outside the
exif block of the jpeg file).

The current behaviour of ExifInterface stops parsing the file
completely at this point, and thus never parses the `GpsInfo` (which
is the next tag after the ExifIFD pointer). An upcoming change will
make ExifInterface skip the invalid offset, but continue parsing later
tags, and therefore the GPS info will be successfully parsed from this
broken file.

Broken/invalid IFD offsets are noted to be a common failure mode in exif
data (see top bullet of

https://en.wikipedia.org/wiki/Exif#Technical_2).

exiftool complains about the invalid offset but is able to parse the
GPS data:

$ exiftool -v4 jpeg_with_exif_invalid_offset.jpg
<snip>
| 6) ExifOffset (SubDirectory) -->
| - Tag 0x8769 (4 bytes, int32u[1]):
| 01e6: 00 ab cd 08 [....]
| Warning = Bad ExifOffset SubDirectory start (directory end is 11259148 but EXIF size is only 790)
| 7) GPSInfo (SubDirectory) -->
| - Tag 0x8825 (4 bytes, int32u[1]):
| 01f2: 00 00 01 16 [....]
| + [GPS directory with 9 entries]
| | 0) GPSLatitudeRef = N
| | - Tag 0x0001 (2 bytes, string[2]):
| | 012c: 4e 00 [N.]
<snip>

Test: ./gradlew :exifinterface:exifinterface:connectedAndroidTest
Bug: 264729367
Change-Id: I1782d5bef9d91caa2fd97508bdf6c3924a7d7963

M exifinterface/exifinterface/src/androidTest/java/androidx/exifinterface/media/ExifInterfaceTest.java
A exifinterface/exifinterface/src/androidTest/res/raw/jpeg_with_exif_invalid_offset.jpg
M exifinterface/exifinterface/src/androidTest/res/values/arrays.xml

https://android-review.googlesource.com/2460712

ib...@google.com <ib...@google.com> Mar 2, 2023 04:39PM

Marked as fixed.

na...@google.com <na...@google.com> #6Dec 13, 2023 06:33PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.exifinterface:exifinterface:1.3.7

na...@google.com <na...@google.com> #7Nov 13, 2024 06:19PM