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.

Sign up for GitHub

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

Avoid jpeg parse fail due to jfxx and exif #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Avoid jpeg parse fail due to jfxx and exif #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e337c95
add jfxx signature when parsing jpeg
Mar 28, 2018
8ad1124
Avoid jpeg parse fail due to exif
Mar 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/main/java/org/apache/commons/imaging/formats/jpeg/JpegConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ public final class JpegConstants {
0x46, // F
0x20, //
});
public static final BinaryConstant JFXX0_EXTENSION_SIGNATURE = new BinaryConstant(
new byte[] { 0x4a, // J
0x46, // F
0x58, // X
0x58, // X
0x0, //
});

public static final BinaryConstant EXIF_IDENTIFIER_CODE = new BinaryConstant(
new byte[] { 0x45, // E
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,12 @@ public byte[] getICCProfileBytes(final ByteSource byteSource, final Map<String,
@Override
public ImageMetadata getMetadata(final ByteSource byteSource, final Map<String, Object> params)
throws ImageReadException, IOException {
final TiffImageMetadata exif = getExifMetadata(byteSource, params);
TiffImageMetadata exif = null;
try {
exif = getExifMetadata(byteSource, params);
} catch (Exception e) {
// exif parse failed.
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should still fail here. Other parsers such as the TiffImageParser fail, so JpegImageParser not throwing an exception would be wrong.

Also, not even logging the exception can cause problems to users of the API that would have to debug the code to understand why they were not seeing the EXIF metadata, just to find an exception that was not propagated.

The rest of the code looks good. Would you be able to provide at least one unit tests for your code @hyunuck, please?

Thanks
Bruno

Copy link
Member

Choose a reason for hiding this comment

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

Ping

Copy link
Member

Choose a reason for hiding this comment

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

Ping AND there are no tests.


final JpegPhotoshopMetadata photoshop = getPhotoshopMetadata(byteSource,
params);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/apache/commons/imaging/formats/jpeg/segments/JfifSegment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public JfifSegment(final int marker, final int markerLength, final InputStream i

final byte[] signature = readBytes(is, JpegConstants.JFIF0_SIGNATURE.size());
if (!JpegConstants.JFIF0_SIGNATURE.equals(signature)
&& !JpegConstants.JFIF0_SIGNATURE_ALTERNATIVE.equals(signature)) {
&& !JpegConstants.JFIF0_SIGNATURE_ALTERNATIVE.equals(signature)
&& !JpegConstants.JFXX0_EXTENSION_SIGNATURE.equals(signature)) {
throw new ImageReadException(
"Not a Valid JPEG File: missing JFIF string");
}
Expand Down