Skip to content

Commit

Permalink
fix: crashing on jp2 decompression (#407)
Browse files Browse the repository at this point in the history
  • Loading branch information
@subotic
subotic authored Jan 26, 2024
1 parent d60af64 commit 93308c2
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 124 deletions.
5 changes: 3 additions & 2 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ add_executable(sipi
src/metadata/SipiIptc.cpp include/metadata/SipiIptc.h
src/metadata/SipiExif.cpp include/metadata/SipiExif.h
src/metadata/SipiEssentials.cpp include/metadata/SipiEssentials.h
src/SipiImage.cpp include/SipiImage.h
src/SipiImage.cpp include/SipiImage.h include/SipiImageError.h
src/formats/SipiIOTiff.cpp include/formats/SipiIOTiff.h
src/formats/SipiIOJ2k.cpp include/formats/SipiIOJ2k.h
src/formats/SipiIOJpeg.cpp include/formats/SipiIOJpeg.h
Expand Down Expand Up @@ -403,7 +403,8 @@ add_executable(sipi
shttps/jwt.c shttps/jwt.h
shttps/makeunique.h
src/SipiFilenameHash.cpp include/SipiFilenameHash.h
include/iiifparser/SipiIdentifier.h src/iiifparser/SipiIdentifier.cpp )
include/iiifparser/SipiIdentifier.h src/iiifparser/SipiIdentifier.cpp
include/SipiImageError.h)

add_dependencies(sipi
icc_profiles)
Expand Down
9 changes: 5 additions & 4 deletions ext/tiff/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ else()
unset(timestamp_policy)
endif()

set(LibTiffVersion "4.1.0")

ExternalProject_Add(project_tiff
DEPENDS jpeg
Expand All @@ -28,11 +29,11 @@ ExternalProject_Add(project_tiff
DEPENDS zstd
INSTALL_DIR ${COMMON_LOCAL}
TEST_BEFORE_INSTALL 1
URL http://download.osgeo.org/libtiff/tiff-4.1.0.tar.gz
URL https://download.osgeo.org/libtiff/tiff-${LibTiffVersion}.tar.gz
DOWNLOAD_DIR ${COMMON_SRCS}/downloads
${timestamp_policy}
SOURCE_DIR ${COMMON_SRCS}/libtiff-4.1.0
PATCH_COMMAND patch ${COMMON_SRCS}/libtiff-4.1.0/libtiff/tif_dirinfo.c < ${COMMON_PATCHES}/tiff-4.0.4.patch
SOURCE_DIR ${COMMON_SRCS}/libtiff-${LibTiffVersion}
PATCH_COMMAND patch ${COMMON_SRCS}/libtiff-${LibTiffVersion}/libtiff/tif_dirinfo.c < ${COMMON_PATCHES}/tiff-4.0.4.patch
CMAKE_ARGS ${CMAKE_CREATE_SHARED}
-DCMAKE_C_STANDARD_LIBRARIES="-lpthread"
-DCMAKE_PREFIX_PATH=${COMMON_LOCAL}
Expand All @@ -42,7 +43,7 @@ ExternalProject_Add(project_tiff
-DWEBP_INCLUDE_DIR=${CONFIGURE_INCDIR}
INSTALL_COMMAND make test
COMMAND make install
COMMAND ${CMAKE_COMMAND} -E copy ${COMMON_SRCS}/libtiff-4.1.0/libtiff/tif_dir.h ${COMMON_LOCAL}/include/
COMMAND ${CMAKE_COMMAND} -E copy ${COMMON_SRCS}/libtiff-${LibTiffVersion}/libtiff/tif_dir.h ${COMMON_LOCAL}/include/
BUILD_IN_SOURCE 1
)
ExternalProject_Get_Property(project_tiff install_dir)
Expand Down
56 changes: 35 additions & 21 deletions include/SipiImage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <string>
#include <unordered_map>
#include <exception>
#include <string>

#include "SipiError.h"
#include "SipiIO.h"
Expand Down Expand Up @@ -90,14 +91,10 @@ namespace Sipi {

enum InfoError { INFO_ERROR };

/*!
* This class implements the error handling for the different image formats.
* It's being derived from the runtime_error so that catching the runtime error
* also catches errors withing reading/writing an image format.
*/
//class SipiImageError : public std::runtime_error {
class SipiImageError : public std::exception {
private:
/*!
* This class implements the error handling for the different image formats.
*/
class SipiImageError final : public std::exception {
std::string file; //!< Source file where the error occurs in
int line; //!< Line within the source file
int errnum; //!< error number if a system call is the reason for the error
Expand All @@ -109,22 +106,30 @@ class SipiImageError : public std::exception {
* Constructor
* \param[in] file_p The source file name (usually __FILE__)
* \param[in] line_p The line number in the source file (usually __LINE__)
* \param[in] Errnum, if a unix system call is the reason for throwing this exception
* \param[in] errnum_p if a unix system call is the reason for throwing this exception
*/
inline SipiImageError(const char *file_p, int line_p, int errnum_p = 0) : file(file_p), line(line_p),
errnum(errnum_p) {}
SipiImageError(const char *file_p, const int line_p, const int errnum_p = 0) : file(file_p), line(line_p), errnum(errnum_p) {
std::ostringstream errStream;
errStream << "Sipi image error at [" << file << ": " << line << "]";
if (errnum != 0) errStream << " (system error: " << std::strerror(errnum) << ")";
errStream << ": " << errmsg;
fullerrmsg = errStream.str();
}

/*!
* Constructor
* \param[in] file_p The source file name (usually __FILE__)
* \param[in] line_p The line number in the source file (usually __LINE__)
* \param[in] msg Error message describing the problem
* \param[in] msg_p Error message describing the problem
* \param[in] errnum_p Errnum, if a unix system call is the reason for throwing this exception
*/
inline SipiImageError(const char *file_p, int line_p, const char *msg_p, int errnum_p = 0) : file(file_p),
line(line_p),
errnum(errnum_p),
errmsg(msg_p) {}
SipiImageError(const char *file_p, const int line_p, const char *msg_p, const int errnum_p = 0) : file(file_p), line(line_p), errnum(errnum_p), errmsg(msg_p) {
std::ostringstream errStream;
errStream << "Sipi image error at [" << file << ": " << line << "]";
if (errnum != 0) errStream << " (system error: " << std::strerror(errnum) << ")";
errStream << ": " << errmsg;
fullerrmsg = errStream.str();
}

/*!
* Constructor
Expand All @@ -133,10 +138,15 @@ class SipiImageError : public std::exception {
* \param[in] msg_p Error message describing the problem
* \param[in] errnum_p Errnum, if a unix system call is the reason for throwing this exception
*/
inline SipiImageError(const char *file_p, int line_p, const std::string &msg_p, int errnum_p = 0) : file(
file_p), line(line_p), errnum(errnum_p), errmsg(msg_p) {}
SipiImageError(const char *file_p, const int line_p, std::string msg_p, const int errnum_p = 0) : file(file_p), line(line_p), errnum(errnum_p), errmsg(std::move(msg_p)) {
std::ostringstream errStream;
errStream << "Sipi image error at [" << file << ": " << line << "]";
if (errnum != 0) errStream << " (system error: " << std::strerror(errnum) << ")";
errStream << ": " << errmsg;
fullerrmsg = errStream.str();
}

inline std::string to_string(void) const {
std::string to_string() const {
std::ostringstream errStream;
errStream << "Sipi image error at [" << file << ": " << line << "]";
if (errnum != 0) errStream << " (system error: " << std::strerror(errnum) << ")";
Expand All @@ -145,8 +155,12 @@ class SipiImageError : public std::exception {
}
//============================================================================

inline friend std::ostream &operator<<(std::ostream &outStream, const SipiImageError &rhs) {
std::string errStr = rhs.to_string();
const char* what() const noexcept override {
return fullerrmsg.c_str();
}

friend std::ostream &operator<<(std::ostream &outStream, const SipiImageError &rhs) {
const std::string errStr = rhs.to_string();
outStream << errStr << std::endl; // TODO: remove the endl, the logging code should do it
return outStream;
}
Expand Down
8 changes: 8 additions & 0 deletions include/SipiImageError.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//
// Created by Ivan Subotic on 26.01.2024.
//

#ifndef SIPIIMAGEERROR_H
#define SIPIIMAGEERROR_H

#endif //SIPIIMAGEERROR_H
56 changes: 5 additions & 51 deletions src/SipiImage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,6 @@ namespace Sipi {
{"jpg", std::make_shared<SipiIOJpeg>()},
{"png", std::make_shared<SipiIOPng>()}};

/* ToDo: remove if everything is OK
std::unordered_map<std::string, std::string> SipiImage::mimetypes = {{"jpx", "image/jp2"},
{"jp2", "image/jp2"},
{"jpg", "image/jpeg"},
{"jpeg", "image/jpeg"},
{"tiff", "image/tiff"},
{"tif", "image/tiff"},
{"png", "image/png"}};
*/

SipiImage::SipiImage() {
nx = 0;
ny = 0;
Expand Down Expand Up @@ -225,50 +215,14 @@ namespace Sipi {
void SipiImage::ensure_exif() {
if (exif == nullptr) exif = std::make_shared<SipiExif>();
}

//============================================================================


/*!
* This function compares the actual mime type of a file (based on its magic number) to
* the given mime type (sent by the client) and the extension of the given filename (sent by the client)
* Reads the image from a file by calling the appropriate reader.
* The readers return either boolean or throw an exception,
* so in any case wrap the call to this method in a try/catch block.
*/
/* ToDo: Delete
bool SipiImage::checkMimeTypeConsistency(const std::string &path, const std::string &given_mimetype,
const std::string &filename) {
try {
std::string actual_mimetype = shttps::Parsing::getFileMimetype(path).first;
if (actual_mimetype != given_mimetype) {
//std::cerr << actual_mimetype << " does not equal " << given_mimetype << std::endl;
return false;
}
size_t dot_pos = filename.find_last_of(".");
if (dot_pos == std::string::npos) {
//std::cerr << "invalid filename " << filename << std::endl;
return false;
}
std::string extension = filename.substr(dot_pos + 1);
std::transform(extension.begin(), extension.end(), extension.begin(), ::tolower); // convert file extension to lower case (uppercase letters in file extension have to be converted for mime type comparison)
std::string mime_from_extension = Sipi::SipiImage::mimetypes.at(extension);
if (mime_from_extension != actual_mimetype) {
//std::cerr << "filename " << filename << "has not mime type " << actual_mimetype << std::endl;
return false;
}
} catch (std::out_of_range &e) {
std::stringstream ss;
ss << "Unsupported file type: \"" << filename;
throw SipiImageError(thisSourceFile, __LINE__, ss.str());
} catch (shttps::Error &err) {
throw SipiImageError(thisSourceFile, __LINE__, err.to_string());
}
return true;
}
*/
void SipiImage::read(const std::string& filepath, int pagenum, const std::shared_ptr<SipiRegion>& region,
const std::shared_ptr<SipiSize>& size, bool force_bps_8, ScalingQuality scaling_quality) {
size_t pos = filepath.find_last_of('.');
Expand Down Expand Up @@ -296,7 +250,7 @@ namespace Sipi {
}

if (!got_file) {
throw SipiImageError(__file__, __LINE__, "Could not read file " + filepath);
throw SipiImageError(__file__, __LINE__, "Error reading file " + filepath);
}
}
//============================================================================
Expand Down
72 changes: 48 additions & 24 deletions src/formats/SipiIOJ2k.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,46 +529,70 @@ namespace Sipi {

//
// the following code directly converts a 16-Bit jpx into an 8-bit image.
// In order to retrieve a 16-Bit image, use kdu_uin16 *buffer an the apropriate signature of the pull_stripe method
// In order to retrieve a 16-Bit image, use kdu_uin16 *buffer and the apropriate signature of the pull_stripe method
//
kdu_supp::kdu_stripe_decompressor decompressor;
decompressor.start(codestream);
int stripe_heights[4] = {dims.size.y, dims.size.y, dims.size.y,
dims.size.y}; // enough for alpha channel (4 components)
//TODO: check image for number of components and make this dynamic
int stripe_heights[5] = {dims.size.y, dims.size.y, dims.size.y, dims.size.y, dims.size.y}; // enough for alpha channel (5 components)

if (force_bps_8) img->bps = 8; // forces kakadu to convert to 8 bit!
switch (img->bps) {
case 8: {
kdu_core::kdu_byte *buffer8 = new kdu_core::kdu_byte[(int) dims.area() * img->nc];
decompressor.pull_stripe(buffer8, stripe_heights);
img->pixels = (byte *) buffer8;
auto *buffer8 = new kdu_core::kdu_byte[static_cast<int>(dims.area()) * img->nc];
try {
decompressor.pull_stripe(buffer8, stripe_heights);
} catch (kdu_exception &exc) {
codestream.destroy();
input->close();
jpx_in.close(); // Not really necessary here.
syslog(LOG_ERR, "Error while decompressing image: %s.", filepath.c_str());
return false;
}
img->pixels = buffer8;
break;
}
case 12: {
std::vector<char> get_signed(img->nc, 0); // vector<bool> does not work -> special treatment in C++
kdu_core::kdu_int16 *buffer16 = new kdu_core::kdu_int16[(int) dims.area() * img->nc];
decompressor.pull_stripe(buffer16,
stripe_heights,
nullptr,
nullptr,
nullptr,
nullptr,
(bool *) get_signed.data());
img->pixels = (byte *) buffer16;
auto *buffer16 = new kdu_core::kdu_int16[(int) dims.area() * img->nc];
try {
decompressor.pull_stripe(buffer16,
stripe_heights,
nullptr,
nullptr,
nullptr,
nullptr,
reinterpret_cast<bool *>(get_signed.data()));
} catch (kdu_exception &exc) {
codestream.destroy();
input->close();
jpx_in.close(); // Not really necessary here.
syslog(LOG_ERR, "Error while decompressing image: %s.", filepath.c_str());
return false;
}
img->pixels = reinterpret_cast<byte *>(buffer16);
img->bps = 16;
break;
}
case 16: {
std::vector<char> get_signed(img->nc, 0); // vector<bool> does not work -> special treatment in C++
kdu_core::kdu_int16 *buffer16 = new kdu_core::kdu_int16[(int) dims.area() * img->nc];
decompressor.pull_stripe(buffer16,
stripe_heights,
nullptr,
nullptr,
nullptr,
nullptr,
(bool *) get_signed.data());
img->pixels = (byte *) buffer16;
auto *buffer16 = new kdu_core::kdu_int16[(int) dims.area() * img->nc];
try {
decompressor.pull_stripe(buffer16,
stripe_heights,
nullptr,
nullptr,
nullptr,
nullptr,
reinterpret_cast<bool *>(get_signed.data()));
} catch (kdu_exception &exc) {
codestream.destroy();
input->close();
jpx_in.close(); // Not really necessary here.
syslog(LOG_ERR, "Error while decompressing image: %s.", filepath.c_str());
return false;
}
img->pixels = reinterpret_cast<byte *>(buffer16);
break;
}
default: {
Expand Down
1 change: 1 addition & 0 deletions test/_test_data/images/unit/.gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
!cielab_2.tif
!cmyk.tif
!cmyk_lossy.jp2
!dev_3229.tif
!gray_with_icc.jp2
!has-missing-sidecar-file.mp4
!has-missing-sidecar-file.mp4.orig
Expand Down
Binary file added test/_test_data/images/unit/dev_3229.tif
View file
Binary file not shown.
12 changes: 2 additions & 10 deletions test/unit/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,8 @@
# This file needs to be updated whenever a new test is added.
#
# all tests can be run from the build directory with `make test`
include(CheckCXXCompilerFlag)
#set(CMAKE_CXX_STANDARD 17)
check_cxx_compiler_flag("-fvisibility-inlines-hidden" SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
if(SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")
endif()
check_cxx_compiler_flag("-fvisibility=hidden" SUPPORTS_FVISIBILITY_FLAG)
if(SUPPORTS_FVISIBILITY_FLAG)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden")
endif()

set(CMAKE_CXX_FLAGS "-O0 -g -std=c++14 -Wall -Wno-uninitialized -Wno-deprecated -Woverloaded-virtual")

find_package(Threads REQUIRED)

Expand Down
15 changes: 3 additions & 12 deletions test/unit/sipiimage/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
include(CheckCXXCompilerFlag)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DCMAKE_CXX_STANDARD=17")

check_cxx_compiler_flag("-fvisibility-inlines-hidden" SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
if(SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")
endif()
check_cxx_compiler_flag("-fvisibility=hidden" SUPPORTS_FVISIBILITY_FLAG)
if(SUPPORTS_FVISIBILITY_FLAG)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden")
endif()

link_directories(
${CONFIGURE_LIBDIR}
Expand Down Expand Up @@ -41,7 +30,8 @@ add_executable(
${PROJECT_SOURCE_DIR}/src/metadata/SipiIptc.cpp ${PROJECT_SOURCE_DIR}/include/metadata/SipiIptc.h
${PROJECT_SOURCE_DIR}/src/metadata/SipiExif.cpp ${PROJECT_SOURCE_DIR}/include/metadata/SipiExif.h
${PROJECT_SOURCE_DIR}/src/metadata/SipiEssentials.cpp ${PROJECT_SOURCE_DIR}/include/metadata/SipiEssentials.h
${PROJECT_SOURCE_DIR}/src/SipiImage.cpp ${PROJECT_SOURCE_DIR}/include/SipiImage.h ${PROJECT_SOURCE_DIR}/include/SipiIO.h
${PROJECT_SOURCE_DIR}/src/SipiImage.cpp ${PROJECT_SOURCE_DIR}/include/SipiImage.h ${PROJECT_SOURCE_DIR}/include/SipiImageError.h
${PROJECT_SOURCE_DIR}/include/SipiIO.h
${PROJECT_SOURCE_DIR}/src/formats/SipiIOTiff.cpp ${PROJECT_SOURCE_DIR}/include/formats/SipiIOTiff.h
${PROJECT_SOURCE_DIR}/src/formats/SipiIOJ2k.cpp ${PROJECT_SOURCE_DIR}/include/formats/SipiIOJ2k.h
${PROJECT_SOURCE_DIR}/src/formats/SipiIOJpeg.cpp ${PROJECT_SOURCE_DIR}/include/formats/SipiIOJpeg.h
Expand Down Expand Up @@ -69,6 +59,7 @@ add_executable(
${PROJECT_SOURCE_DIR}/shttps/Server.cpp ${PROJECT_SOURCE_DIR}/shttps/Server.h
${PROJECT_SOURCE_DIR}/shttps/jwt.c ${PROJECT_SOURCE_DIR}/shttps/jwt.h
${PROJECT_SOURCE_DIR}/shttps/makeunique.h ${PROJECT_SOURCE_DIR}/src/SipiFilenameHash.cpp ${PROJECT_SOURCE_DIR}/include/SipiFilenameHash.h
../../../include/SipiImageError.h
)
add_dependencies(sipiimage icc_profiles)

Expand Down
Loading

0 comments on commit 93308c2

Please sign in to comment.