Mathern/remove obsolete apis memfixes

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()

include/c2pa.hpp

@@ -567,7 +567,8 @@ namespace c2pa
         explicit CppIStream(IStream &istream) {
             static_assert(std::is_base_of<std::istream, IStream>::value,
                       "Stream must be derived from std::istream");
-            c_stream = c2pa_create_stream(reinterpret_cast<StreamContext *>(&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<StreamContext *>(static_cast<std::istream *>(&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 <typename OStream>
         explicit CppOStream(OStream &ostream) {
             static_assert(std::is_base_of<std::ostream, OStream>::value, "Stream must be derived from std::ostream");
-            c_stream = c2pa_create_stream(reinterpret_cast<StreamContext *>(&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<StreamContext *>(static_cast<std::ostream *>(&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 <typename IOStream>
         CppIOStream(IOStream &iostream) {
             static_assert(std::is_base_of<std::iostream, IOStream>::value, "Stream must be derived from std::iostream");
-            c_stream = c2pa_create_stream(reinterpret_cast<StreamContext *>(&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<StreamContext *>(static_cast<std::iostream *>(&iostream)), reader, seeker, writer, flusher);
             if (c_stream == nullptr) {
                 throw C2paException("Failed to create I/O stream wrapper: is stream open and valid?");
             }

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) {

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;
     }
 

src/c2pa_internal.hpp

@@ -46,9 +46,16 @@ inline std::vector<std::string> c_mime_types_to_vector(const char* const* mime_t
   std::vector<std::string> 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<std::iostream> {
 };
 
 /// Seeker impl.
+/// Exceptions must not unwind into Rust/C, so any throw
+/// is converted to an IoError return.
 template<typename Stream>
 intptr_t stream_seeker(StreamContext* context, intptr_t offset, C2paSeekMode whence) {
-    auto* stream = reinterpret_cast<Stream*>(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<Stream>::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<Stream>::tell(stream);
-    if (pos < 0) {
+    try {
+        auto* stream = reinterpret_cast<Stream*>(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<Stream>::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<Stream>::tell(stream);
+        if (pos < 0) {
+            return stream_error_return(StreamError::IoError);
+        }
+        return static_cast<intptr_t>(pos);
+    } catch (...) {
         return stream_error_return(StreamError::IoError);
     }
-    return static_cast<intptr_t>(pos);
 }
 
 /// Reader impl.
+/// Exceptions must not unwind into Rust/C, so any throw
+/// is converted to an IoError return.
 template<typename Stream>
 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<Stream*>(context);
-    if (!is_stream_usable(stream)) {
-        return stream_error_return(StreamError::IoError);
-    }
-    stream->read(reinterpret_cast<char*>(buffer), size);
-    if (stream->fail()) {
-        if (!stream->eof()) {
-            return stream_error_return(StreamError::InvalidArgument);
+    try {
+        auto* stream = reinterpret_cast<Stream*>(context);
+        if (!is_stream_usable(stream)) {
+            return stream_error_return(StreamError::IoError);
         }
-    }
-    if (stream->bad()) {
+        stream->read(reinterpret_cast<char*>(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<intptr_t>(stream->gcount());
+    } catch (...) {
         return stream_error_return(StreamError::IoError);
     }
-    return static_cast<intptr_t>(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<typename Stream, typename Op>
 intptr_t stream_op(StreamContext* context, Op op) {
-    auto* stream = reinterpret_cast<Stream*>(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<Stream*>(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<unsigned char> 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<unsigned char>(data, data + size);
     c2pa_free(data);

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<CppIStream>(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<CppIStream>(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,23 +72,24 @@ 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<std::ifstream>(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());
         }
 
         std::string extension = detail::extract_file_extension(source_path);
 
         // CppIStream stores reference to owned_stream, which lives as long as Reader
         cpp_stream = std::make_unique<CppIStream>(*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<std::string> 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)

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();
         }
     }

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<std::string> &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()

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<c2pa::Context>();
+    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<c2pa::Context>();
+    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);
+}