diff --git a/include/c2pa.hpp b/include/c2pa.hpp index f890a4c..31a7bad 100644 --- a/include/c2pa.hpp +++ b/include/c2pa.hpp @@ -112,6 +112,56 @@ namespace c2pa std::string message_; }; + /// @brief Phase values reported to the ProgressCallbackFunc. + /// + /// @details A scoped C++ mirror of `C2paProgressPhase` from c2pa.h. + /// Values are verified at compile time to match the C enum, so any + /// future divergence in c2pa-rs will be caught as a build error. + /// + /// Phases emitted during a typical sign cycle (in order): + /// AddingIngredient → Thumbnail → Hashing → Signing → Embedding → + /// (if verify_after_sign) VerifyingManifest → VerifyingSignature → + /// VerifyingAssetHash → VerifyingIngredient + /// + /// Phases emitted during reading: + /// Reading → VerifyingManifest → VerifyingSignature → + /// VerifyingAssetHash → VerifyingIngredient + enum class ProgressPhase : uint8_t { + Reading = 0, + VerifyingManifest = 1, + VerifyingSignature = 2, + VerifyingIngredient = 3, + VerifyingAssetHash = 4, + AddingIngredient = 5, + Thumbnail = 6, + Hashing = 7, + Signing = 8, + Embedding = 9, + FetchingRemoteManifest = 10, + Writing = 11, + FetchingOCSP = 12, + FetchingTimestamp = 13, + }; + + /// @brief Type alias for the progress callback passed to ContextBuilder::with_progress_callback(). + /// + /// @details The callback is invoked at each major phase of signing and reading operations. + /// Returning false from the callback aborts the operation with an + /// OperationCancelled error (equivalent to calling Context::cancel()). + /// + /// @param phase Current operation phase. + /// @param step 1-based step index within the phase. + /// 0 = indeterminate (use as liveness signal); resets to 1 at each new phase. + /// @param total 0 = indeterminate; 1 = single-shot; >1 = determinate (step/total = fraction). + /// @return true to continue the operation, false to request cancellation. + /// + /// @note The callback must not throw. If it throws, the implementation catches the + /// exception and reports cancellation to the underlying library (same as returning + /// false); the original exception is not propagated. Prefer returning false or + /// using Context::cancel() instead of throwing. + /// + using ProgressCallbackFunc = std::function; + /// @brief Interface for types that can provide C2PA context functionality. /// @details This interface can be implemented by external libraries to provide /// custom context implementations (e.g. AdobeContext wrappers). @@ -165,11 +215,23 @@ namespace c2pa /// @warning Implementations must ensure is_valid() == true implies c_context() != nullptr. [[nodiscard]] virtual bool is_valid() const noexcept = 0; + /// @brief Get shared ownership of the progress callback, if any. + /// @return shared_ptr to the callback, or empty if none was set. + [[nodiscard]] std::shared_ptr callback_guard() const noexcept { + return callback_owner_; + } + protected: IContextProvider() = default; IContextProvider(const IContextProvider&) = delete; IContextProvider& operator=(const IContextProvider&) = delete; + + /// Shared-owned progress callback. Non-empty only when a progress callback + /// has been configured (e.g. via ContextBuilder::with_progress_callback()). + /// Builder and Reader copy this at construction to extend the callback's + /// lifetime beyond the originating context. + std::shared_ptr callback_owner_; }; /// @brief (C2PA SDK) Settings configuration object for creating contexts. @@ -238,56 +300,6 @@ namespace c2pa C2paSettings* settings_ptr; }; - /// @brief Phase values reported to the ProgressCallbackFunc. - /// - /// @details A scoped C++ mirror of `C2paProgressPhase` from c2pa.h. - /// Values are verified at compile time to match the C enum, so any - /// future divergence in c2pa-rs will be caught as a build error. - /// - /// Phases emitted during a typical sign cycle (in order): - /// AddingIngredient → Thumbnail → Hashing → Signing → Embedding → - /// (if verify_after_sign) VerifyingManifest → VerifyingSignature → - /// VerifyingAssetHash → VerifyingIngredient - /// - /// Phases emitted during reading: - /// Reading → VerifyingManifest → VerifyingSignature → - /// VerifyingAssetHash → VerifyingIngredient - enum class ProgressPhase : uint8_t { - Reading = 0, - VerifyingManifest = 1, - VerifyingSignature = 2, - VerifyingIngredient = 3, - VerifyingAssetHash = 4, - AddingIngredient = 5, - Thumbnail = 6, - Hashing = 7, - Signing = 8, - Embedding = 9, - FetchingRemoteManifest = 10, - Writing = 11, - FetchingOCSP = 12, - FetchingTimestamp = 13, - }; - - /// @brief Type alias for the progress callback passed to ContextBuilder::with_progress_callback(). - /// - /// @details The callback is invoked at each major phase of signing and reading operations. - /// Returning false from the callback aborts the operation with an - /// OperationCancelled error (equivalent to calling Context::cancel()). - /// - /// @param phase Current operation phase. - /// @param step 1-based step index within the phase. - /// 0 = indeterminate (use as liveness signal); resets to 1 at each new phase. - /// @param total 0 = indeterminate; 1 = single-shot; >1 = determinate (step/total = fraction). - /// @return true to continue the operation, false to request cancellation. - /// - /// @note The callback must not throw. If it throws, the implementation catches the - /// exception and reports cancellation to the underlying library (same as returning - /// false); the original exception is not propagated. Prefer returning false or - /// using Context::cancel() instead of throwing. - /// - using ProgressCallbackFunc = std::function; - /// @brief C2PA context implementing IContextProvider. /// @details Context objects manage C2PA SDK configuration and state. /// Contexts can be created via direct construction or the ContextBuilder: @@ -477,10 +489,6 @@ namespace c2pa private: C2paContext* context; - - /// Heap-owned ProgressCallbackFunc; non-null only when set via - /// ContextBuilder::with_progress_callback(). Deleted in the destructor. - void* callback_owner_ = nullptr; }; /// @brief Get the version of the C2PA library. @@ -728,6 +736,7 @@ namespace c2pa C2paReader *c2pa_reader; std::unique_ptr owned_stream; // Owns file stream when created from path std::unique_ptr cpp_stream; // Wraps stream for C API; destroyed before owned_stream + std::shared_ptr callback_guard_; public: /// @brief Create a Reader from a context and stream. @@ -782,7 +791,8 @@ namespace c2pa Reader(Reader&& other) noexcept : c2pa_reader(std::exchange(other.c2pa_reader, nullptr)), owned_stream(std::move(other.owned_stream)), - cpp_stream(std::move(other.cpp_stream)) { + cpp_stream(std::move(other.cpp_stream)), + callback_guard_(std::move(other.callback_guard_)) { } Reader& operator=(Reader&& other) noexcept { @@ -791,6 +801,7 @@ namespace c2pa c2pa_reader = std::exchange(other.c2pa_reader, nullptr); owned_stream = std::move(other.owned_stream); cpp_stream = std::move(other.cpp_stream); + callback_guard_ = std::move(other.callback_guard_); } return *this; } @@ -939,6 +950,7 @@ namespace c2pa { private: C2paBuilder *builder; + std::shared_ptr callback_guard_; public: /// @brief Create a Builder from a context with an empty manifest. @@ -970,13 +982,16 @@ namespace c2pa Builder& operator=(const Builder&) = delete; - Builder(Builder&& other) noexcept : builder(std::exchange(other.builder, nullptr)) { + Builder(Builder&& other) noexcept + : builder(std::exchange(other.builder, nullptr)), + callback_guard_(std::move(other.callback_guard_)) { } Builder& operator=(Builder&& other) noexcept { if (this != &other) { c2pa_free(builder); builder = std::exchange(other.builder, nullptr); + callback_guard_ = std::move(other.callback_guard_); } return *this; } diff --git a/src/c2pa_builder.cpp b/src/c2pa_builder.cpp index 16bd63d..561bb12 100644 --- a/src/c2pa_builder.cpp +++ b/src/c2pa_builder.cpp @@ -41,6 +41,7 @@ namespace c2pa if (builder == nullptr) { throw C2paException("Failed to create builder from context"); } + callback_guard_ = context.callback_guard(); } Builder::Builder(IContextProvider& context, const std::string &manifest_json) @@ -54,6 +55,7 @@ namespace c2pa if (builder == nullptr) { throw C2paException("Failed to create builder from context"); } + callback_guard_ = context.callback_guard(); // Apply the manifest definition to the Builder. // Note: c2pa_builder_with_definition always consumes the builder pointer, diff --git a/src/c2pa_context.cpp b/src/c2pa_context.cpp index 616bae9..d3909a5 100644 --- a/src/c2pa_context.cpp +++ b/src/c2pa_context.cpp @@ -39,23 +39,21 @@ namespace c2pa } Context::Context(Context&& other) noexcept - : context(std::exchange(other.context, nullptr)), - callback_owner_(std::exchange(other.callback_owner_, nullptr)) { + : context(std::exchange(other.context, nullptr)) { + callback_owner_ = std::move(other.callback_owner_); } Context& Context::operator=(Context&& other) noexcept { if (this != &other) { c2pa_free(context); - delete static_cast(callback_owner_); context = std::exchange(other.context, nullptr); - callback_owner_ = std::exchange(other.callback_owner_, nullptr); + callback_owner_ = std::move(other.callback_owner_); } return *this; } Context::~Context() noexcept { c2pa_free(context); - delete static_cast(callback_owner_); } void Context::cancel() noexcept { @@ -257,9 +255,9 @@ namespace c2pa context_builder = nullptr; Context result(ctx); - // Transfer progress callback heap ownership to the Context so it is freed - // when the Context is destroyed (the C side holds a raw pointer to it). - result.callback_owner_ = pending_callback_.release(); + // Transfer progress callback heap ownership to the Context. + // Using shared_ptr so Builder/Reader can extend the callback's lifetime. + result.callback_owner_ = std::move(pending_callback_); return result; } } // namespace c2pa diff --git a/src/c2pa_reader.cpp b/src/c2pa_reader.cpp index 42730e9..b49b3fc 100644 --- a/src/c2pa_reader.cpp +++ b/src/c2pa_reader.cpp @@ -49,6 +49,7 @@ namespace c2pa if (c2pa_reader == nullptr) { throw C2paException("Failed to create reader from context"); } + callback_guard_ = context.callback_guard(); cpp_stream = std::make_unique(stream); // Update reader with stream. @@ -73,6 +74,7 @@ namespace c2pa if (c2pa_reader == nullptr) { throw C2paException("Failed to create reader from context"); } + callback_guard_ = context.callback_guard(); // Create owned stream that will live as long as the Reader owned_stream = std::make_unique(source_path, std::ios::binary); diff --git a/tests/context.test.cpp b/tests/context.test.cpp index 5ba0762..c36074c 100644 --- a/tests/context.test.cpp +++ b/tests/context.test.cpp @@ -684,3 +684,60 @@ TEST_F(ContextTest, ProgressCallback_SurvivesBuilderMove) { EXPECT_NO_THROW(sign_with_progress_context(context, get_temp_path("progress_builder_move.jpg"))); EXPECT_GT(call_count.load(), 0); } + +// Context destroyed before Builder is used; callback must still work. +TEST_F(ContextTest, ProgressCallback_SurvivesContextDestruction_Builder) { + std::atomic call_count{0}; + + // Build a Builder inside a nested scope so the Context is destroyed first. + c2pa::Builder builder = [&]() { + auto context = c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++call_count; + return true; + }) + .create_context(); + + auto manifest = c2pa_test::read_text_file(c2pa_test::get_fixture_path("training.json")); + return c2pa::Builder(context, manifest); + // context is destroyed here + }(); + + auto certs = c2pa_test::read_text_file(c2pa_test::get_fixture_path("es256_certs.pem")); + auto private_key = c2pa_test::read_text_file(c2pa_test::get_fixture_path("es256_private.key")); + auto asset_path = c2pa_test::get_fixture_path("A.jpg"); + c2pa::Signer signer("es256", certs, private_key); + + // Context is already destroyed; signing must not crash. + EXPECT_NO_THROW(builder.sign(asset_path, get_temp_path("callback_survives_builder.jpg"), signer)); + EXPECT_GT(call_count.load(), 0); +} + +// Context destroyed before Reader is used; callback must still work. +TEST_F(ContextTest, ProgressCallback_SurvivesContextDestruction_Reader) { + // First sign a file so we have something to read. + { + c2pa::Context sign_ctx; + sign_with_progress_context(sign_ctx, get_temp_path("progress_read_survive_src.jpg")); + } + + std::atomic call_count{0}; + auto signed_path = get_temp_path("progress_read_survive_src.jpg"); + + // Build a Reader inside a nested scope so the Context is destroyed first. + c2pa::Reader reader = [&]() { + auto context = c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++call_count; + return true; + }) + .create_context(); + + return c2pa::Reader(context, signed_path); + // context is destroyed here + }(); + + // Context is already destroyed; accessing the reader must not crash. + EXPECT_NO_THROW((void)reader.json()); + EXPECT_GT(call_count.load(), 0); +}