diff --git a/src/opentimelineio/algo/editAlgorithm.cpp b/src/opentimelineio/algo/editAlgorithm.cpp index 55e41f226..f3e69e105 100644 --- a/src/opentimelineio/algo/editAlgorithm.cpp +++ b/src/opentimelineio/algo/editAlgorithm.cpp @@ -181,9 +181,22 @@ 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::CANNOT_CLONE_ITEM; + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus::TYPE_MISMATCH; + return; + } + trimmed_range = second_item->trimmed_range(); + source_range = TimeRange( trimmed_range.start_time() + first_duration + range.duration(), second_duration); @@ -362,7 +375,20 @@ 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::CANNOT_CLONE_ITEM; + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus::TYPE_MISMATCH; + return; + } second_item->set_source_range(second_source_range); composition->insert_child(insert_index + 1, second_item); } @@ -530,7 +556,20 @@ 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::CANNOT_CLONE_ITEM; + return; + } + auto second_item = dynamic_cast(cloned_item); + if (!second_item) + { + if (error_status) + *error_status = ErrorStatus::TYPE_MISMATCH; + return; + } const TimeRange second_source_range( first_source_range.start_time() + first_source_range.duration(), range.duration() - first_source_range.duration()); @@ -793,7 +832,20 @@ 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::CANNOT_CLONE_ITEM; + return; + } + auto track_item = dynamic_cast(cloned_item); + if (!track_item) + { + if (error_status) + *error_status = ErrorStatus::TYPE_MISMATCH; + 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/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 30b82858d..edb9e892a 100644 --- a/tests/test_editAlgorithm.cpp +++ b/tests/test_editAlgorithm.cpp @@ -2131,6 +2131,125 @@ main(int argc, char** argv) TimeRange(RationalTime(0.0, 24.0), RationalTime(10.0, 24.0)) }); }); + 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::CANNOT_CLONE_ITEM); + }); + + tests.add_test("regression: crash in overwrite", [] { + 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::overwrite( + small_clip, + track, + TimeRange(RationalTime(0.0, 24.0), RationalTime(12.0, 24.0)), + true, + nullptr, + &error_status); + + assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); + }); + + 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, + track, + RationalTime(12.0, 24.0), + true, + nullptr, + &error_status); + + assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); + }); + + 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( + big_clip, + track, + RationalTime(12.0, 24.0), + ReferencePoint::Sequence, + &error_status); + + assert(is_error(error_status)); + assert(error_status.outcome == OTIO_NS::ErrorStatus::CANNOT_CLONE_ITEM); + }); + tests.run(argc, argv); return 0; } 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; }