A lot of bugfixes

- There may be multiple prop-tags, and vdirsyncer only picked one of
  them, seemingly at random.

- getcontenttype doesn't work for everything

These bugs were found with Baikal 0.2.7
This commit is contained in:
Markus Unterwaditzer 2015-01-04 00:14:28 +01:00
parent ccc3dee28b
commit 863d574261
2 changed files with 42 additions and 34 deletions

View file

@ -10,7 +10,7 @@ env:
- BUILD=tests DAV_SERVER=radicale RADICALE_BACKEND=multifilesystem REQUIREMENTS=devel - BUILD=tests DAV_SERVER=radicale RADICALE_BACKEND=multifilesystem REQUIREMENTS=devel
- BUILD=tests DAV_SERVER=radicale RADICALE_BACKEND=database REQUIREMENTS=devel - BUILD=tests DAV_SERVER=radicale RADICALE_BACKEND=database REQUIREMENTS=devel
- BUILD=tests DAV_SERVER=owncloud REQUIREMENTS=release - BUILD=tests DAV_SERVER=owncloud REQUIREMENTS=release
#- BUILD=tests DAV_SERVER=baikal REQUIREMENTS=release - BUILD=tests DAV_SERVER=baikal REQUIREMENTS=release
- BUILD=style - BUILD=style
install: install:

View file

