[buteo-sync-plugin-carddav] Prefer definite addressbook resources. Co…

…ntributes to JB#43847 Some services (e.g. Memotoo) do not produce output which conforms to the RFC, specifically they don't tag their addressbook resources with the appropriate resource-type tag. Previously, we worked around that case by assuming that a resource which otherwise meet the requirements must be an addressbook resource. This commit fixes the behaviour so that we only assume that such a resource is an addressbook resource if NO OTHER resources are explicitly tagged with the appropriate resource-type tag. This fixes upsync with e.g. Yandex. Contributes to JB#43847
sailfishos · Apr 2, 2019 · 2fd6390 · 2fd6390
1 parent 2e038ef
commit 2fd6390
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 4 deletions.
diff --git a/rpm/buteo-sync-plugin-carddav.spec b/rpm/buteo-sync-plugin-carddav.spec
@@ -31,7 +31,6 @@ A Buteo plugin which syncs contact data from CardDAV services
 Summary:    Unit tests for buteo-sync-plugin-carddav
 Group:      System/Libraries
 BuildRequires:  pkgconfig(Qt5Test)
-Requires:   blts-tools
 Requires:   %{name} = %{version}-%{release}
 
 %description tests
@@ -60,6 +59,7 @@ This package contains unit tests for the CardDAV Buteo sync plugin.
 /opt/tests/buteo/plugins/carddav/data/replyparser_addressbookinformation_addressbook-plus-contact.xml
 /opt/tests/buteo/plugins/carddav/data/replyparser_addressbookinformation_addressbook-calendar-principal.xml
 /opt/tests/buteo/plugins/carddav/data/replyparser_addressbookinformation_addressbook-principal-proxy.xml
+/opt/tests/buteo/plugins/carddav/data/replyparser_addressbookinformation_addressbook-plus-collection-resource.xml
 /opt/tests/buteo/plugins/carddav/data/replyparser_synctokendelta_empty.xml
 /opt/tests/buteo/plugins/carddav/data/replyparser_synctokendelta_single-well-formed-add-mod-rem.xml
 /opt/tests/buteo/plugins/carddav/data/replyparser_synctokendelta_single-well-formed-addition.xml

diff --git a/src/replyparser.cpp b/src/replyparser.cpp
@@ -250,6 +250,8 @@ QList<ReplyParser::AddressBookInformation> ReplyParser::parseAddressbookInformat
     debugDumpData(QString::fromUtf8(addressbookInformationResponse));
     QXmlStreamReader reader(addressbookInformationResponse);
     QList<ReplyParser::AddressBookInformation> infos;
+    QList<ReplyParser::AddressBookInformation> possibleAddressbookInfos;
+    QList<ReplyParser::AddressBookInformation> unlikelyAddressbookInfos;
 
     QVariantMap vmap = xmlToVMap(reader);
     QVariantMap multistatusMap = vmap[QLatin1String("multistatus")].toMap();
@@ -338,8 +340,8 @@ QList<ReplyParser::AddressBookInformation> ReplyParser::parseAddressbookInformat
                         // This is probably a carddav addressbook collection.
                         // Despite section 5.2 of RFC6352 stating that a CardDAV
                         // server MUST return the 'addressbook' value in the resource types
-                        // property, some CardDAV implementations (eg, Memotoo) do not.
-                        addressbookResourceSpecified = StatusExplicitlyTrue;
+                        // property, some CardDAV implementations (eg, Memotoo, Kerio) do not.
+                        addressbookResourceSpecified = StatusUnknown;
                         LOG_DEBUG(Q_FUNC_INFO << "have probable addressbook resource:" << currInfo.url);
                     } else {
                         // we don't know how to handle this resource type.
@@ -397,7 +399,17 @@ QList<ReplyParser::AddressBookInformation> ReplyParser::parseAddressbookInformat
                 && addressbookResourceSpecified == StatusUnknown   // resource type unknown
                 && otherPropertyStatus == StatusExplicitly2xxOk) { // status was explicitly ok
             // we assume that this was an implicit Addressbook Collection resourcetype response.
-            LOG_DEBUG(Q_FUNC_INFO << "have probable addressbook resource with status OK:" << currInfo.url);
+            // append it to our list of possible addressbook infos, to be added if we have no "certain" addressbooks.
+            LOG_DEBUG(Q_FUNC_INFO << "have possible addressbook resource with status OK:" << currInfo.url);
+            possibleAddressbookInfos.append(currInfo);
+            continue;
+        } else if (addressbookResourceSpecified == StatusUnknown
+                && resourcetypeStatus == StatusExplicitly2xxOk) {
+            // workaround for Kerio servers.  The "principal" may be used as
+            // the carddav addressbook url if no other urls are valid.
+            LOG_DEBUG(Q_FUNC_INFO << "have unlikely addressbook resource with status OK:" << currInfo.url);
+            unlikelyAddressbookInfos.append(currInfo);
+            continue;
         } else {
             // we either cannot infer that this was an Addressbook Collection
             // or we were told explicitly that the collection status was NOT OK.
@@ -415,6 +427,17 @@ QList<ReplyParser::AddressBookInformation> ReplyParser::parseAddressbookInformat
         infos.append(currInfo);
     }
 
