diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bc1e541..332f34f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Version 0.16.0 - Strip ``METHOD:PUBLISH`` added by some calendar providers. - Fix crash of Google storages when saving token file. +- Make DAV discovery more RFC-conformant, see :ghpr:`585`. Version 0.15.0 ============== diff --git a/vdirsyncer/http.py b/vdirsyncer/http.py index f59d837..8ba299b 100644 --- a/vdirsyncer/http.py +++ b/vdirsyncer/http.py @@ -150,6 +150,8 @@ def request(method, url, session=None, latin1_fallback=True, if verify_fingerprint is not None: _install_fingerprint_adapter(session, verify_fingerprint) + session.hooks = dict(response=_fix_redirects) + func = session.request logger.debug(u'{} {}'.format(method, url)) @@ -180,3 +182,21 @@ def request(method, url, session=None, latin1_fallback=True, r.raise_for_status() return r + + +def _fix_redirects(r, *args, **kwargs): + ''' + Requests discards of the body content when it is following a redirect that + is not a 307 or 308. We never want that to happen. + + See: + https://github.com/kennethreitz/requests/issues/3915 + https://github.com/pimutils/vdirsyncer/pull/585 + https://github.com/pimutils/vdirsyncer/issues/586 + + FIXME: This solution isn't very nice. A new hook in requests would be + better. + ''' + if r.is_redirect: + logger.debug('Rewriting status code from %s to 307', r.status_code) + r.status_code = 307 diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 0d17ded..39403f2 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -134,23 +134,14 @@ class Discover(object): _, collection = url.rstrip('/').rsplit('/', 1) return urlparse.unquote(collection) - def find_dav(self): + def find_principal(self): try: - response = self.session.request( - 'GET', self._well_known_uri, allow_redirects=True, - headers=self.session.get_default_headers() - ) - return response.url + return self._find_principal_impl('') except (HTTPError, exceptions.Error): - # The user might not have well-known URLs set up and instead points - # vdirsyncer directly to the DAV server. - dav_logger.debug('Server does not support well-known URIs.') - return '' - - def find_principal(self, url=None): - if url is None: - url = self.find_dav() + dav_logger.debug('Trying out well-known URI') + return self._find_principal_impl(self._well_known_uri) + def _find_principal_impl(self, url): headers = self.session.get_default_headers() headers['Depth'] = '0' body = b""" @@ -160,8 +151,10 @@ class Discover(object): """ + response = self.session.request('PROPFIND', url, headers=headers, data=body) + root = _parse_xml(response.content) rv = root.find('.//{DAV:}current-user-principal/{DAV:}href') if rv is None: @@ -174,9 +167,8 @@ class Discover(object): return response.url return urlparse.urljoin(response.url, rv.text) - def find_home(self, url=None): - if url is None: - url = self.find_principal() + def find_home(self): + url = self.find_principal() headers = self.session.get_default_headers() headers['Depth'] = '0' response = self.session.request('PROPFIND', url, diff --git a/vdirsyncer/storage/google.py b/vdirsyncer/storage/google.py index fab681a..8e3d3ec 100644 --- a/vdirsyncer/storage/google.py +++ b/vdirsyncer/storage/google.py @@ -159,8 +159,13 @@ class GoogleContactsStorage(dav.CardDAVStorage): ''' + GOOGLE_PARAMS_DOCS class session_class(GoogleSession): - # Apparently Google wants us to submit a PROPFIND to the well-known - # URL, instead of looking for a redirect. + # Google CardDAV is completely bonkers. Collection discovery doesn't + # work properly, well-known URI takes us directly to single collection + # from where we can't discover principal or homeset URIs (the PROPFINDs + # 404). + # + # So we configure the well-known URI here again, such that discovery + # tries collection enumeration on it directly. That appears to work. url = 'https://www.googleapis.com/.well-known/carddav' scope = ['https://www.googleapis.com/auth/carddav']