Skip to content

Commit

Permalink
Merge branch 'jb43847' into 'master'
Browse files Browse the repository at this point in the history 
Fix a variety of issues with CardDAV sync with standards-compliant services

See merge request mer-core/buteo-sync-plugin-carddav!19
  • Loading branch information
chriadam committed Apr 2, 2019
2 parents e348a2f + ff465bd commit f956e6e
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 69 deletions.
3 changes: 2 additions & 1 deletion rpm/buteo-sync-plugin-carddav.spec
View file
Expand Up @@ -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
Expand Down Expand Up @@ -60,11 +59,13 @@ 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
/opt/tests/buteo/plugins/carddav/data/replyparser_contactmetadata_empty.xml
/opt/tests/buteo/plugins/carddav/data/replyparser_contactmetadata_single-well-formed-add-mod-rem-unch.xml
/opt/tests/buteo/plugins/carddav/data/replyparser_contactmetadata_single-vcf-and-non-vcf.xml
/opt/tests/buteo/plugins/carddav/data/replyparser_contactdata_empty.xml
/opt/tests/buteo/plugins/carddav/data/replyparser_contactdata_single-well-formed.xml
/opt/tests/buteo/plugins/carddav/data/replyparser_contactdata_single-hs-utc-iso8601-bday.xml
Expand Down
9 changes: 7 additions & 2 deletions src/carddav.cpp
View file
Expand Up @@ -506,6 +506,11 @@ void CardDav::fetchUserInformation()
*/

QUrl serverUrl(m_serverUrl);
if (serverUrl.scheme().isEmpty() && (serverUrl.host().isEmpty() || serverUrl.path().isEmpty())) {
// assume the supplied server url is like: "carddav.server.tld"
m_serverUrl = QStringLiteral("https://%1/").arg(m_serverUrl);
serverUrl = QUrl(m_serverUrl);
}
QString wellKnownUrl = QStringLiteral("%1://%2/.well-known/carddav").arg(serverUrl.scheme()).arg(serverUrl.host());
bool firstRequest = m_discoveryStage == CardDav::DiscoveryStarted;
m_serverUrl = firstRequest && (serverUrl.path().isEmpty() || serverUrl.path() == QStringLiteral("/"))
Expand Down Expand Up @@ -658,7 +663,7 @@ void CardDav::addressbookUrlsResponse()

