Merge pull request #1619 from lutraconsulting/1618-infinite-loop-sele…

…ctive-sync

Fix inifnite loop when config is invalid

app/test/testmerginapi.cpp

@@ -112,6 +112,7 @@ void TestMerginApi::initTestCase()
   deleteRemoteProject( mApiExtra, mUsername, "testSelectiveSyncRemoveConfig" );
   deleteRemoteProject( mApiExtra, mUsername, "testSelectiveSyncChangeSyncFolder" );
   deleteRemoteProject( mApiExtra, mUsername, "testSelectiveSyncDisabledInConfig" );
+  deleteRemoteProject( mApiExtra, mUsername, "testSelectiveSyncCorruptedFormat" );
 
   deleteLocalDir( mApi, "testExcludeFromSync" );
 }
@@ -2181,6 +2182,68 @@ void TestMerginApi::testSelectiveSyncChangeSyncFolder()
   delete serverMirrorProjects;
 }
 
+void TestMerginApi::testSelectiveSyncCorruptedFormat()
+{
+
+  /*
+   * Case: Test what happens when someone uploads not valid config file (not valid json)
+   *
+   * We will create another API client that will serve as a server mirror, it will not use selective sync,
+   * but will simulate as if someone manipulated project from browser via Mergin
+   */
+  QString serverMirrorDataPath = mApi->projectsPath() + "/" + "serverMirror";
+  QDir serverMirrorDataDir( serverMirrorDataPath );
+  if ( !serverMirrorDataDir.exists() )
+    serverMirrorDataDir.mkpath( serverMirrorDataPath );
+
+  LocalProjectsManager *serverMirrorProjects = new LocalProjectsManager( serverMirrorDataPath + "/" );
+  MerginApi *serverMirror = new MerginApi( *serverMirrorProjects, this );
+  serverMirror->setSupportsSelectiveSync( false );
+
+  // Create a project with photos and mergin-config
+  QString projectName = "testSelectiveSyncCorruptedFormat";
+
+  QString projectClient1 = mApi->projectsPath() + "/" + projectName;
+  QString projectClient2 = mApiExtra->projectsPath() + "/" + projectName;
+  QString projectServer = serverMirror->projectsPath() + "/" + projectName;
+
+  createRemoteProject( mApi, mUsername, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" );
+  downloadRemoteProject( mApi, mUsername, projectName );
+
+  // Create photo files
+  QDir dir;
+  QString photoPathClient1( projectClient1 + "/" + "photos" );
+  if ( !dir.exists( photoPathClient1 ) )
+    dir.mkpath( photoPathClient1 );
+
+  QFile file( photoPathClient1 + "/" + "photoC1-A.jpg" );
+  file.open( QIODevice::WriteOnly );
+  file.close();
+
+  QFile file1( photoPathClient1 + "/" + "photoC1-B.png" );
+  file1.open( QIODevice::WriteOnly );
+  file1.close();
+
+  uploadRemoteProject( mApi, mUsername, projectName );
+  downloadRemoteProject( serverMirror, mUsername, projectName );
+
+  // add corrupted config file
+  QString configFilePath = projectServer + "/" + "mergin-config.json";
+  QVERIFY( QFile::copy( mTestDataPath + "/mergin-config-corrupted.json", configFilePath ) );
+
+  uploadRemoteProject( serverMirror, mUsername, projectName );
+  downloadRemoteProject( mApiExtra, mUsername, projectName );
+
+  // client 2 should have all photos from client 1
+  QString photoPathClient2( projectClient2 + "/" + "photos" );
+
+  QFile fileExtra( photoPathClient2 + "/" + "photoC1-A.jpg" );
+  QVERIFY( fileExtra.exists() );
+
+  QFile fileExtra1( photoPathClient2 + "/" + "photoC1-B.png" );
+  QVERIFY( fileExtra1.exists() );
+}
+
 void TestMerginApi::testRegister()
 {
   QString password = mApi->userAuth()->password();

app/test/testmerginapi.h

@@ -72,6 +72,7 @@ class TestMerginApi: public QObject
     void testSelectiveSyncRemoveConfig();
     void testSelectiveSyncDisabledInConfig();
     void testSelectiveSyncChangeSyncFolder();
+    void testSelectiveSyncCorruptedFormat();
 
     void testRegister();
 

core/merginapi.cpp

@@ -361,7 +361,7 @@ void MerginApi::cacheServerConfig()
     transaction.replyDownloadItem->deleteLater();
     transaction.replyDownloadItem = nullptr;
 
-    prepareDownloadConfig( projectFullName );
+    prepareDownloadConfig( projectFullName, true );
   }
   else
   {
@@ -1953,7 +1953,7 @@ void MerginApi::startProjectUpdate( const QString &projectFullName )
   downloadNextItem( projectFullName );
 }
 
-void MerginApi::prepareDownloadConfig( const QString &projectFullName )
+void MerginApi::prepareDownloadConfig( const QString &projectFullName, bool downloaded )
 {
   Q_ASSERT( mTransactionalStatus.contains( projectFullName ) );
   TransactionStatus &transaction = mTransactionalStatus[projectFullName];
@@ -1968,7 +1968,7 @@ void MerginApi::prepareDownloadConfig( const QString &projectFullName )
 
   if ( serverContainsConfig )
   {
-    if ( !transaction.config.isValid )
+    if ( !downloaded )
     {
       // we should have server config but we do not have it yet
       return requestServerConfig( projectFullName );
@@ -1984,7 +1984,13 @@ void MerginApi::prepareDownloadConfig( const QString &projectFullName )
 
   bool previousVersionContainedConfig = ( resOld != oldServerVersion.files.end() ) && !transaction.firstTimeDownload;
 
-  if ( serverContainsConfig && previousVersionContainedConfig )
+  if ( !transaction.config.isValid )
+  {
+    // if transaction is not valid, consider it as deleted
+    transaction.config.downloadMissingFiles = true;
+    CoreUtils::log( "MerginConfig", "Config has invalid structure, continuing as if project had no config" );
+  }
+  else if ( serverContainsConfig && previousVersionContainedConfig )
   {
     // config was there, check if there are changes
     QString newChk = newServerVersion.fileInfo( sMerginConfigFile ).checksum;

core/merginapi.h

@@ -575,7 +575,7 @@ class MerginApi: public QObject
     void startProjectUpdate( const QString &projectFullName );
 
     //! Takes care of finding the correct config file, appends it to current transaction and proceeds with project update
-    void prepareDownloadConfig( const QString &projectFullName );
+    void prepareDownloadConfig( const QString &projectFullName, bool downloaded = false );
     void requestServerConfig( const QString &projectFullName );
 
     //! Starts download request of another item

test/test_data/mergin-config-corrupted.json

@@ -0,0 +1,4 @@
+{
+  "input-selective-sync": true
+  "input-selective-sync-dir": "photos" 
+}