fix: Improve memory handling on error/failure

src/c2pa_builder.cpp

@@ -51,12 +51,12 @@ namespace c2pa
         init_from_context(context);
 
         // Apply the manifest definition to the Builder.
-        // Note: c2pa_builder_with_definition consumes the builder pointer on success
-        // and on operation failure.
+        // Note: c2pa_builder_with_definition consumes the builder pointer on success.
+        // On failure the handle is only ours again when the library rejected it
+        // before taking ownership, which error_from_failed_call works out.
         C2paBuilder* updated = c2pa_builder_with_definition(builder, manifest_json.c_str());
-        builder = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(builder);
         }
         builder = updated;
     }
@@ -116,10 +116,11 @@ namespace c2pa
             throw C2paException();
         }
 
+        // base is a local, so no destructor reclaims it if this throws: a
+        // constructor that throws never gets ~Builder(). Free it here on failure.
         builder = c2pa_builder_with_archive(base, c_archive.c_stream);
-        base = nullptr;
         if (builder == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(base);
         }
     }
 
@@ -152,8 +153,8 @@ namespace c2pa
         // c2pa_builder_with_definition consumes the builder pointer,
         // so the original pointer is invalid after the call.
         C2paBuilder* updated = c2pa_builder_with_definition(builder, manifest_json.c_str());
-        builder = nullptr;
         if (updated == nullptr) {
+            (void)detail::error_from_failed_call(builder);
             throw C2paException("Failed to set builder definition");
         }
         builder = updated;
@@ -344,11 +345,10 @@ namespace c2pa
     {
         CppIStream c_archive(archive);
 
-        // c2pa_builder_with_archive consumes the builder pointer and returns a new one
+        // c2pa_builder_with_archive consumes the builder pointer, and returns a new one.
         C2paBuilder* updated = c2pa_builder_with_archive(builder, c_archive.c_stream);
-        builder = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(builder);
         }
         builder = updated;
         return *this;

src/c2pa_context.cpp

@@ -183,9 +183,8 @@ namespace c2pa
         if (!raw) {
             throw C2paException("Signer is not valid");
         }
-
         if (c2pa_context_builder_set_signer(context_builder, raw) != 0) {
-            throw C2paException();
+            throw detail::error_from_failed_call(raw);
         }
         return *this;
     }
@@ -203,7 +202,7 @@ namespace c2pa
             throw C2paException("Failed to create HTTP resolver");
         }
         if (c2pa_context_builder_set_http_resolver(context_builder, resolver) != 0) {
-            throw C2paException();
+            throw detail::error_from_failed_call(resolver);
         }
         return *this;
     }
@@ -275,10 +274,12 @@ namespace c2pa
             throw C2paException("ContextBuilder is invalid (moved from)");
         }
 
-        // The C API consumes the context builder on build (success or failure).
+        // The C API consumes the context builder on build.
         C2paContext* ctx = c2pa_context_builder_build(context_builder);
-        context_builder = nullptr;
         if (!ctx) {
+            (void)detail::error_from_failed_call(context_builder);
+            // The native builder is gone, so nothing can invoke the callback now.
+            pending_callback_.reset();
             throw C2paException("Failed to build context");
         }
 

src/c2pa_internal.hpp

@@ -39,6 +39,51 @@ inline bool error_indicates_manifest_not_found(const char* message) noexcept {
     return message != nullptr && std::strstr(message, "ManifestNotFound") != nullptr;
 }
 
+/// @brief Error prefixes the library reports when it rejects a handle
+///        before taking ownership, leaving the handle ours to free.
+/// @details Any other error means the library took the handle and dropped it itself.
+///          Both entries say the library never took the handle, so it is still ours
+///          and we remain responsible for releasing it.
+inline constexpr const char *kPreConsumeErrorTags[] = {
+    "UntrackedPointer:",  // handle not in the registry
+    "WrongPointerType:",  // handle tracked under a different type
+};
+
+/// @brief True when @p message says the library rejected the handle before
+///        taking ownership.
+/// @param message The library's error text, or nullptr when it reported none.
+/// @return False for nullptr: with no error, assume the handle was consumed.
+inline bool is_pre_consume_error(const char *message) noexcept {
+    if (message == nullptr) {
+        return false;
+    }
+    for (const char *tag : kPreConsumeErrorTags) {
+        if (std::strstr(message, tag) != nullptr) {
+            return true;
+        }
+    }
+    return false;
+}
+
+/// @brief Finish with a handle whose consuming C API call failed: release it if
+///        the library left it to us, and return the error to throw.
+/// @param handle The handle passed to the failed call, set to null before this
+///        returns. Invalid either way afterwards, whichever side released it.
+/// @return The exception to throw, carrying the library's error message. Meant to be
+///         used directly: `throw detail::error_from_failed_call(handle);`.
+/// @details The exception is constructed before the free because a failing
+///          c2pa_free() overwrites the thread-local last error,
+///          which would replace the real failure message.
+template<typename Handle>
+[[nodiscard]] inline C2paException error_from_failed_call(Handle*& handle) {
+    C2paException error;
+    if (is_pre_consume_error(error.what())) {
+        c2pa_free(handle);
+    }
+    handle = nullptr;
+    return error;
+}
+
 /// @brief Converts a C array of C strings to a std::vector of std::string.
 /// @param mime_types Pointer to an array of C strings (const char*).
 /// @param count Number of elements in the array.

