From a30775a19e98267ca503663b139958d291e3b358 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Tue, 6 Jul 2021 15:45:10 +0200 Subject: [PATCH] QmlDesigner: Prevent prototype chains cycles A cycle would lead to an endless loop. So we throw an exception for synchronization. Maybe we can add later more information to user so he can easily resolve the error. Task-number: QDS-4457 Change-Id: I83092ccdff030a610942c155571a0bfa899e808c Reviewed-by: Thomas Hartmann Reviewed-by: Qt CI Bot --- .../projectstorage/projectstorage.h | 33 +++++++++++--- .../projectstorage/projectstorageexceptions.h | 6 +++ tests/unit/unittest/projectstorage-test.cpp | 45 ++++++++++++++++++- 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h index bf2f80790f1..540f10105f6 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h @@ -555,7 +555,7 @@ private: return Sqlite::CallbackControl::Continue; }; - updatePrototypeToNullStatement.readCallback(callback, &prototypeId); + updatePrototypeIdToNullStatement.readCallback(callback, &prototypeId); } void deleteType(TypeId typeId, @@ -640,6 +640,7 @@ private: } updateTypePrototypeStatement.write(&prototype.typeId, &prototypeId, &prototypeNameId); + checkForPrototypeChainCycle(prototype.typeId); } } @@ -1093,6 +1094,18 @@ private: synchronizeEnumerationDeclarations(typeId, type.enumerationDeclarations); } + void checkForPrototypeChainCycle(TypeId typeId) + { + auto callback = [=](long long currentTypeId) { + if (typeId == TypeId{currentTypeId}) + throw PrototypeChainCycle{}; + + return Sqlite::CallbackControl::Continue; + }; + + selectTypeIdsForPrototypeChainIdStatement.readCallback(callback, &typeId); + } + void syncPrototypes(Storage::Type &type) { if (Utils::visit([](auto &&typeName) -> bool { return typeName.name.isEmpty(); }, @@ -1106,6 +1119,7 @@ private: throw TypeNameDoesNotExists{}; updatePrototypeStatement.write(&type.typeId, &prototypeId, &prototypeTypeNameId); + checkForPrototypeChainCycle(type.typeId); } } @@ -1517,11 +1531,8 @@ public: Initializer initializer; ReadWriteStatement<1> upsertTypeStatement{ "INSERT INTO types(importId, name, accessSemantics, sourceId) VALUES(?1, ?2, " - "?3, nullif(?4, -1)) ON " - "CONFLICT DO UPDATE SET prototypeId=excluded.prototypeId, " - "accessSemantics=excluded.accessSemantics, sourceId=excluded.sourceId WHERE " - "prototypeId iS NOT excluded.prototypeId OR accessSemantics IS NOT " - "excluded.accessSemantics OR " + "?3, nullif(?4, -1)) ON CONFLICT DO UPDATE SET accessSemantics=excluded.accessSemantics, " + "sourceId=excluded.sourceId WHERE accessSemantics IS NOT excluded.accessSemantics OR " "sourceId IS NOT excluded.sourceId RETURNING typeId", database}; WriteStatement updatePrototypeStatement{ @@ -1819,13 +1830,21 @@ public: ReadStatement<1> selectPropertyDeclarationIdStatement{ "SELECT propertyDeclarationId FROM propertyDeclarations WHERE propertyDeclarationId=?", database}; - ReadWriteStatement<3> updatePrototypeToNullStatement{ + ReadWriteStatement<3> updatePrototypeIdToNullStatement{ "UPDATE types SET prototypeId=NULL WHERE prototypeId=?1 RETURNING " "typeId, prototypeNameId, sourceId", database}; ReadStatement<1> selectTypeIdStatement{"SELECT typeId FROM types WHERE typeId=?", database}; WriteStatement updateTypePrototypeStatement{ "UPDATE types SET prototypeId=?2, prototypeNameId=?3 WHERE typeId=?1", database}; + mutable ReadStatement<1> selectTypeIdsForPrototypeChainIdStatement{ + "WITH RECURSIVE " + " prototypes(typeId) AS (" + " SELECT prototypeId FROM types WHERE typeId=? " + " UNION ALL " + " SELECT prototypeId FROM types JOIN prototypes USING(typeId)) " + "SELECT typeId FROM prototypes", + database}; }; } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageexceptions.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageexceptions.h index aedfcca712f..567d1989976 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageexceptions.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageexceptions.h @@ -89,4 +89,10 @@ public: const char *what() const noexcept override { return "The property name does not exist!"; } }; +class PrototypeChainCycle : std::exception +{ +public: + const char *what() const noexcept override { return "There is a prototype chain cycle!"; } +}; + } // namespace QmlDesigner diff --git a/tests/unit/unittest/projectstorage-test.cpp b/tests/unit/unittest/projectstorage-test.cpp index ddb3e2e2532..0caf8bc5433 100644 --- a/tests/unit/unittest/projectstorage-test.cpp +++ b/tests/unit/unittest/projectstorage-test.cpp @@ -912,8 +912,15 @@ TEST_F(ProjectStorageSlowTest, SynchronizeTypesAddsNewTypesThrowsWithWrongExport TEST_F(ProjectStorageSlowTest, SynchronizeTypesAddsNewTypesWithMissingImportAndExportedPrototypeName) { Storage::Types types{createTypes()}; + types.push_back(Storage::Type{importId3, + "QObject2", + Storage::NativeType{}, + TypeAccessSemantics::Reference, + sourceId4, + importIds, + {Storage::ExportedType{"Object2"}, Storage::ExportedType{"Obj2"}}}); storage.synchronizeDocuments({Storage::Document{sourceId1, {importId1}}}); - types[1].prototype = Storage::ExportedType{"Object"}; + types[1].prototype = Storage::ExportedType{"Object2"}; ASSERT_THROW(storage.synchronizeTypes(types, {sourceId1, sourceId2}), QmlDesigner::TypeNameDoesNotExists); @@ -3070,4 +3077,40 @@ TEST_F(ProjectStorageSlowTest, ChangePrototypeTypeNameThrowsForWrongNativeProtot QmlDesigner::TypeNameDoesNotExists); } +TEST_F(ProjectStorageSlowTest, ThrowForPrototypeChainCycles) +{ + Storage::Types types{createTypes()}; + types[1].prototype = Storage::ExportedType{"Object2"}; + types.push_back(Storage::Type{importId3, + "QObject2", + Storage::ExportedType{"Item"}, + TypeAccessSemantics::Reference, + sourceId3, + importIds, + {Storage::ExportedType{"Object2"}, Storage::ExportedType{"Obj2"}}}); + + ASSERT_THROW(storage.synchronizeTypes(types, {sourceId1, sourceId2, sourceId3}), + QmlDesigner::PrototypeChainCycle); +} + +TEST_F(ProjectStorageSlowTest, ThrowForTypeIdAndPrototypeIdAreTheSame) +{ + Storage::Types types{createTypes()}; + types[1].prototype = Storage::ExportedType{"Object"}; + + ASSERT_THROW(storage.synchronizeTypes(types, {sourceId1, sourceId2}), + QmlDesigner::PrototypeChainCycle); +} + +TEST_F(ProjectStorageSlowTest, ThrowForTypeIdAndPrototypeIdAreTheSameForRelinking) +{ + Storage::Types types{createTypes()}; + types[0].propertyDeclarations[0].typeName = Storage::ExportedType{"Object"}; + types[0].prototype = Storage::ExportedType{"Object"}; + storage.synchronizeTypes(types, {sourceId1, sourceId2}); + types[1].prototype = Storage::ExportedType{"Item"}; + types[1].typeName = "QObject2"; + + ASSERT_THROW(storage.synchronizeTypes({types[1]}, {sourceId2}), QmlDesigner::PrototypeChainCycle); +} } // namespace