[GitHub] commons-imaging pull request #36: Avoid jpeg parse fail due to jfxx and exif

[hyunuck <gi...@git.apache.org>]

GitHub user hyunuck opened a pull request:

    https://github.com/apache/commons-imaging/pull/36

    Avoid jpeg parse fail due to jfxx and exif

    - add jfxx signature when parsing jpeg
    - Avoid jpeg parse fail due to exif

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hyunuck/commons-imaging master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/commons-imaging/pull/36.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #36
    
----
commit e337c954847bf3a5ff17b64ec17c45916280fe31
Author: Justin <hu...@...>
Date:   2018-03-28T14:41:52Z

    add jfxx signature when parsing jpeg

commit 8ad11248834e21154a09fdeb9c130bd800ea8250
Author: Justin <hu...@...>
Date:   2018-03-28T14:44:45Z

    Avoid jpeg parse fail due to exif

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

[GitHub] commons-imaging pull request #36: Avoid jpeg parse fail due to jfxx and exif

[kinow <gi...@git.apache.org>]

Github user kinow commented on a diff in the pull request:

    https://github.com/apache/commons-imaging/pull/36#discussion_r204226858
  
    --- Diff: src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java ---
    @@ -312,7 +312,12 @@ private void dumpSegments(final List<? extends Segment> v) {
         @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.
    +        }
    --- End diff --
    
    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


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org