mooltipass / moolticute

Mooltipass crossplatform daemon/tools
GNU General Public License v3.0
143 stars 67 forks source link

DB integrity check does not check for and repair empty services #1127

Closed barathrm closed 1 year ago

barathrm commented 1 year ago

Expected behavior

DB integrity check should remove empty services.

Actual behavior

It doees not.

Step by step guide to reproduce the problem

Of course db import could find and re-use this empty service (it doesn't evaluate as "equal" enough), but, integrity check should fix this problem imo.

Moolticute Version

1.00.1

Operating System

Linux

Mooltipass Device

FWIW, I was able to make the integrity check remove the empty parent node with this diff. It feels kind of hacky though.

diff --git a/src/MPDevice.cpp b/src/MPDevice.cpp
index 0e98098..1cdf16f 100755
--- a/src/MPDevice.cpp
+++ b/src/MPDevice.cpp
@@ -3162,6 +3162,36 @@ bool MPDevice::checkLoadedNodes(bool checkCredentials, bool checkData, bool repa
     /* Tag pointed nodes, also detects DB errors */
     if (checkCredentials || checkData)
     {
+
+        MPNode* tempNextParentNodePt = nullptr;
+        QByteArray tempParentAddress = startNode[Common::CRED_ADDR_IDX];
+        quint32 tempVirtualParentAddress = virtualStartNode[Common::CRED_ADDR_IDX];
+
+        /* Loop through the parent nodes */
+        while ((tempParentAddress != MPNode::EmptyAddress) || (tempParentAddress.isNull() && tempVirtualParentAddress != 0))
+        {
+            tempNextParentNodePt = findNodeWithAddressInList(loginNodes, tempParentAddress, tempVirtualParentAddress);
+            if (!tempNextParentNodePt)
+                break;
+
+            if (tempNextParentNodePt->getStartChildAddress() ==
+                MPNode::EmptyAddress) {
+                qWarning() << "Parent " << tempNextParentNodePt->getService()
+                           << " has no children. erasing.";
+                MPNode *toDelete = tempNextParentNodePt;
+
+                tempParentAddress = tempNextParentNodePt->getNextParentAddress();
+                tempVirtualParentAddress = tempNextParentNodePt->getNextParentVirtualAddress();
+                bool ret = removeEmptyParentFromDB(toDelete, false);
+                qWarning() << "ret " << ret;
+                return_bool = !ret;
+                continue;
+            }
+            tempParentAddress = tempNextParentNodePt->getNextParentAddress();
+            tempVirtualParentAddress = tempNextParentNodePt->getNextParentVirtualAddress();
+
+        }
+
         return_bool &= tagPointedNodes(checkCredentials, checkData, repairAllowed);
     }
limpkin commented 1 year ago

very nice find @barathrm !!! I don't think an empty service is an issue to be honest... but let's remove it :) assigning to @deXol