Merge pull request #2153 from Exiv2/027_FixJp2

0.27 - Fix JP2 write/read metadata
Exiv2 · Mar 23, 2022 · 86c25e2 · 86c25e2
2 parents 060de55 + d03e56e
commit 86c25e2
Show file tree

Hide file tree

Showing 11 changed files with 608 additions and 406 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -19,6 +19,7 @@ add_library( exiv2lib_int OBJECT
     fujimn_int.cpp          fujimn_int.hpp
     helper_functions.cpp    helper_functions.hpp
     image_int.cpp           image_int.hpp
+    jp2image_int.cpp        jp2image_int.hpp
     makernote_int.cpp       makernote_int.hpp
     minoltamn_int.cpp       minoltamn_int.hpp
     nikonmn_int.cpp         nikonmn_int.hpp

diff --git a/src/jp2image.cpp b/src/jp2image.cpp
diff --git a/src/jp2image_int.cpp b/src/jp2image_int.cpp
@@ -0,0 +1,33 @@
+#include "jp2image_int.hpp"
+
+#include <cassert>
+
+#include "error.hpp"
+#include "types.hpp"
+
+namespace Exiv2 {
+namespace Internal {
+    bool isValidBoxFileType(const std::vector<uint8_t> &boxData)
+    {
+        // BR & MinV are obligatory (4 + 4 bytes). Afterwards we have N compatibility
+        // lists (of size 4)
+        if ((boxData.size() - 8u) % 4u != 0) {
+            return false;
+        }
+
+        const size_t N = (boxData.size() - 8u) / 4u;
+        const uint32_t brand = getULong(boxData.data(), bigEndian);
+        const uint32_t minorVersion = getULong(boxData.data() + 4, bigEndian);
+
+        bool clWithRightBrand = false;
+        for (size_t i = 0; i < N; i++) {
+            uint32_t compatibilityList = getULong(boxData.data() + 8 + i * 4, bigEndian);
+            if (compatibilityList == brandJp2) {
+                clWithRightBrand = true;
+                break;
+            }
+        }
+        return (brand == brandJp2 && minorVersion == 0 && clWithRightBrand);
+    }
+}
+}
diff --git a/src/jp2image_int.hpp b/src/jp2image_int.hpp
@@ -0,0 +1,40 @@
+#ifndef JP2IMAGE_INT_HPP
+#define JP2IMAGE_INT_HPP
+
+#include <stdint.h>
+#include <vector>
+
+namespace Exiv2
+{
+    namespace Internal
+    {
+        struct Jp2BoxHeader
+        {
+            uint32_t length;
+            uint32_t type;
+        };
+
+        struct Jp2ImageHeaderBox
+        {
+            uint32_t imageHeight;
+            uint32_t imageWidth;
+            uint16_t componentCount;
+            uint8_t bpc;   //<! Bits per component
+            uint8_t c;     //<! Compression type
+            uint8_t unkC;  //<! Colourspace unknown
+            uint8_t ipr;   //<! Intellectual property
+        };
+
+        struct Jp2UuidBox
+        {
+            uint8_t uuid[16];
+        };
+
+        const uint32_t brandJp2 = 0x6a703220;
+
+        /// @brief Determines if the File Type box is valid
+        bool isValidBoxFileType(const std::vector<uint8_t>& boxData);
+    }  // namespace Internal
+}  // namespace Exiv2
+
+#endif  // JP2IMAGE_INT_HPP
diff --git a/test/data/icc-test.out b/test/data/icc-test.out
@@ -735,23 +735,23 @@ STRUCTURE OF JPEG2000 FILE: Reagan2.jp2
       12 |       20 | ftyp      | 
       32 |     3185 | jp2h      | 
       40 |       22 |  sub:ihdr | .............
-      62 |     3155 |  sub:colr | ......HLino....mntrRGB XYZ .. | pad: 2 0 0 | iccLength:3144
+      62 |     3155 |  sub:colr | ......HLino....mntrRGB XYZ .. | iccLength:3144
     3217 |        0 | jp2c      | 
 STRUCTURE OF JPEG2000 FILE: Reagan2.jp2
  address |   length | box       | data
        0 |       12 | jP        | 
       12 |       20 | ftyp      | 
       32 |  1613641 | jp2h      | 
       40 |       22 |  sub:ihdr | .............
-      62 |  1613611 |  sub:colr | ...... APPL....prtrRGB Lab .. | pad: 2 0 0 | iccLength:1613600
+      62 |  1613611 |  sub:colr | ...... APPL....prtrRGB Lab .. | iccLength:1613600
  1613673 |        0 | jp2c      | 
 STRUCTURE OF JPEG2000 FILE: Reagan2.jp2
  address |   length | box       | data
        0 |       12 | jP        | 
       12 |       20 | ftyp      | 
       32 |      601 | jp2h      | 
       40 |       22 |  sub:ihdr | .............
-      62 |      571 |  sub:colr | ......0ADBE....mntrRGB XYZ .. | pad: 2 0 0 | iccLength:560
+      62 |      571 |  sub:colr | ......0ADBE....mntrRGB XYZ .. | iccLength:560
      633 |        0 | jp2c      | 
 1d3fda2edb4a89ab60a23c5f7c7d81dd
 1d3fda2edb4a89ab60a23c5f7c7d81dd

diff --git a/tests/bugfixes/github/test_issue_1845.py b/tests/bugfixes/github/test_issue_1845.py
@@ -12,6 +12,9 @@ class TiffDirectoryWriteDirEntryAssert(metaclass=CaseMeta):
 
     filename = path("$tmp_path/issue_1845_poc.jp2")
     commands = ["$exiv2 -q -D +1 ad $filename"]
-    stderr = [""]
+    stderr = [
+        """$exception_in_adjust """ + filename + """:
+$kerCorruptedMetadata
+"""]
     stdout = [""]
-    retval = [0]
+    retval = [1]
diff --git a/tests/bugfixes/github/test_issue_ghsa_mxw9_qx4c_6m8v.py b/tests/bugfixes/github/test_issue_ghsa_mxw9_qx4c_6m8v.py
@@ -13,6 +13,8 @@ class Jp2ImageEncodeJp2HeaderOutOfBoundsRead2(metaclass=CaseMeta):
     filename = path("$tmp_path/issue_ghsa_mxw9_qx4c_6m8v_poc.jp2")
     commands = ["$exiv2 rm $filename"]
     stdout = [""]
-    retval = [0]
-
-    compare_stderr = check_no_ASAN_UBSAN_errors
+    stderr = [
+        """$exception_in_erase """ + filename + """:
+$kerCorruptedMetadata
+"""]
+    retval = [1]
diff --git a/tests/suite.conf b/tests/suite.conf
@@ -48,5 +48,7 @@ addition_overflow_message: Overflow in addition
 exiv2_exception_message: Exiv2 exception in print action for file
 exiv2_overflow_exception_message: std::overflow_error exception in print action for file
 exception_in_extract: Exiv2 exception in extract action for file
+exception_in_adjust: Exiv2 exception in adjust action for file
+exception_in_erase: Exiv2 exception in erase action for file
 uncaught_exception: Uncaught exception:
 no_exif_data_found_retval: 253
diff --git a/unitTests/CMakeLists.txt b/unitTests/CMakeLists.txt
@@ -12,6 +12,7 @@ add_executable(unit_tests
     test_futils.cpp
     test_helper_functions.cpp
     test_image_int.cpp
+    test_jp2image.cpp
     test_safe_op.cpp
     test_slice.cpp
     test_tiffheader.cpp

diff --git a/unitTests/test_basicio.cpp b/unitTests/test_basicio.cpp
@@ -22,6 +22,55 @@
 #include <gtest/gtest.h>
 using namespace Exiv2;
 
+TEST(MemIo_Default, readEReturns0)
+{
+    std::vector<byte> buf(10);
+    MemIo io;
+    ASSERT_EQ(0, io.read(buf.data(), (long)buf.size()));
+}
+
+TEST(MemIo_Default, isNotAtEof)
+{
+    MemIo io;
+    ASSERT_FALSE(io.eof());
+}
+
+TEST(MemIo_Default, seekBeyondBufferSizeReturns1AndSetsEofToTrue)
+{
+    MemIo io;
+    ASSERT_EQ(1, io.seek(1, BasicIo::beg));
+    ASSERT_TRUE(io.eof());
+}
+
+TEST(MemIo_Default, seekBefore0Returns1ButItDoesNotSetEofToTrue)
+{
+    MemIo io;
+    ASSERT_EQ(1, io.seek(-1, BasicIo::beg));
+    ASSERT_FALSE(io.eof());
+}
+
+TEST(MemIo_Default, seekToEndPosition_doesNotTriggerEof)
+{
+    MemIo io;
+    ASSERT_EQ(0, io.tell());
+    ASSERT_EQ(0, io.seek(0, BasicIo::end));
+    ASSERT_EQ(0, io.tell());
+    ASSERT_FALSE(io.eof());
+}
+
+TEST(MemIo_Default, seekToEndPositionAndReadTriggersEof)
+{
+    MemIo io;
+    ASSERT_EQ(0, io.seek(0, BasicIo::end));
+    ASSERT_EQ(0, io.tell());
+
+    std::vector<byte> buf2(64, 0);
+    ASSERT_EQ(0, io.read(buf2.data(), 1)); // Note that we cannot even read 1 byte being at the end
+    ASSERT_TRUE(io.eof());
+}
+
+// -------------------------
+
 TEST(MemIo, seek_out_of_bounds_00)
 {
     byte buf[1024];

diff --git a/unitTests/test_jp2image.cpp b/unitTests/test_jp2image.cpp
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <exiv2/jp2image.hpp>
+#include <exiv2/error.hpp>
+
+#include <gtest/gtest.h>
+
+using namespace Exiv2;
+
+TEST(Jp2Image, canBeCreatedFromScratch)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    ASSERT_NO_THROW(newJp2Instance(memIo, create));
+}
+
+TEST(Jp2Image, canBeOpenedEvenWithAnEmptyMemIo)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = false;
+    ASSERT_NO_THROW(newJp2Instance(memIo, create));
+}
+
+TEST(Jp2Image, mimeTypeIsPng)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+    ASSERT_EQ("image/jp2", image->mimeType());
+}
+
+TEST(Jp2Image, printStructurePrintsNothingWithKpsNone)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+
+    std::ostringstream stream;
+    image->printStructure(stream, Exiv2::kpsNone, 1);
+
+    ASSERT_TRUE(stream.str().empty());
+}
+
+TEST(Jp2Image, printStructurePrintsDataWithKpsBasic)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+
+    std::ostringstream stream;
+    image->printStructure(stream, Exiv2::kpsBasic, 1);
+
+    ASSERT_FALSE(stream.str().empty());
+}
+
+TEST(Jp2Image, cannotReadMetadataFromEmptyIo)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = false;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+    ASSERT_TRUE(image.get() == NULL);
+}
+
+TEST(Jp2Image, cannotReadMetadataFromIoWhichCannotBeOpened)
+{
+    Exiv2::BasicIo::AutoPtr io (new Exiv2::FileIo("NonExistingPath.jp2"));
+    const bool create = false;
+    Image::AutoPtr image = newJp2Instance(io, create);
+    ASSERT_TRUE(image.get() == NULL);
+}
+
+TEST(Jp2Image, cannotWriteMetadataToEmptyIo)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = false;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+    ASSERT_TRUE(image.get() == NULL);
+}
+
+TEST(Jp2Image, canWriteMetadataFromCreatedJp2Image)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+    ASSERT_NO_THROW(image->writeMetadata());
+}
+
+TEST(Jp2Image, canWriteMetadataAndReadAfterwards)
+{
+    Exiv2::BasicIo::AutoPtr memIo (new Exiv2::MemIo);
+    const bool create = true;
+    Image::AutoPtr image = newJp2Instance(memIo, create);
+
+    ASSERT_NO_THROW(image->writeMetadata());
+    ASSERT_NO_THROW(image->readMetadata());
+}