mirrors/nextcloud-desktop
1
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2024-11-22 13:05:51 +03:00
Code Issues Projects Releases Packages Wiki Activity

Discovery: 403 and 503 on root cause error

Browse source 
Previously these result codes during remote discovery of the sync root
would not cause an error and the discovery would get stuck.

Also extends RemoteDiscovery tests to check for errors on the root item.
This commit is contained in:
Christian Kamm 2019-04-10 11:07:15 +02:00 committed by Kevin Ottens
parent 7b96321df2
commit c50f041c5b
No known key found for this signature in database
GPG key ID: 074BBBCB8DECC9E2
2 changed files with 49 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
src/libsync/discovery.cpp
View file

@ -1342,32 +1342,38 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery()
if (_localQueryDone)
process();
} else {
if (results.error().code == 403) {
// 403 Forbidden can be sent by the server if the file firewall is active.
// A file or directory should be ignored and sync must continue. See #3490
qCWarning(lcDisco, "Directory access Forbidden (File Firewall?)");
auto fatalError = [&] {
emit _discoveryData->fatalError(tr("Server replied with an error while reading directory '%1' : %2")
.arg(_currentFolder._server, results.error().message));
};
auto ignoreOrFatal = [&] {
if (_dirItem) {
_dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
_dirItem->_errorString = results.error().message;
emit finished();
return;
} else {
// Fatal for the root job since it has no SyncFileItem
fatalError();
}
};
if (results.error().code == 403) {
// 403 Forbidden can be sent by the server if the file firewall is active.
// A file or directory should be ignored and sync must continue. See #3490
qCWarning(lcDisco, "Directory access Forbidden (File Firewall?)");
ignoreOrFatal();
} else if (results.error().code == 503) {
// The server usually replies with the custom "503 Storage not available"
// if some path is temporarily unavailable. But in some cases a standard 503
// is returned too. Thus we can't distinguish the two and will treat any
// 503 as request to ignore the folder. See #3113 #2884.
qCWarning(lcDisco(), "Storage was not available!");
if (_dirItem) {
_dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
_dirItem->_errorString = results.error().message;
emit finished();
return;
ignoreOrFatal();
} else {
qCWarning(lcDisco) << "Server error in directory" << _currentFolder._server << results.error().message;
fatalError();
}
}
emit _discoveryData->fatalError(tr("Server replied with an error while reading directory '%1' : %2")
.arg(_currentFolder._server, results.error().message));
}
});
connect(serverJob, &DiscoverySingleDirectoryJob::firstDirectoryPermissions, this,
[this](const RemotePermissions &perms) { _rootPermissions = perms; });

43
test/testremotediscovery.cpp
View file

@ -65,15 +65,16 @@ private slots:
QTest::addColumn<int>("errorKind");
QTest::addColumn<QString>("expectedErrorString");
QString httpErrorMessage = "Server replied with an error while reading directory 'B' : Internal Server Fake Error";
QString itemErrorMessage = "Internal Server Fake Error";
QTest::newRow("404") << 404 << httpErrorMessage;
QTest::newRow("500") << 500 << httpErrorMessage;
QTest::newRow("503") << 503 << httpErrorMessage;
QTest::newRow("403") << 403 << itemErrorMessage;
QTest::newRow("404") << 404 << itemErrorMessage;
QTest::newRow("500") << 500 << itemErrorMessage;
QTest::newRow("503") << 503 << itemErrorMessage;
// 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("Timeout") << +Timeout << "error while reading directory 'B' : Operation canceled";
QTest::newRow("200") << 200 << itemErrorMessage;
QTest::newRow("InvalidXML") << +InvalidXML << "Unknown error";
QTest::newRow("Timeout") << +Timeout << "Operation canceled";
}
@ -82,7 +83,8 @@ private slots:
{
QFETCH(int, errorKind);
QFETCH(QString, expectedErrorString);
bool syncSucceeds = errorKind == 503; // 503 just ignore the temporarily unavailable directory
// 403/503 just ignore the temporarily unavailable directory
bool syncSucceeds = errorKind == 503 || errorKind == 403;
FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
@ -97,9 +99,11 @@ private slots:
auto oldLocalState = fakeFolder.currentLocalState();
auto oldRemoteState = fakeFolder.currentRemoteState();
QString errorFolder = "webdav/B";
QString fatalErrorPrefix = "Server replied with an error while reading directory 'B' : ";
fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *)
-> QNetworkReply *{
if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith("/B")) {
if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith(errorFolder)) {
if (errorKind == InvalidXML) {
return new FakeBrokenXmlPropfindReply(fakeFolder.remoteModifier(), op, req, this);
} else if (errorKind == Timeout) {
@ -114,22 +118,35 @@ private slots:
// So the test that test timeout finishes fast
QScopedValueRollback<int> setHttpTimeout(AbstractNetworkJob::httpTimeout, errorKind == Timeout ? 1 : 10000);
QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
QSignalSpy errorSpy(&fakeFolder.syncEngine(), &SyncEngine::syncError);
QCOMPARE(fakeFolder.syncOnce(), syncSucceeds);
qDebug() << "errorSpy=" << errorSpy;
// The folder B should not have been sync'ed (and in particular not removed)
QCOMPARE(oldLocalState.children["B"], fakeFolder.currentLocalState().children["B"]);
QCOMPARE(oldRemoteState.children["B"], fakeFolder.currentRemoteState().children["B"]);
if (!syncSucceeds) {
// Check we got the right error
QCOMPARE(errorSpy.count(), 1);
QVERIFY(errorSpy[0][0].toString().contains(expectedErrorString));
QCOMPARE(errorSpy.size(), 1);
QCOMPARE(errorSpy[0][0].toString(), QString(fatalErrorPrefix + expectedErrorString));
} else {
QCOMPARE(findItem(completeSpy, "B")->_instruction, CSYNC_INSTRUCTION_IGNORE);
QVERIFY(findItem(completeSpy, "B")->_errorString.contains(expectedErrorString));
// The other folder should have been sync'ed as the sync just ignored the faulty dir
QCOMPARE(fakeFolder.currentRemoteState().children["A"], fakeFolder.currentLocalState().children["A"]);
QCOMPARE(fakeFolder.currentRemoteState().children["C"], fakeFolder.currentLocalState().children["C"]);
QCOMPARE(findItem(completeSpy, "A/z1")->_instruction, CSYNC_INSTRUCTION_NEW);
}
//
// Check the same discovery error on the sync root
//
errorFolder = "webdav/";
fatalErrorPrefix = "Server replied with an error while reading directory '' : ";
errorSpy.clear();
QVERIFY(!fakeFolder.syncOnce());
QCOMPARE(errorSpy.size(), 1);
QCOMPARE(errorSpy[0][0].toString(), QString(fatalErrorPrefix + expectedErrorString));
}
void testMissingData()