+    // if the server was returning malformed response (without 'addressbook' resource type)
+    // we can still use the response path as an addressbook url in some cases (e.g. Memotoo).
+    if (infos.isEmpty()) {
+        LOG_DEBUG(Q_FUNC_INFO << "Have no certain addressbook resources; assuming possible resources are addressbooks!");
+        infos = possibleAddressbookInfos;
+        if (infos.isEmpty()) {
+            LOG_DEBUG(Q_FUNC_INFO << "Have no possible addressbook resources; assuming unlikely resources are addressbooks!");
+            infos = unlikelyAddressbookInfos;
+        }
+    }
+
     return infos;
 }
 

diff --git a/...lyparser/data/replyparser_addressbookinformation_addressbook-plus-collection-resource.xml b/...lyparser/data/replyparser_addressbookinformation_addressbook-plus-collection-resource.xml
@@ -0,0 +1,42 @@
+<?xml version='1.0' encoding='utf-8'?>
+<D:multistatus xmlns:D="DAV:">
+    <D:response>
+        <href xmlns="DAV:">/carddav/accountname%40server.tld/</href>
+        <D:propstat>
+            <D:prop>
+                <D:displayname>Display Name</D:displayname>
+                <D:resourcetype>
+                    <D:collection/>
+                </D:resourcetype>
+            </D:prop>
+            <status xmlns="DAV:">HTTP/1.1 200 OK</status>
+        </D:propstat>
+        <D:propstat>
+            <D:prop>
+                <D:sync-token/>
+                <getctag xmlns="http://calendarserver.org/ns/"/>
+            </D:prop>
+            <status xmlns="DAV:">HTTP/1.1 404 Not Found</status>
+        </D:propstat>
+    </D:response>
+    <D:response>
+        <href xmlns="DAV:">/carddav/accountname%40server.tld/addressbook/</href>
+        <D:propstat>
+            <D:prop>
+                <D:displayname>Display Name</D:displayname>
+                <D:resourcetype>
+                    <E:addressbook xmlns:E="urn:ietf:params:xml:ns:carddav"/>
+                    <D:collection/>
+                </D:resourcetype>
+                <getctag xmlns="http://calendarserver.org/ns/">123456789</getctag>
+            </D:prop>
+            <status xmlns="DAV:">HTTP/1.1 200 OK</status>
+        </D:propstat>
+        <D:propstat>
+            <D:prop>
+                <D:sync-token/>
+            </D:prop>
+            <status xmlns="DAV:">HTTP/1.1 404 Not Found</status>
+        </D:propstat>
+    </D:response>
+</D:multistatus>
diff --git a/tests/replyparser/tst_replyparser.cpp b/tests/replyparser/tst_replyparser.cpp
@@ -235,6 +235,18 @@ void tst_replyparser::parseAddressbookInformation_data()
         << QStringLiteral("data/replyparser_addressbookinformation_addressbook-principal-proxy.xml")
         << QString() // in the non-discovery case, the user provides the addressbook-home-set path directly.
         << infos;    // we then don't pass that into the parseAddressbookInformation() function, to avoid incorrect cycle detection.
+
+    infos.clear();
+    ReplyParser::AddressBookInformation a6;
+    a6.url = QStringLiteral("/carddav/accountname@server.tld/addressbook/");
+    a6.displayName = QStringLiteral("Display Name");
+    a6.ctag = QStringLiteral("123456789");
+    a6.syncToken = QString();
+    infos << a6;
+    QTest::newRow("addressbook information in response including home-set collections resource")
+        << QStringLiteral("data/replyparser_addressbookinformation_addressbook-plus-collection-resource.xml")
+        << QStringLiteral("/carddav/accountname%40server.tld/addressbook/")
+        << infos;
 }
 
 bool operator==(const ReplyParser::AddressBookInformation& first, const ReplyParser::AddressBookInformation& second)
@@ -259,6 +271,7 @@ void tst_replyparser::parseAddressbookInformation()
     QByteArray addressbookInformationResponse = f.readAll();
     QList<ReplyParser::AddressBookInformation> addressbookInfo = m_rp.parseAddressbookInformation(addressbookInformationResponse, addressbooksHomePath);
 
+    QCOMPARE(addressbookInfo.size(), expectedAddressbookInformation.size());
     QCOMPARE(addressbookInfo, expectedAddressbookInformation);
 }