fix: Clarify ownership handling on error on C FFI calls

src/c2pa_builder.cpp

@@ -116,11 +116,18 @@ namespace c2pa
             throw C2paException();
         }
 
-        builder = c2pa_builder_with_archive(base, c_archive.c_stream);
-        base = nullptr;
-        if (builder == nullptr) {
-            throw C2paException();
+        // On success this consumes base, on error base is retained.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
+        C2paBuilder* result = c2pa_builder_with_archive(base, c_archive.c_stream);
+        if (result == nullptr) {
+            C2paException ex;
+            c2pa_free(base);
+            base = nullptr;
+            throw ex;
         }
+        builder = result;
+        base = nullptr;
     }
 
     Builder::~Builder()
@@ -332,11 +339,15 @@ namespace c2pa
     {
         CppIStream c_archive(archive);
 
-        // c2pa_builder_with_archive consumes the builder pointer and returns a new one
+        // On success this consumes the builder and returns a new one.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
         C2paBuilder* updated = c2pa_builder_with_archive(builder, c_archive.c_stream);
-        builder = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            C2paException ex;
+            c2pa_free(builder);
+            builder = nullptr;
+            throw ex;
         }
         builder = updated;
         return *this;

src/c2pa_reader.cpp

@@ -55,11 +55,15 @@ namespace c2pa
         }
 
         // Update reader with stream.
-        // Note: c2pa_reader_with_stream consumes the reader pointer.
+        // On success c2pa_reader_with_stream consumes the reader.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
         C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, format.c_str(), cpp_stream->c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            C2paException ex;      // owns the error message before we free
+            c2pa_free(c2pa_reader);
+            c2pa_reader = nullptr;
+            throw ex;
         }
         c2pa_reader = updated;
     }
@@ -89,11 +93,15 @@ namespace c2pa
             throw C2paException("Failed to create reader from context");
         }
 
-        // Note: c2pa_reader_with_stream consumes the reader pointer.
+        // On success c2pa_reader_with_stream consumes the reader.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
         C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, extension.c_str(), cpp_stream->c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            C2paException ex;
+            c2pa_free(c2pa_reader);
+            c2pa_reader = nullptr;
+            throw ex;
         }
         c2pa_reader = updated;
     }
@@ -118,16 +126,20 @@ namespace c2pa
             throw C2paException("Failed to create reader from context");
         }
 
-        // c2pa_reader_with_manifest_data_and_stream always consumes c2pa_reader.
+        // On success this consumes c2pa_reader.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
         C2paReader* updated = c2pa_reader_with_manifest_data_and_stream(
             c2pa_reader,
             format.c_str(),
             cpp_stream->c_stream,
             manifest_jumbf.data(),
             manifest_jumbf.size());
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            C2paException ex;
+            c2pa_free(c2pa_reader);
+            c2pa_reader = nullptr;
+            throw ex;
         }
         c2pa_reader = updated;
 
@@ -187,16 +199,19 @@ namespace c2pa
         CppIStream main_wrapper(stream);
         CppIStream fragment_wrapper(fragment);
 
-        // c2pa_reader_with_fragment consumes the existing reader and returns a new one.
-        // *this is returned for chaining so reading can go through all segments.
+        // On success c2pa_reader_with_fragment consumes the existing reader and returns a new one.
+        // On error it can be retained and we need to free explicitly
+        // (c2pa_free handles verifying free is possible).
         C2paReader* updated = c2pa_reader_with_fragment(
             c2pa_reader,
             format.c_str(),
             main_wrapper.c_stream,
             fragment_wrapper.c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            C2paException ex;
+            c2pa_free(c2pa_reader);
+            c2pa_reader = nullptr;
+            throw ex;
         }
         c2pa_reader = updated;
 

tests/builder.test.cpp

@@ -7278,3 +7278,27 @@ TEST_F(BuilderTest, ArchiveToFstreamBackedCppOStream) {
 
     EXPECT_GT(std::filesystem::file_size(archive_path), 0u);
 }
+
+// A malformed archive makes c2pa_builder_with_archive fail.
+// The wrapper must throw and free the retained builder.
+TEST(BuilderErrorHandling, FromArchiveMalformedThrows)
+{
+    std::stringstream bad_archive(std::ios::in | std::ios::out | std::ios::binary);
+    bad_archive << "not a valid c2pa archive";
+    bad_archive.seekg(0);
+
+    EXPECT_THROW(c2pa::Builder::from_archive(bad_archive), c2pa::C2paException);
+}
+
+TEST(BuilderErrorHandling, WithArchiveMalformedThrows)
+{
+    auto manifest = c2pa_test::read_text_file(c2pa_test::get_fixture_path("training.json"));
+    auto context = c2pa::Context();
+    auto builder = c2pa::Builder(context, manifest);
+
+    std::stringstream bad_archive(std::ios::in | std::ios::out | std::ios::binary);
+    bad_archive << "not a valid c2pa archive";
+    bad_archive.seekg(0);
+
+    EXPECT_THROW(builder.with_archive(bad_archive), c2pa::C2paException);
+}