@ -54,11 +54,9 @@ def _parse_xml(content):
.format(e)) .format(e))
def _find_xml(root, query, **kw): def _merge_xml(items):
rv = root.find(query, **kw) rv = items[0]
if rv is None: rv.extend(items[1:])
raise InvalidXMLResponse('Invalid response from the DAV server. '
'Was looking for {}.'.format(query))
return rv return rv
@ -125,8 +123,10 @@ class Discover(object):
response = self.session.request('PROPFIND', url, headers=headers, response = self.session.request('PROPFIND', url, headers=headers,
data=body, is_subpath=False) data=body, is_subpath=False)
root = _parse_xml(response.content) root = _parse_xml(response.content)
return _find_xml(root, ('.//{DAV:}current-user-principal' rv = root.find('.//{DAV:}current-user-principal/{DAV:}href')
'/{DAV:}href')).text if rv is None:
raise InvalidXMLResponse()
return rv.text
def find_home(self, url=None): def find_home(self, url=None):
if url is None: if url is None:
@ -140,7 +140,10 @@ class Discover(object):
root = etree.fromstring(response.content) root = etree.fromstring(response.content)
# Better don't do string formatting here, because of XML namespaces # Better don't do string formatting here, because of XML namespaces
return _find_xml(root, './/' + self._homeset_tag + '/{*}href').text rv = root.find('.//' + self._homeset_tag + '/{*}href')
if rv is None:
raise InvalidXMLResponse()
return rv.text
def find_collections(self, url=None): def find_collections(self, url=None):
if url is None: if url is None:
@ -153,13 +156,15 @@ class Discover(object):
root = _parse_xml(response.content) root = _parse_xml(response.content)
done = set() done = set()
for response in root.findall('{DAV:}response'): for response in root.findall('{DAV:}response'):
prop = _find_xml(response, '{*}propstat/{*}prop') props = _merge_xml(response.findall('{*}propstat/{*}prop'))
if prop.find('{*}resourcetype/{*}' + self._resourcetype) is None: if props.find('{*}resourcetype/{*}' + self._resourcetype) is None:
continue continue
displayname = getattr(prop.find('{*}displayname'), 'text', '') displayname = getattr(props.find('{*}displayname'), 'text', '')
href = utils.urlparse.urljoin(self.session.url, href = response.find('{*}href')
_find_xml(response, '{*}href').text) if href is None:
raise InvalidXMLResponse()
href = utils.urlparse.urljoin(self.session.url, href.text)
if href not in done: if href not in done:
done.add(href) done.add(href)
yield {'href': href, 'displayname': displayname} yield {'href': href, 'displayname': displayname}
@ -380,13 +385,13 @@ class DavStorage(Storage):
rv = [] rv = []
hrefs_left = set(hrefs) hrefs_left = set(hrefs)
for href, etag, prop in self._parse_prop_responses(root): for href, etag, prop in self._parse_prop_responses(root):
try: raw = prop.find(self.get_multi_data_query)
raw = _find_xml(prop, self.get_multi_data_query).text if raw is None:
except InvalidXMLResponse:
dav_logger.warning('Skipping {}, the item content is missing.' dav_logger.warning('Skipping {}, the item content is missing.'
.format(href)) .format(href))
continue continue
else:
raw = raw.text
if isinstance(raw, bytes): if isinstance(raw, bytes):
raw = raw.decode(response.encoding) raw = raw.decode(response.encoding)
if isinstance(etag, bytes): if isinstance(etag, bytes):
@ -454,13 +459,12 @@ class DavStorage(Storage):
def _parse_prop_responses(self, root, decoding_rounds=1): def _parse_prop_responses(self, root, decoding_rounds=1):
hrefs = set() hrefs = set()
for response in root.iter('{DAV:}response'): for response in root.iter('{DAV:}response'):
try: href = response.find('{DAV:}href')
href = _find_xml(response, '{DAV:}href').text if href is None:
except InvalidXMLResponse:
dav_logger.error('Skipping response, href is missing.') dav_logger.error('Skipping response, href is missing.')
continue continue
href = self._normalize_href(href) href = self._normalize_href(href.text)
for i in range(decoding_rounds): for i in range(decoding_rounds):
href = _decode_href(href) href = _decode_href(href)
@ -475,20 +479,26 @@ class DavStorage(Storage):
continue continue
try: try:
prop = _find_xml(response, '{DAV:}propstat/{DAV:}prop') props = response.findall('{DAV:}propstat/{DAV:}prop')
contenttype = _find_xml(prop, '{DAV:}getcontenttype').text if props is None:
etag = _find_xml(prop, '{DAV:}getetag').text raise InvalidXMLResponse()
props = _merge_xml(props)
etag = getattr(props.find('{DAV:}getetag'), 'text', '')
if not etag:
raise InvalidXMLResponse('Etag is missing.')
except InvalidXMLResponse as e: except InvalidXMLResponse as e:
dav_logger.error(str(e)) dav_logger.error(str(e))
dav_logger.warning('Skipping {!r}, properties are missing.' dav_logger.warning('Skipping {!r}, properties are missing.'
.format(href)) .format(href))
continue continue
is_collection = prop.find('{DAV:}resourcetype/{DAV:}collection') if props.find('{DAV:}resourcetype/{DAV:}collection') is not None:
if is_collection is not None:
dav_logger.debug('Skipping {!r}, is collection.'.format(href)) dav_logger.debug('Skipping {!r}, is collection.'.format(href))
continue continue
contenttype = getattr(props.find('{DAV:}getcontenttype'),
'text', None)
if not self._is_item_mimetype(contenttype): if not self._is_item_mimetype(contenttype):
dav_logger.debug('Skipping {!r}, {!r} != {!r}.' dav_logger.debug('Skipping {!r}, {!r} != {!r}.'
.format(href, contenttype, .format(href, contenttype,
@ -496,7 +506,7 @@ class DavStorage(Storage):
continue continue
hrefs.add(href) hrefs.add(href)
yield href, etag, prop yield href, etag, props
class CaldavStorage(DavStorage): class CaldavStorage(DavStorage):
@ -536,7 +546,6 @@ class CaldavStorage(DavStorage):
xmlns:C="urn:ietf:params:xml:ns:caldav"> xmlns:C="urn:ietf:params:xml:ns:caldav">
<D:prop> <D:prop>
<D:getetag/> <D:getetag/>
<D:getcontenttype/>
<C:calendar-data/> <C:calendar-data/>
</D:prop> </D:prop>
{hrefs} {hrefs}
@ -650,7 +659,6 @@ class CarddavStorage(DavStorage):
xmlns:C="urn:ietf:params:xml:ns:carddav"> xmlns:C="urn:ietf:params:xml:ns:carddav">
<D:prop> <D:prop>
<D:getetag/> <D:getetag/>
<D:getcontenttype/>
<C:address-data/> <C:address-data/>
</D:prop> </D:prop>
{hrefs} {hrefs}