src/c2pa_reader.cpp

@@ -58,11 +58,10 @@ namespace c2pa
         }
 
         // Update reader with stream.
-        // Note: c2pa_reader_with_stream consumes the reader pointer.
+        // Note: c2pa_reader_with_stream consumes the reader pointer on success.
         C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, resolved.c_str(), cpp_stream->c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(c2pa_reader);
         }
         c2pa_reader = updated;
     }
@@ -94,11 +93,10 @@ namespace c2pa
             throw C2paException("Failed to create reader from context");
         }
 
-        // Note: c2pa_reader_with_stream consumes the reader pointer.
+        // Note: c2pa_reader_with_stream consumes the reader pointer on success.
         C2paReader* updated = c2pa_reader_with_stream(c2pa_reader, extension.c_str(), cpp_stream->c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(c2pa_reader);
         }
         c2pa_reader = updated;
     }
@@ -126,16 +124,15 @@ namespace c2pa
             throw C2paException("Failed to create reader from context");
         }
 
-        // c2pa_reader_with_manifest_data_and_stream always consumes c2pa_reader.
+        // c2pa_reader_with_manifest_data_and_stream consumes c2pa_reader on success.
         C2paReader* updated = c2pa_reader_with_manifest_data_and_stream(
             c2pa_reader,
             resolved.c_str(),
             cpp_stream->c_stream,
             manifest_jumbf.data(),
             manifest_jumbf.size());
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(c2pa_reader);
         }
         c2pa_reader = updated;
 
@@ -209,9 +206,8 @@ namespace c2pa
             resolved.c_str(),
             main_wrapper.c_stream,
             fragment_wrapper.c_stream);
-        c2pa_reader = nullptr;
         if (updated == nullptr) {
-            throw C2paException();
+            throw detail::error_from_failed_call(c2pa_reader);
         }
         c2pa_reader = updated;
 

tests/builder.test.cpp

@@ -7597,6 +7597,37 @@ TEST_F(BuilderTest, SignTrimsPaddedFormat) {
     EXPECT_NO_THROW({ builder.sign(" \t image/jpeg \n ", source, dest, signer); });
 }
 
+TEST_F(BuilderTest, FailedConsumingCallKeepsTheLibraryErrorMessage) {
+    // A failed consuming call reclaims the handle, and c2pa_free() on a handle the
+    // library already took overwrites the thread-local error with its own
+    // "Other: UntrackedPointer: 0x...".
+    auto context = std::make_shared<c2pa::Context>();
+
+    // Provoked failure: the JSON passes argument validation, then fails to parse.
+    try {
+        c2pa::Builder builder(context, "{ this is not valid json ");
+        FAIL() << "expected malformed JSON to throw";
+    } catch (const c2pa::C2paException& e) {
+        const std::string message = e.what();
+        EXPECT_NE(message.find("Json:"), std::string::npos)
+            << "lost the library's parse error, got: " << message;
+        EXPECT_EQ(message.find("UntrackedPointer"), std::string::npos)
+            << "error slot was dirtied by the reclaiming free, got: " << message;
+    }
+
+    // Validation failure: rejected before the library takes the handle,
+    // so the reclaiming free succeeds and sets no error of its own.
+    const std::string oversized = "{\"x\":\"" + std::string(1100000, 'a') + "\"}";
+    try {
+        c2pa::Builder builder(context, oversized);
+        FAIL() << "expected an over-long manifest to throw";
+    } catch (const c2pa::C2paException& e) {
+        const std::string message = e.what();
+        EXPECT_NE(message.find("StringTooLong"), std::string::npos)
+            << "lost the library's length error, got: " << message;
+    }
+}
+
 TEST_F(BuilderTest, SignWithUnsupportedFormatThrows) {
     auto signer = c2pa_test::create_test_signer();
     auto builder = make_builder();