SyncEngine: invalid the blacklist entry when the rename destination change

The problem in this case is if we rename the file "xxx" to "invalid\file".
The rename will fail because the new filename constains a slash, and it
will be blacklisted.
But then if the user re-rename the file to "valid_name", then we should
invalidate the blacklist entry and retry to upload. But we did not do
that because renaming don't change the mtime and we did not store the
rename target in the database

IL issue 558

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 98d5802420ee40c94d69942f801a9c58677c7911..a7928d081228fbc46f3d6b12626bdee4f4cb8a12 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -183,6 +183,12 @@ QString SyncEngine::csyncErrorToString(CSYNC_STATUS err)
 
 }
 
+/**
+ * Check if the item is in the blacklist.
+ * If it should not be sync'ed because of the blacklist, update the item with the error instruction
+ * and proper error message, and return true.
+ * If the item is not in the blacklist, or the blacklist is stale, return false.
+ */
 bool SyncEngine::checkErrorBlacklisting( SyncFileItem &item )
 {
     if( !_journal ) {
@@ -214,6 +220,9 @@ bool SyncEngine::checkErrorBlacklisting( SyncFileItem &item )
         } else if( item._modtime != entry._lastTryModtime ) {
             qDebug() << item._file << " is blacklisted, but has changed mtime!";
             return false;
+        } else if( item._renameTarget != entry._renameTarget) {
+            qDebug() << item._file << " is blacklisted, but rename target changed from" << entry._renameTarget;
+            return false;
         }
     } else if( item._direction == SyncFileItem::Down ) {
         // download, check the etag.

diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp
index 50506f9ea3aa50fb7957f4ebe7587b8854c5f9e2..fc6aabad2d41160e00921ce852a2a4e96eadd650 100644 (file)
--- a/src/libsync/syncjournaldb.cpp
+++ b/src/libsync/syncjournaldb.cpp
@@ -409,7 +409,7 @@ bool SyncJournalDb::checkConnect()
     _deleteFileRecordRecursively.reset(new SqlQuery(_db));
     _deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE path LIKE(?||'/%')");
 
-    QString sql( "SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration "
+    QString sql( "SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget "
                  "FROM blacklist WHERE path=?1");
     if( Utility::fsCasePreserving() ) {
         // if the file system is case preserving we have to check the blacklist
@@ -421,8 +421,8 @@ bool SyncJournalDb::checkConnect()
 
     _setErrorBlacklistQuery.reset(new SqlQuery(_db));
     _setErrorBlacklistQuery->prepare("INSERT OR REPLACE INTO blacklist "
-                                "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration) "
-                                "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7)");
+                                "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget) "
+                                "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)");
 
     _getSelectiveSyncListQuery.reset(new SqlQuery(_db));
     _getSelectiveSyncListQuery->prepare("SELECT path FROM selectivesync WHERE type=?1");
@@ -612,6 +612,15 @@ bool SyncJournalDb::updateErrorBlacklistTableStructure()
         }
         commitInternal("update database structure: add lastTryTime, ignoreDuration cols");
     }
+    if( columns.indexOf(QLatin1String("renameTarget")) == -1 ) {
+        SqlQuery query(_db);
+        query.prepare("ALTER TABLE blacklist ADD COLUMN renameTarget VARCHAR(4096);");
+        if( !query.exec() ) {
+            sqlFail("updateBlacklistTableStructure: Add renameTarget", query);
+            re = false;
+        }
+        commitInternal("update database structure: add lastTryTime, ignoreDuration cols");
+    }
 
     SqlQuery query(_db);
     query.prepare("CREATE INDEX IF NOT EXISTS blacklist_index ON blacklist(path collate nocase);");
@@ -1213,6 +1222,7 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry( const QStrin
                 entry._errorString    = _getErrorBlacklistQuery->stringValue(3);
                 entry._lastTryTime    = _getErrorBlacklistQuery->int64Value(4);
                 entry._ignoreDuration = _getErrorBlacklistQuery->int64Value(5);
+                entry._renameTarget   = _getErrorBlacklistQuery->stringValue(6);
                 entry._file           = file;
             }
             _getErrorBlacklistQuery->reset_and_clear_bindings();
@@ -1324,13 +1334,14 @@ void SyncJournalDb::updateErrorBlacklistEntry( const SyncJournalErrorBlacklistRe
     _setErrorBlacklistQuery->bindValue(5, item._errorString);
     _setErrorBlacklistQuery->bindValue(6, QString::number(item._lastTryTime));
     _setErrorBlacklistQuery->bindValue(7, QString::number(item._ignoreDuration));
+    _setErrorBlacklistQuery->bindValue(8, item._renameTarget);
     if( !_setErrorBlacklistQuery->exec() ) {
         QString bug = _setErrorBlacklistQuery->error();
         qDebug() << "SQL exec blacklistitem insert or replace failed: "<< bug;
     }
     qDebug() << "set blacklist entry for " << item._file << item._retryCount
              << item._errorString << item._lastTryTime << item._ignoreDuration
-             << item._lastTryModtime << item._lastTryEtag;
+             << item._lastTryModtime << item._lastTryEtag << item._renameTarget ;
     _setErrorBlacklistQuery->reset_and_clear_bindings();
 
 }

diff --git a/src/libsync/syncjournalfilerecord.cpp b/src/libsync/syncjournalfilerecord.cpp
index 96f90076774bce611e9f7313a445a076f2276ee0..583b7c41d8d7ad8f0809814e1c5e1f6eb1ebe07b 100644 (file)
--- a/src/libsync/syncjournalfilerecord.cpp
+++ b/src/libsync/syncjournalfilerecord.cpp
@@ -151,6 +151,7 @@ SyncJournalErrorBlacklistRecord SyncJournalErrorBlacklistRecord::update(
     // The factor of 5 feels natural: 25s, 2 min, 10 min, ~1h, ~5h, ~24h
     entry._ignoreDuration = old._ignoreDuration * 5;
     entry._file = item._file;
+    entry._renameTarget = item._renameTarget;
 
     if( item._httpErrorCode == 403 ) {
         qDebug() << "Probably firewall error: " << item._httpErrorCode << ", blacklisting up to 1h only";

diff --git a/src/libsync/syncjournalfilerecord.h b/src/libsync/syncjournalfilerecord.h
index ec2ec9c16646a32cdd153d2ded911cb375fea011..ce52b3e82a627baee727963cbfba166b49bb7795 100644 (file)
--- a/src/libsync/syncjournalfilerecord.h
+++ b/src/libsync/syncjournalfilerecord.h
@@ -89,6 +89,7 @@ public:
     time_t     _ignoreDuration;
 
     QString    _file;
+    QString    _renameTarget;
 
     bool isValid() const;