From 3d51797aedbd5ef83be878146d5bc69d641cda41 Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Thu, 30 Apr 2026 23:11:19 -0700 Subject: [PATCH 1/6] Add tests for clone and clone-with-cycle Signed-off-by: Joshua Minor --- tests/test_serialization.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_serialization.cpp b/tests/test_serialization.cpp index 7ff13f76c..e5245459f 100644 --- a/tests/test_serialization.cpp +++ b/tests/test_serialization.cpp @@ -110,6 +110,39 @@ main(int argc, char** argv) })CONTENT"); }); + tests.add_test("clone", [] { + auto json = R"CONTENT({ + "OTIO_SCHEMA": "SerializableObjectWithMetadata.1", + "metadata": { + "a": 1, + "b": "two", + "c": [ + 3, + 4, + 5 + ], + "d": { + "hello": "nested" + } + }, + "name": "" +})CONTENT"; + auto original = SerializableObjectWithMetadata::from_json_string(json); + auto cloned = original->clone(); + assert(cloned != nullptr); + OTIO_NS::ErrorStatus err; + auto cloned_json = cloned->to_json_string(&err, {}, 2); + assertFalse(is_error(err)); + assertEqual(json, cloned_json.c_str()); + }); + + tests.add_test("clone with cycle returns nullptr", [] { + auto original = new SerializableObjectWithMetadata(); + original->metadata()["cycle"] = original; + auto cloned = original->clone(); + assert(cloned == nullptr); + }); + tests.run(argc, argv); return 0; } From 310b244aef68aca5bb5b41c2b8d5811201db50aa Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Thu, 30 Apr 2026 23:37:18 -0700 Subject: [PATCH 2/6] Prevent crash in algo::fill() Signed-off-by: Joshua Minor --- src/opentimelineio/algo/editAlgorithm.cpp | 18 +++++++++- tests/test_editAlgorithm.cpp | 41 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index 55e41f226..1f88b933c 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -530,7 +530,23 @@ slice( item->set_source_range(first_source_range); // Clone the item for the second slice. - auto second_item = dynamic_cast(item->clone()); + auto cloned_item = item->clone(); + if (!cloned_item) + { + if (error_status) + *error_status = + ErrorStatus(ErrorStatus::INTERNAL_ERROR, "clone failed"); + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone was unexpected type"); + return; + } const TimeRange second_source_range( first_source_range.start_time() + first_source_range.duration(), range.duration() - first_source_range.duration()); diff --git a/tests/test_editAlgorithm.cpp b/tests/test_editAlgorithm.cpp index 30b82858d..77be71145 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2131,6 +2131,47 @@ main(int argc, char** argv) TimeRange(RationalTime(0.0, 24.0), RationalTime(10.0, 24.0)) }); }); + tests.add_test("crash in overwrite, insert, slice, fill", [] { + SerializableObject::Retainer clip = new Clip( + "clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + clip->metadata()["cycle"] = clip; + SerializableObject::Retainer track = new Track(); + track->append_child(clip); + + OTIO_NS::ErrorStatus error_status; + + // algo::overwrite( + // clip, + // track, + // TimeRange(RationalTime(0.0, 24.0), RationalTime(12.0, 24.0)), + // true, + // nullptr, + // &error_status); + // assert(is_error(error_status)); + + // algo::insert( + // clip, + // track, + // RationalTime(12.0, 24.0), + // true, + // nullptr, + // &error_status); + // assert(is_error(error_status)); + + algo::slice(track, RationalTime(12.0, 24.0), false, &error_status); + assert(is_error(error_status)); + + // algo::fill( + // clip, + // track, + // RationalTime(12.0, 24.0), + // ReferencePoint::Fit, + // &error_status); + // assert(is_error(error_status)); + }); + tests.run(argc, argv); return 0; } From 264aa1c0db471b201081f09a055983e0691fe63b Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Thu, 30 Apr 2026 23:49:09 -0700 Subject: [PATCH 3/6] Fix crash in algo::insert Signed-off-by: Joshua Minor --- src/opentimelineio/algo/editAlgorithm.cpp | 19 +++++++++++++- tests/test_editAlgorithm.cpp | 31 ++++++++++++++--------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index 1f88b933c..62a00e26f 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -362,7 +362,24 @@ insert( // Clone the item for the second partially overwritten item. if (!isEqual(second_source_range.duration().value(), 0.0)) { - auto second_item = dynamic_cast(item->clone()); + auto cloned_item = item->clone(); + if (!cloned_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone failed"); + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone was unexpected type"); + return; + } second_item->set_source_range(second_source_range); composition->insert_child(insert_index + 1, second_item); } diff --git a/tests/test_editAlgorithm.cpp b/tests/test_editAlgorithm.cpp index 77be71145..8eafd8ace 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2132,13 +2132,20 @@ main(int argc, char** argv) }); tests.add_test("crash in overwrite, insert, slice, fill", [] { - SerializableObject::Retainer clip = new Clip( - "clip", + SerializableObject::Retainer big_clip = new Clip( + "big clip", nullptr, TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); - clip->metadata()["cycle"] = clip; + big_clip->metadata()["cycle"] = big_clip; + + SerializableObject::Retainer small_clip = new Clip( + "small clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(5.0, 24.0))); + small_clip->metadata()["cycle"] = small_clip; + SerializableObject::Retainer track = new Track(); - track->append_child(clip); + track->append_child(big_clip); OTIO_NS::ErrorStatus error_status; @@ -2151,14 +2158,14 @@ main(int argc, char** argv) // &error_status); // assert(is_error(error_status)); - // algo::insert( - // clip, - // track, - // RationalTime(12.0, 24.0), - // true, - // nullptr, - // &error_status); - // assert(is_error(error_status)); + algo::insert( + small_clip, + track, + RationalTime(12.0, 24.0), + true, + nullptr, + &error_status); + assert(is_error(error_status)); algo::slice(track, RationalTime(12.0, 24.0), false, &error_status); assert(is_error(error_status)); From c5c4741a419dccb5640b602e87fdddcf81787f60 Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Thu, 30 Apr 2026 23:51:51 -0700 Subject: [PATCH 4/6] Fix crash in algo::overwrite Signed-off-by: Joshua Minor --- src/opentimelineio/algo/editAlgorithm.cpp | 23 ++++++++++++++++++++--- tests/test_editAlgorithm.cpp | 16 ++++++++-------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index 62a00e26f..d73fa8a8b 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -181,9 +181,26 @@ overwrite( composition->insert_child(insert_index, item); if (!isEqual(second_duration.value(), 0.0)) { - auto second_item = dynamic_cast(items.front()->clone()); - trimmed_range = second_item->trimmed_range(); - source_range = TimeRange( + auto cloned_item = items.front()->clone(); + if (!cloned_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone failed"); + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone was unexpected type"); + return; + } + trimmed_range = second_item->trimmed_range(); + source_range = TimeRange( trimmed_range.start_time() + first_duration + range.duration(), second_duration); diff --git a/tests/test_editAlgorithm.cpp b/tests/test_editAlgorithm.cpp index 8eafd8ace..bd8b649a5 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2149,14 +2149,14 @@ main(int argc, char** argv) OTIO_NS::ErrorStatus error_status; - // algo::overwrite( - // clip, - // track, - // TimeRange(RationalTime(0.0, 24.0), RationalTime(12.0, 24.0)), - // true, - // nullptr, - // &error_status); - // assert(is_error(error_status)); + algo::overwrite( + small_clip, + track, + TimeRange(RationalTime(0.0, 24.0), RationalTime(12.0, 24.0)), + true, + nullptr, + &error_status); + assert(is_error(error_status)); algo::insert( small_clip, From 11ac50e534e48c2f2356926d9c4b498556bbf571 Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Fri, 1 May 2026 00:12:25 -0700 Subject: [PATCH 5/6] Fixed crash in algo::fill() + some refactoring of tests Signed-off-by: Joshua Minor --- src/opentimelineio/algo/editAlgorithm.cpp | 25 +++++- tests/test_editAlgorithm.cpp | 94 ++++++++++++++++++++--- 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index d73fa8a8b..c9d834ffc 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -195,7 +195,7 @@ overwrite( { if (error_status) *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, + ErrorStatus::TYPE_MISMATCH, "clone was unexpected type"); return; } @@ -393,7 +393,7 @@ insert( { if (error_status) *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, + ErrorStatus::TYPE_MISMATCH, "clone was unexpected type"); return; } @@ -577,7 +577,7 @@ slice( { if (error_status) *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, + ErrorStatus::TYPE_MISMATCH, "clone was unexpected type"); return; } @@ -843,7 +843,24 @@ fill( case ReferencePoint::Sequence: { RationalTime start_time = clip_range.start_time(); const RationalTime gap_start_time = gap_range.start_time(); - auto track_item = dynamic_cast(item->clone()); + auto cloned_item = item->clone(); + if (!cloned_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::INTERNAL_ERROR, + "clone failed"); + return; + } + auto track_item = dynamic_cast(cloned_item); + if (!track_item) + { + if (error_status) + *error_status = ErrorStatus( + ErrorStatus::TYPE_MISMATCH, + "clone was unexpected type"); + return; + } // Check if start time is less than gap's start time (trim it if so) if (start_time < gap_start_time) diff --git a/tests/test_editAlgorithm.cpp b/tests/test_editAlgorithm.cpp index bd8b649a5..31caa7332 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2131,7 +2131,31 @@ main(int argc, char** argv) TimeRange(RationalTime(0.0, 24.0), RationalTime(10.0, 24.0)) }); }); - tests.add_test("crash in overwrite, insert, slice, fill", [] { + tests.add_test("regression: crash in slice", [] { + SerializableObject::Retainer big_clip = new Clip( + "big clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + big_clip->metadata()["cycle"] = big_clip; + + SerializableObject::Retainer small_clip = new Clip( + "small clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(5.0, 24.0))); + small_clip->metadata()["cycle"] = small_clip; + + SerializableObject::Retainer track = new Track(); + track->append_child(big_clip); + + OTIO_NS::ErrorStatus error_status; + + algo::slice(track, RationalTime(12.0, 24.0), false, &error_status); + + assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + }); + + tests.add_test("regression: crash in overwrite", [] { SerializableObject::Retainer big_clip = new Clip( "big clip", nullptr, @@ -2157,6 +2181,26 @@ main(int argc, char** argv) nullptr, &error_status); assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + }); + + tests.add_test("regression: crash in insert", [] { + SerializableObject::Retainer big_clip = new Clip( + "big clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + big_clip->metadata()["cycle"] = big_clip; + + SerializableObject::Retainer small_clip = new Clip( + "small clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(5.0, 24.0))); + small_clip->metadata()["cycle"] = small_clip; + + SerializableObject::Retainer track = new Track(); + track->append_child(big_clip); + + OTIO_NS::ErrorStatus error_status; algo::insert( small_clip, @@ -2166,17 +2210,47 @@ main(int argc, char** argv) nullptr, &error_status); assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + }); - algo::slice(track, RationalTime(12.0, 24.0), false, &error_status); - assert(is_error(error_status)); + tests.add_test("regression: crash in fill", [] { + SerializableObject::Retainer big_clip = new Clip( + "big clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + big_clip->metadata()["cycle"] = big_clip; + + SerializableObject::Retainer small_clip = new Clip( + "small clip", + nullptr, + TimeRange(RationalTime(0.0, 24.0), RationalTime(5.0, 24.0))); + small_clip->metadata()["cycle"] = small_clip; + + SerializableObject::Retainer gap = new Gap( + TimeRange(RationalTime(0.0, 24.0), RationalTime(20.0, 24.0)), + "gap"); + + SerializableObject::Retainer track = new Track(); + track->append_child(small_clip); + track->append_child(gap); + track->append_child(small_clip); + + OTIO_NS::ErrorStatus error_status; - // algo::fill( - // clip, - // track, - // RationalTime(12.0, 24.0), - // ReferencePoint::Fit, - // &error_status); - // assert(is_error(error_status)); + algo::fill( + big_clip, + track, + RationalTime(12.0, 24.0), + ReferencePoint::Sequence, + &error_status); + assert(is_error(error_status)); + fprintf( + stderr, + "error_status: %s %s\n", + OTIO_NS::ErrorStatus::outcome_to_string(error_status.outcome) + .c_str(), + error_status.details.c_str()); + assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); }); tests.run(argc, argv); From 40478d634821f18fdc795f58725c5d8d49b6f570 Mon Sep 17 00:00:00 2001 From: Joshua Minor Date: Fri, 1 May 2026 00:18:58 -0700 Subject: [PATCH 6/6] Introduced ErrorStatus::CANNOT_CLONE_ITEM Signed-off-by: Joshua Minor --- src/opentimelineio/algo/editAlgorithm.cpp | 31 ++++++----------------- src/opentimelineio/errorStatus.cpp | 2 ++ src/opentimelineio/errorStatus.h | 3 ++- tests/test_editAlgorithm.cpp | 17 +++++-------- 4 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index c9d834ffc..f3e69e105 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -185,18 +185,14 @@ overwrite( if (!cloned_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, - "clone failed"); + *error_status = ErrorStatus::CANNOT_CLONE_ITEM; return; } auto second_item = dynamic_cast(cloned_item); if (!second_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::TYPE_MISMATCH, - "clone was unexpected type"); + *error_status = ErrorStatus::TYPE_MISMATCH; return; } trimmed_range = second_item->trimmed_range(); @@ -383,18 +379,14 @@ insert( if (!cloned_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, - "clone failed"); + *error_status = ErrorStatus::CANNOT_CLONE_ITEM; return; } auto second_item = dynamic_cast(cloned_item); if (!second_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::TYPE_MISMATCH, - "clone was unexpected type"); + *error_status = ErrorStatus::TYPE_MISMATCH; return; } second_item->set_source_range(second_source_range); @@ -568,17 +560,14 @@ slice( if (!cloned_item) { if (error_status) - *error_status = - ErrorStatus(ErrorStatus::INTERNAL_ERROR, "clone failed"); + *error_status = ErrorStatus::CANNOT_CLONE_ITEM; return; } auto second_item = dynamic_cast(cloned_item); if (!second_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::TYPE_MISMATCH, - "clone was unexpected type"); + *error_status = ErrorStatus::TYPE_MISMATCH; return; } const TimeRange second_source_range( @@ -847,18 +836,14 @@ fill( if (!cloned_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::INTERNAL_ERROR, - "clone failed"); + *error_status = ErrorStatus::CANNOT_CLONE_ITEM; return; } auto track_item = dynamic_cast(cloned_item); if (!track_item) { if (error_status) - *error_status = ErrorStatus( - ErrorStatus::TYPE_MISMATCH, - "clone was unexpected type"); + *error_status = ErrorStatus::TYPE_MISMATCH; return; } diff --git a/src/opentimelineio/errorStatus.cpp b/src/opentimelineio/errorStatus.cpp index 8b33003bf..7419fb660 100644 --- a/src/opentimelineio/errorStatus.cpp +++ b/src/opentimelineio/errorStatus.cpp @@ -66,6 +66,8 @@ ErrorStatus::outcome_to_string(Outcome o) return "the media references cannot contain an empty key"; case NOT_A_GAP: return "object is not descendent of Gap type"; + case CANNOT_CLONE_ITEM: + return "cannot clone item"; default: return "unknown/illegal ErrorStatus::Outcome code"; }; diff --git a/src/opentimelineio/errorStatus.h b/src/opentimelineio/errorStatus.h index ca991a1a1..855b8d547 100644 --- a/src/opentimelineio/errorStatus.h +++ b/src/opentimelineio/errorStatus.h @@ -45,7 +45,8 @@ struct OTIO_API_TYPE ErrorStatus CANNOT_COMPUTE_BOUNDS, MEDIA_REFERENCES_DO_NOT_CONTAIN_ACTIVE_KEY, MEDIA_REFERENCES_CONTAIN_EMPTY_KEY, - NOT_A_GAP + NOT_A_GAP, + CANNOT_CLONE_ITEM }; /// @brief Construct a new status with no error. diff --git a/tests/test_editAlgorithm.cpp b/tests/test_editAlgorithm.cpp index 31caa7332..edb9e892a 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2152,7 +2152,7 @@ main(int argc, char** argv) algo::slice(track, RationalTime(12.0, 24.0), false, &error_status); assert(is_error(error_status)); - assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); }); tests.add_test("regression: crash in overwrite", [] { @@ -2180,8 +2180,9 @@ main(int argc, char** argv) true, nullptr, &error_status); + assert(is_error(error_status)); - assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); }); tests.add_test("regression: crash in insert", [] { @@ -2209,8 +2210,9 @@ main(int argc, char** argv) true, nullptr, &error_status); + assert(is_error(error_status)); - assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); }); tests.add_test("regression: crash in fill", [] { @@ -2243,14 +2245,9 @@ main(int argc, char** argv) RationalTime(12.0, 24.0), ReferencePoint::Sequence, &error_status); + assert(is_error(error_status)); - fprintf( - stderr, - "error_status: %s %s\n", - OTIO_NS::ErrorStatus::outcome_to_string(error_status.outcome) - .c_str(), - error_status.details.c_str()); - assert(error_status.outcome == OTIO_NS::ErrorStatus::INTERNAL_ERROR); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); }); tests.run(argc, argv);