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
This commit is contained in:
Olivier Goffart 2016-05-25 15:01:26 +02:00
parent fc1933803e
commit f6b35e5d58
4 changed files with 26 additions and 4 deletions

View file

@ -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.

View file

@ -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();
}

View file

@ -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";

View file

@ -89,6 +89,7 @@ public:
time_t _ignoreDuration;
QString _file;
QString _renameTarget;
bool isValid() const;