diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 41fd96c7b..20f566f0d 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -367,6 +367,27 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( item->_directDownloadUrl = serverEntry.directDownloadUrl; item->_directDownloadCookies = serverEntry.directDownloadCookies; + // Check for missing server data + { + QStringList missingData; + if (serverEntry.size == -1) + missingData.append(tr("size")); + if (serverEntry.remotePerm.isNull()) + missingData.append(tr("permissions")); + if (serverEntry.etag.isEmpty()) + missingData.append(tr("etag")); + if (serverEntry.fileId.isEmpty()) + missingData.append(tr("file id")); + if (!missingData.isEmpty()) { + item->_instruction = CSYNC_INSTRUCTION_ERROR; + item->_status = SyncFileItem::NormalError; + _childIgnored = true; + item->_errorString = tr("server reported no %1").arg(missingData.join(QLatin1String(", "))); + emit _discoveryData->itemDiscovered(item); + return; + } + } + // The file is known in the db already if (dbEntry.isValid()) { if (serverEntry.isDirectory != dbEntry.isDirectory()) { diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index bb80a9a53..e53b2db66 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -332,16 +332,6 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con propertyMapToRemoteInfo(map, result); if (result.isDirectory) result.size = 0; - if (result.size == -1 - || result.remotePerm.isNull() - || result.etag.isEmpty() - || result.fileId.isEmpty()) { - _error = tr("The server file discovery reply is missing data."); - qCWarning(lcDiscovery) - << "Missing properties:" << file << result.isDirectory << result.size - << result.modtime << result.remotePerm.toString() - << result.etag << result.fileId; - } if (_isExternalStorage && result.remotePerm.hasPermission(RemotePermissions::IsMounted)) { /* All the entries in a external storage have 'M' in their permission. However, for all diff --git a/test/testremotediscovery.cpp b/test/testremotediscovery.cpp index 43ef76a07..e18622d5a 100644 --- a/test/testremotediscovery.cpp +++ b/test/testremotediscovery.cpp @@ -12,6 +12,16 @@ using namespace OCC; +SyncFileItemPtr findItem(const QSignalSpy &spy, const QString &path) +{ + for (const QList<QVariant> &args : spy) { + auto item = args[0].value<SyncFileItemPtr>(); + if (item->destination() == path) + return item; + } + return SyncFileItemPtr(new SyncFileItem); +} + struct FakeBrokenXmlPropfindReply : FakePropfindReply { FakeBrokenXmlPropfindReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent) @@ -38,7 +48,6 @@ struct MissingPermissionsPropfindReply : FakePropfindReply { enum ErrorKind : int { // Lower code are corresponding to HTML error code InvalidXML = 1000, - MissingPermissions, Timeout, }; @@ -64,7 +73,6 @@ private slots: // 200 should be an error since propfind should return 207 QTest::newRow("200") << 200 << httpErrorMessage; QTest::newRow("InvalidXML") << +InvalidXML << "error while reading directory 'B' : Unknown error"; - QTest::newRow("MissingPermissions") << +MissingPermissions << "error while reading directory 'B' : The server file discovery reply is missing data."; QTest::newRow("Timeout") << +Timeout << "error while reading directory 'B' : Operation canceled"; } @@ -94,8 +102,6 @@ private slots: if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith("/B")) { if (errorKind == InvalidXML) { return new FakeBrokenXmlPropfindReply(fakeFolder.remoteModifier(), op, req, this); - } else if (errorKind == MissingPermissions) { - return new MissingPermissionsPropfindReply(fakeFolder.remoteModifier(), op, req, this); } else if (errorKind == Timeout) { return new FakeHangingReply(op, req, this); } else if (errorKind < 1000) { @@ -125,6 +131,37 @@ private slots: QCOMPARE(fakeFolder.currentRemoteState().children["C"], fakeFolder.currentLocalState().children["C"]); } } + + void testMissingData() + { + FakeFolder fakeFolder{ FileInfo() }; + fakeFolder.remoteModifier().insert("good"); + fakeFolder.remoteModifier().insert("noetag"); + fakeFolder.remoteModifier().find("noetag")->etag.clear(); + fakeFolder.remoteModifier().insert("nofileid"); + fakeFolder.remoteModifier().find("nofileid")->fileId.clear(); + fakeFolder.remoteModifier().mkdir("nopermissions"); + fakeFolder.remoteModifier().insert("nopermissions/A"); + + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *) + -> QNetworkReply *{ + if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith("nopermissions")) + return new MissingPermissionsPropfindReply(fakeFolder.remoteModifier(), op, req, this); + return nullptr; + }); + + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + QVERIFY(!fakeFolder.syncOnce()); + + QCOMPARE(findItem(completeSpy, "good")->_instruction, CSYNC_INSTRUCTION_NEW); + QCOMPARE(findItem(completeSpy, "noetag")->_instruction, CSYNC_INSTRUCTION_ERROR); + QCOMPARE(findItem(completeSpy, "nofileid")->_instruction, CSYNC_INSTRUCTION_ERROR); + QCOMPARE(findItem(completeSpy, "nopermissions")->_instruction, CSYNC_INSTRUCTION_NEW); + QCOMPARE(findItem(completeSpy, "nopermissions/A")->_instruction, CSYNC_INSTRUCTION_ERROR); + QVERIFY(findItem(completeSpy, "noetag")->_errorString.contains("etag")); + QVERIFY(findItem(completeSpy, "nofileid")->_errorString.contains("file id")); + QVERIFY(findItem(completeSpy, "nopermissions/A")->_errorString.contains("permissions")); + } }; QTEST_GUILESS_MAIN(TestRemoteDiscovery)