diff --git a/CMakeLists.txt b/CMakeLists.txt index fb21e13..8ae6ae4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,15 @@ set(CMAKE_C_STANDARD 17) set(CMAKE_CXX_STANDARD 17) set(CMAKE_OSX_DEPLOYMENT_TARGET "13.3") +# Default to an optimized build when the caller didn't pick a type. +# Skipped for multi-config generators (Visual Studio, Xcode), which choose at build time. +get_property(_is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) +if(NOT _is_multi_config AND NOT CMAKE_BUILD_TYPE) + set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "Build type" FORCE) + set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS + Debug Release RelWithDebInfo MinSizeRel) +endif() + if(MSVC) add_compile_options(/utf-8) endif() diff --git a/include/c2pa.hpp b/include/c2pa.hpp index 91fbfe5..f78ed6f 100644 --- a/include/c2pa.hpp +++ b/include/c2pa.hpp @@ -567,7 +567,8 @@ namespace c2pa explicit CppIStream(IStream &istream) { static_assert(std::is_base_of::value, "Stream must be derived from std::istream"); - c_stream = c2pa_create_stream(reinterpret_cast(&istream), reader, seeker, writer, flusher); + // Upcast to std::istream* before type erasure; the callbacks cast the context back to std::istream* + c_stream = c2pa_create_stream(reinterpret_cast(static_cast(&istream)), reader, seeker, writer, flusher); if (c_stream == nullptr) { throw C2paException("Failed to create input stream wrapper: is stream open and valid?"); } @@ -628,7 +629,8 @@ namespace c2pa template explicit CppOStream(OStream &ostream) { static_assert(std::is_base_of::value, "Stream must be derived from std::ostream"); - c_stream = c2pa_create_stream(reinterpret_cast(&ostream), reader, seeker, writer, flusher); + // Upcast to std::ostream* before type erasure; the callbacks cast the context back to std::ostream* + c_stream = c2pa_create_stream(reinterpret_cast(static_cast(&ostream)), reader, seeker, writer, flusher); if (c_stream == nullptr) { throw C2paException("Failed to create output stream wrapper: is stream open and valid?"); } @@ -687,7 +689,8 @@ namespace c2pa template CppIOStream(IOStream &iostream) { static_assert(std::is_base_of::value, "Stream must be derived from std::iostream"); - c_stream = c2pa_create_stream(reinterpret_cast(&iostream), reader, seeker, writer, flusher); + // Upcast to std::iostream* before type erasure; the callbacks cast the context back to std::iostream* + c_stream = c2pa_create_stream(reinterpret_cast(static_cast(&iostream)), reader, seeker, writer, flusher); if (c_stream == nullptr) { throw C2paException("Failed to create I/O stream wrapper: is stream open and valid?"); } diff --git a/src/c2pa_builder.cpp b/src/c2pa_builder.cpp index 58f8667..e68dc9c 100644 --- a/src/c2pa_builder.cpp +++ b/src/c2pa_builder.cpp @@ -51,8 +51,8 @@ namespace c2pa init_from_context(context); // Apply the manifest definition to the Builder. - // Note: c2pa_builder_with_definition always consumes the builder pointer, - // so the original pointer is invalid after this call regardless of success/error. + // Note: c2pa_builder_with_definition consumes the builder pointer on success + // and on operation failure. C2paBuilder* updated = c2pa_builder_with_definition(builder, manifest_json.c_str()); builder = nullptr; if (updated == nullptr) { diff --git a/src/c2pa_context.cpp b/src/c2pa_context.cpp index bcecb7c..5337495 100644 --- a/src/c2pa_context.cpp +++ b/src/c2pa_context.cpp @@ -183,7 +183,11 @@ namespace c2pa if (!raw) { throw C2paException("Signer is not valid"); } - c2pa_context_builder_set_signer(context_builder, raw); + // On error the signer may not have been consumed by the C API, + // surface an error + if (c2pa_context_builder_set_signer(context_builder, raw) != 0) { + throw C2paException(); + } return *this; } diff --git a/src/c2pa_internal.hpp b/src/c2pa_internal.hpp index e1fe8ab..1cf16e0 100644 --- a/src/c2pa_internal.hpp +++ b/src/c2pa_internal.hpp @@ -46,9 +46,16 @@ inline std::vector c_mime_types_to_vector(const char* const* mime_t std::vector result; if (mime_types == nullptr) { return result; } - result.reserve(count); - for(uintptr_t i = 0; i < count; i++) { - result.emplace_back(mime_types[i]); + try { + result.reserve(count); + for(uintptr_t i = 0; i < count; i++) { + if (mime_types[i] != nullptr) { + result.emplace_back(mime_types[i]); + } + } + } catch (...) { + c2pa_free_string_array(mime_types, count); + throw; } c2pa_free_string_array(mime_types, count); @@ -107,29 +114,37 @@ struct StreamSeekTraits { }; /// Seeker impl. +/// Exceptions must not unwind into Rust/C, so any throw +/// is converted to an IoError return. template intptr_t stream_seeker(StreamContext* context, intptr_t offset, C2paSeekMode whence) { - auto* stream = reinterpret_cast(context); - if (!is_stream_usable(stream)) { - return stream_error_return(StreamError::IoError); - } - const std::ios_base::seekdir dir = whence_to_seekdir(whence); - stream->clear(); - StreamSeekTraits::seek(stream, offset, dir); - if (stream->fail()) { - return stream_error_return(StreamError::InvalidArgument); - } - if (stream->bad()) { - return stream_error_return(StreamError::IoError); - } - const int64_t pos = StreamSeekTraits::tell(stream); - if (pos < 0) { + try { + auto* stream = reinterpret_cast(context); + if (!is_stream_usable(stream)) { + return stream_error_return(StreamError::IoError); + } + const std::ios_base::seekdir dir = whence_to_seekdir(whence); + stream->clear(); + StreamSeekTraits::seek(stream, offset, dir); + if (stream->fail()) { + return stream_error_return(StreamError::InvalidArgument); + } + if (stream->bad()) { + return stream_error_return(StreamError::IoError); + } + const int64_t pos = StreamSeekTraits::tell(stream); + if (pos < 0) { + return stream_error_return(StreamError::IoError); + } + return static_cast(pos); + } catch (...) { return stream_error_return(StreamError::IoError); } - return static_cast(pos); } /// Reader impl. +/// Exceptions must not unwind into Rust/C, so any throw +/// is converted to an IoError return. template intptr_t stream_reader(StreamContext* context, uint8_t* buffer, intptr_t size) { if (!context || !buffer) { @@ -141,37 +156,47 @@ intptr_t stream_reader(StreamContext* context, uint8_t* buffer, intptr_t size) { if (size == 0) { return 0; } - auto* stream = reinterpret_cast(context); - if (!is_stream_usable(stream)) { - return stream_error_return(StreamError::IoError); - } - stream->read(reinterpret_cast(buffer), size); - if (stream->fail()) { - if (!stream->eof()) { - return stream_error_return(StreamError::InvalidArgument); + try { + auto* stream = reinterpret_cast(context); + if (!is_stream_usable(stream)) { + return stream_error_return(StreamError::IoError); } - } - if (stream->bad()) { + stream->read(reinterpret_cast(buffer), size); + if (stream->fail()) { + if (!stream->eof()) { + return stream_error_return(StreamError::InvalidArgument); + } + } + if (stream->bad()) { + return stream_error_return(StreamError::IoError); + } + return static_cast(stream->gcount()); + } catch (...) { return stream_error_return(StreamError::IoError); } - return static_cast(stream->gcount()); } /// Get stream from context, used by writer and flusher. +/// Exceptions must not unwind into Rust/C, so any throw +/// is converted to an IoError return. template intptr_t stream_op(StreamContext* context, Op op) { - auto* stream = reinterpret_cast(context); - if (!is_stream_usable(stream)) { - return stream_error_return(StreamError::IoError); - } - const intptr_t result = op(stream); - if (stream->fail()) { - return stream_error_return(StreamError::InvalidArgument); - } - if (stream->bad()) { + try { + auto* stream = reinterpret_cast(context); + if (!is_stream_usable(stream)) { + return stream_error_return(StreamError::IoError); + } + const intptr_t result = op(stream); + if (stream->fail()) { + return stream_error_return(StreamError::InvalidArgument); + } + if (stream->bad()) { + return stream_error_return(StreamError::IoError); + } + return result; + } catch (...) { return stream_error_return(StreamError::IoError); } - return result; } /// Writer impl. @@ -236,12 +261,18 @@ inline std::string c_string_to_string(T* c_result) { /// @return Vector containing the bytes (throws if null or negative size) /// @details This helper extracts the pattern of checking C API results, /// copying to a vector, and freeing the C-allocated memory. -/// The C API contract is: if result < 0 or data == nullptr, the operation failed. +/// The C API contract is: if result < 0, the operation failed. A null +/// data pointer with size == 0 is a valid empty result (the C API +/// returns null for empty byte arrays). inline std::vector to_byte_vector(const unsigned char* data, int64_t size) { - if (size < 0 || data == nullptr) { + if (size < 0 || (data == nullptr && size > 0)) { c2pa_free(data); // May be null or allocated, c2pa_free handles both throw C2paException(); } + if (size == 0) { + c2pa_free(data); + return {}; + } auto result = std::vector(data, data + size); c2pa_free(data); diff --git a/src/c2pa_reader.cpp b/src/c2pa_reader.cpp index 909e61e..a2301a0 100644 --- a/src/c2pa_reader.cpp +++ b/src/c2pa_reader.cpp @@ -46,15 +46,16 @@ namespace c2pa throw C2paException("Invalid Context provider IContextProvider"); } + // Create the stream wrapper before the reader handle + cpp_stream = std::make_unique(stream); + c2pa_reader = c2pa_reader_from_context(context.c_context()); if (c2pa_reader == nullptr) { throw C2paException("Failed to create reader from context"); } - cpp_stream = std::make_unique(stream); // Update reader with stream. - // Note: c2pa_reader_with_stream always consumes the reader pointer, - // so the original pointer is invalid after this call regardless of success/error. + // Note: c2pa_reader_with_stream consumes the reader pointer. C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, format.c_str(), cpp_stream->c_stream); c2pa_reader = nullptr; if (updated == nullptr) { @@ -71,15 +72,10 @@ namespace c2pa throw C2paException("Invalid Context provider IContextProvider"); } - c2pa_reader = c2pa_reader_from_context(context.c_context()); - if (c2pa_reader == nullptr) { - throw C2paException("Failed to create reader from context"); - } - - // Create owned stream that will live as long as the Reader + // Create the streams before the reader handle. + // Create owned stream that will live as long as the Reader. owned_stream = std::make_unique(source_path, std::ios::binary); if (!owned_stream->is_open()) { - c2pa_free(c2pa_reader); throw std::system_error(errno, std::system_category(), "Failed to open file: " + source_path.string()); } @@ -87,7 +83,13 @@ namespace c2pa // CppIStream stores reference to owned_stream, which lives as long as Reader cpp_stream = std::make_unique(*owned_stream); - // Note: c2pa_reader_with_stream always consumes the reader pointer. + + c2pa_reader = c2pa_reader_from_context(context.c_context()); + if (c2pa_reader == nullptr) { + throw C2paException("Failed to create reader from context"); + } + + // Note: c2pa_reader_with_stream consumes the reader pointer. C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, extension.c_str(), cpp_stream->c_stream); c2pa_reader = nullptr; if (updated == nullptr) { @@ -175,15 +177,20 @@ namespace c2pa [[nodiscard]] std::optional Reader::remote_url() const { auto url = c2pa_reader_remote_url(c2pa_reader); if (url == nullptr) { return std::nullopt; } - std::string url_str(url); // The C2PA library returns a `const char*` that needs to be released. // The underlying `char*` is mutable; however, to indicate the value // shouldn't be modified, it's returned as a const char*. // // TODO: Revisit after determining how we want c2pa-rs to handle // strings that shouldn't be modified by our bindings. - c2pa_free(url); - return url_str; + try { + std::string url_str(url); + c2pa_free(url); + return url_str; + } catch (...) { + c2pa_free(url); + throw; + } } int64_t Reader::get_resource(const std::string &uri, const std::filesystem::path &path) diff --git a/src/c2pa_settings.cpp b/src/c2pa_settings.cpp index be989ec..abdec65 100644 --- a/src/c2pa_settings.cpp +++ b/src/c2pa_settings.cpp @@ -34,6 +34,7 @@ namespace c2pa } if (c2pa_settings_update_from_string(settings_ptr, data.c_str(), format.c_str()) != 0) { c2pa_free(settings_ptr); + settings_ptr = nullptr; throw C2paException(); } } diff --git a/src/c2pa_signer.cpp b/src/c2pa_signer.cpp index 8172f53..9e6954e 100644 --- a/src/c2pa_signer.cpp +++ b/src/c2pa_signer.cpp @@ -71,12 +71,20 @@ namespace c2pa { // Pass the C++ callback as a context to our static callback wrapper. signer = c2pa_signer_create((const void *)callback, &signer_passthrough, alg, sign_cert.c_str(), validate_tsa_uri(tsa_uri)); + if (signer == nullptr) + { + throw C2paException(); + } } Signer::Signer(const std::string &alg, const std::string &sign_cert, const std::string &private_key, const std::optional &tsa_uri) { auto info = C2paSignerInfo { alg.c_str(), sign_cert.c_str(), private_key.c_str(), validate_tsa_uri(tsa_uri) }; signer = c2pa_signer_from_info(&info); + if (signer == nullptr) + { + throw C2paException(); + } } Signer::~Signer() diff --git a/tests/builder.test.cpp b/tests/builder.test.cpp index 95ef03c..d61d9db 100644 --- a/tests/builder.test.cpp +++ b/tests/builder.test.cpp @@ -6005,3 +6005,51 @@ TEST_F(BuilderTest, ArchiveIngredientWithProvenanceRoundTripAndReuse) EXPECT_EQ(out_ingredients[0]["title"], "C.jpg"); EXPECT_EQ(out_ingredients[0]["relationship"], "componentOf"); } + +TEST(SignerTest, InvalidCredentialsThrowFromConstructor) { + EXPECT_THROW( + c2pa::Signer("Es256", "not a certificate", "not a private key"), + c2pa::C2paException); +} + +TEST_F(BuilderTest, SignSourceStreamWithExceptions) { + auto image_path = c2pa_test::get_fixture_path("A.jpg"); + auto manifest = c2pa_test::read_text_file(c2pa_test::get_fixture_path("training.json")); + + auto context = std::make_shared(); + auto builder = c2pa::Builder(context, manifest); + auto signer = c2pa_test::create_test_signer(); + + std::ifstream source(image_path, std::ios::binary); + ASSERT_TRUE(source.is_open()); + source.exceptions(std::ios::failbit | std::ios::badbit); + + std::stringstream memory_buffer(std::ios::in | std::ios::out | std::ios::binary); + std::iostream& dest = memory_buffer; + + try { + auto manifest_data = builder.sign("image/jpeg", source, dest, signer); + EXPECT_FALSE(manifest_data.empty()); + } catch (const c2pa::C2paException&) { + // An error result is acceptable; crossing the FFI with an exception is not. + } +} + +TEST_F(BuilderTest, ArchiveToFstreamBackedCppOStream) { + auto manifest = c2pa_test::read_text_file(c2pa_test::get_fixture_path("training.json")); + + auto context = std::make_shared(); + auto builder = c2pa::Builder(context, manifest); + + auto archive_path = get_temp_path("archive_fstream_cppostream.bin"); + std::fstream dest(archive_path, + std::ios_base::binary | std::ios_base::trunc | + std::ios_base::in | std::ios_base::out); + ASSERT_TRUE(dest.is_open()); + + c2pa::CppOStream c_dest(dest); + ASSERT_EQ(c2pa_builder_to_archive(builder.c2pa_builder(), c_dest.c_stream), 0); + dest.flush(); + + EXPECT_GT(std::filesystem::file_size(archive_path), 0u); +} diff --git a/tests/reader.test.cpp b/tests/reader.test.cpp index 68827ee..9ab8a4e 100644 --- a/tests/reader.test.cpp +++ b/tests/reader.test.cpp @@ -718,3 +718,21 @@ TEST_F(ReaderTest, ReadCrJsonSpecialChars) auto crjson = reader.crjson(); EXPECT_FALSE(crjson.empty()); } + +TEST(Reader, StreamWithExceptions) { + fs::path current_dir = fs::path(__FILE__).parent_path(); + fs::path test_file = current_dir / "../tests/fixtures/C.jpg"; + + std::ifstream stream(test_file, std::ios::binary); + ASSERT_TRUE(stream.is_open()); + stream.exceptions(std::ios::failbit | std::ios::badbit); + + auto context = std::make_shared(); + try { + auto reader = c2pa::Reader(context, "image/jpeg", stream); + auto json_result = reader.json(); + EXPECT_FALSE(json_result.empty()); + } catch (const c2pa::C2paException&) { + // An error result is acceptable; crossing the FFI with an exception is not. + } +}