void CardDav::fetchAddressbooksInformation(const QString &addressbooksHomePath)
{
LOG_DEBUG(Q_FUNC_INFO << "requesting addressbook sync information");
LOG_DEBUG(Q_FUNC_INFO << "requesting addressbook sync information from" << addressbooksHomePath);
QNetworkReply *reply = m_request->addressbooksInformation(m_serverUrl, addressbooksHomePath);
reply->setProperty("addressbooksHomePath", addressbooksHomePath);
if (!reply) {
Expand Down Expand Up @@ -1089,7 +1094,7 @@ void CardDav::upsyncUpdates(const QString &addressbookUrl, const QList<QContact>
// transform into local-device guid
QString guid = QStringLiteral("%1:AB:%2:%3").arg(QString::number(q->m_accountId), addressbookUrl, uid);
// generate a valid uri
QString uri = addressbookUrl + "/" + uid + ".vcf";
QString uri = addressbookUrl + (addressbookUrl.endsWith('/') ? QString() : QStringLiteral("/")) + uid + QStringLiteral(".vcf");
// update our state data
q->m_contactUids[guid] = uid;
q->m_contactUris[guid] = uri;
Expand Down
51 changes: 46 additions & 5 deletions src/replyparser.cpp
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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;
}

Expand Down Expand Up @@ -483,9 +506,17 @@ QList<ReplyParser::ContactInformation> ReplyParser::parseSyncTokenDelta(const QB
status = rmap.value("status").toMap().value("@text").toString();
}
if (status.contains(QLatin1String("200 OK"))) {
if (!currInfo.uri.endsWith(QStringLiteral(".vcf"), Qt::CaseInsensitive)) {
if (currInfo.uri.endsWith(QChar('/'))) {
// this is probably a response for the addressbook resource,
// rather than for a contact resource within the addressbook.
LOG_DEBUG(Q_FUNC_INFO << "ignoring non-contact (addressbook?) resource:" << currInfo.uri << currInfo.etag << status);
continue;
} else if (currInfo.uri.length() > 5
&& (currInfo.uri.at(currInfo.uri.length()-4) == QChar('.')
|| currInfo.uri.at(currInfo.uri.length()-3) == QChar('.'))
&& !currInfo.uri.endsWith(QStringLiteral(".vcf"), Qt::CaseInsensitive)) {
// the uri has a file suffix like .ics or .eml rather than .vcf.
// this is probably not a contact resource, but instead some other file reported erroneously.
LOG_DEBUG(Q_FUNC_INFO << "ignoring non-contact resource:" << currInfo.uri << currInfo.etag << status);
continue;
}
Expand Down Expand Up @@ -559,12 +590,22 @@ QList<ReplyParser::ContactInformation> ReplyParser::parseContactMetadata(const Q
if (status.isEmpty()) {
status = rmap.value("status").toMap().value("@text").toString();
}
if (!currInfo.uri.endsWith(QStringLiteral(".vcf"), Qt::CaseInsensitive)) {

if (currInfo.uri.endsWith(QChar('/'))) {
// this is probably a response for the addressbook resource,
// rather than for a contact resource within the addressbook.
LOG_DEBUG(Q_FUNC_INFO << "ignoring non-contact (addressbook?) resource:" << currInfo.uri << currInfo.etag << status);
continue;
} else if (currInfo.uri.length() > 5
&& (currInfo.uri.at(currInfo.uri.length()-4) == QChar('.')
|| currInfo.uri.at(currInfo.uri.length()-3) == QChar('.'))
&& !currInfo.uri.endsWith(QStringLiteral(".vcf"), Qt::CaseInsensitive)) {
// the uri has a file suffix like .ics or .eml rather than .vcf.
// this is probably not a contact resource, but instead some other file reported erroneously.
LOG_DEBUG(Q_FUNC_INFO << "ignoring non-contact resource:" << currInfo.uri << currInfo.etag << status);
continue;
}

QMap<QString, QString>::const_iterator it = q->m_contactUris.constBegin();
for ( ; it != q->m_contactUris.constEnd(); ++it) {
if (it.value() == currInfo.uri) {
Expand Down
131 changes: 70 additions & 61 deletions src/requestgenerator.cpp
View file
Expand Up @@ -36,6 +36,68 @@

#include <QtContacts/QContact>

namespace {
QUrl setRequestUrl(const QString &url, const QString &path, const QString &username, const QString &password)
{
QUrl ret(url);
QString modifiedPath(path);
if (!path.isEmpty()) {
// common case: the path may contain %40 instead of @ symbol,
// if the server returns paths in percent-encoded form.
// QUrl::setPath() will automatically percent-encode the input,
// so if we have received percent-encoded path, we need to undo
// the percent encoding first. This is suboptimal but works
// at least for the common case.
if (path.contains(QStringLiteral("%40"))) {
modifiedPath = QUrl::fromPercentEncoding(path.toUtf8());
}

// override the path from the given url with the path argument.
// this is because the initial URL may be a user-principals URL
// but subsequent paths are not relative to that one, but instead
// are relative to the root path /
if (path.startsWith('/')) {
ret.setPath(modifiedPath);
} else {
ret.setPath('/' + modifiedPath);
}
}
if (!username.isEmpty() && !password.isEmpty()) {
ret.setUserName(username);
ret.setPassword(password);
}
return ret;
}

QNetworkRequest setRequestData(const QUrl &url,
const QByteArray &requestData,
const QString &depth,
const QString &ifMatch,
const QString &contentType,
const QString &accessToken)
{
QNetworkRequest ret(url);
if (!contentType.isEmpty()) {
ret.setHeader(QNetworkRequest::ContentTypeHeader,
contentType.toUtf8());
}
ret.setHeader(QNetworkRequest::ContentLengthHeader,
requestData.length());
if (!depth.isEmpty()) {
ret.setRawHeader("Depth", depth.toUtf8());
}
if (!ifMatch.isEmpty()) {
ret.setRawHeader("If-Match", ifMatch.toUtf8());
}
if (!accessToken.isEmpty()) {
ret.setRawHeader("Authorization",
QString(QLatin1String("Bearer ")
+ accessToken).toUtf8());
}
return ret;
}
}

RequestGenerator::RequestGenerator(Syncer *parent,
const QString &username,
const QString &password)
Expand All @@ -58,38 +120,10 @@ QNetworkReply *RequestGenerator::generateRequest(const QString &url,
const QString &requestType,
const QString &request) const
{
const QByteArray contentType("application/xml; charset=utf-8");
QByteArray requestData(request.toUtf8());
QUrl reqUrl(url);
if (!path.isEmpty()) {
// override the path from the given url with the path argument.
// this is because the initial URL may be a user-principals URL
// but subsequent paths are not relative to that one, but instead
// are relative to the root path /
if (path.startsWith('/')) {
reqUrl.setPath(path);
} else {
reqUrl.setPath('/' + path);
}
}
if (!m_username.isEmpty() && !m_password.isEmpty()) {
reqUrl.setUserName(m_username);
reqUrl.setPassword(m_password);
}

QNetworkRequest req(reqUrl);
req.setHeader(QNetworkRequest::ContentTypeHeader,
"application/xml; charset=utf-8");
req.setHeader(QNetworkRequest::ContentLengthHeader,
requestData.length());
if (!depth.isEmpty()) {
req.setRawHeader("Depth", depth.toUtf8());
}
if (!m_accessToken.isEmpty()) {
req.setRawHeader("Authorization",
QString(QLatin1String("Bearer ")
+ m_accessToken).toUtf8());
}

QUrl reqUrl(setRequestUrl(url, path, m_username, m_password));
QNetworkRequest req(setRequestData(reqUrl, requestData, depth, QString(), contentType, m_accessToken));
QBuffer *requestDataBuffer = new QBuffer(q);
requestDataBuffer->setData(requestData);
LOG_DEBUG("generateRequest():"
Expand All @@ -106,36 +140,8 @@ QNetworkReply *RequestGenerator::generateUpsyncRequest(const QString &url,
const QString &request) const
{
QByteArray requestData(request.toUtf8());
QUrl reqUrl(url);
if (!path.isEmpty()) {
// override the path from the given url with the path argument.
// this is because the initial URL may be a user-principals URL
// but subsequent paths are not relative to that one, but instead
// are relative to the root path /
reqUrl.setPath(path);
}
if (!m_username.isEmpty() && !m_password.isEmpty()) {
reqUrl.setUserName(m_username);
reqUrl.setPassword(m_password);
}

QNetworkRequest req(reqUrl);
if (!contentType.isEmpty()) {
req.setHeader(QNetworkRequest::ContentTypeHeader,
contentType);
}
if (!request.isEmpty()) {
req.setHeader(QNetworkRequest::ContentLengthHeader,
requestData.length());
}
if (!ifMatch.isEmpty()) {
req.setRawHeader("If-Match", ifMatch.toUtf8());
}
if (!m_accessToken.isEmpty()) {
req.setRawHeader("Authorization",
QString(QLatin1String("Bearer ")
+ m_accessToken).toUtf8());
}
QUrl reqUrl(setRequestUrl(url, path, m_username, m_password));
QNetworkRequest req(setRequestData(reqUrl, requestData, QString(), ifMatch, contentType, m_accessToken));

LOG_DEBUG("generateUpsyncRequest():" << m_accessToken << reqUrl << requestType << ":" << requestData.length() << "bytes");
Q_FOREACH (const QByteArray &headerName, req.rawHeaderList()) {
Expand Down Expand Up @@ -353,6 +359,9 @@ QNetworkReply *RequestGenerator::contactMultiget(const QString &serverUrl, const
}
if (uri.endsWith(QStringLiteral(".vcf")) && uri.startsWith(addressbookPath)) {
uriHrefs.append(QStringLiteral("<d:href>%1</d:href>").arg(href));
} else if (uri.startsWith(addressbookPath)) {
// contact resource which doesn't end in .vcf but is otherwise well-formed / fully specified.
uriHrefs.append(QStringLiteral("<d:href>%1</d:href>").arg(href));
} else {
uriHrefs.append(QStringLiteral("<d:href>%1/%2.vcf</d:href>").arg(addressbookPath).arg(href));
}
Expand Down
42 changes: 42 additions & 0 deletions ...lyparser/data/replyparser_addressbookinformation_addressbook-plus-collection-resource.xml
View file
@@ -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>
20 changes: 20 additions & 0 deletions tests/replyparser/data/replyparser_contactmetadata_single-vcf-and-non-vcf.xml
View file
@@ -0,0 +1,20 @@
<d:multistatus xmlns:d="DAV:" xmlns:card="urn:ietf:params:xml:ns:carddav">
<d:response>
<d:href>/addressbooks/johndoe/contacts/new.vcf</d:href>
<d:propstat>
<d:prop>
<d:getetag>"0021-0021"</d:getetag> <!-- new uri/etag :. added -->
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>/addressbooks/johndoe/contacts/alsonew</d:href>
<d:propstat>
<d:prop>
<d:getetag>"0022-0022"</d:getetag> <!-- new uri/etag :. added -->
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
</d:multistatus>

0 comments on commit f956e6e

Please sign in to comment.