Rewrite DAV storages' encoding behavior

This is more explicit than the old behavior. See
https://github.com/Kozea/Radicale/issues/128 for the discussion that led
to this.
This commit is contained in:
Markus Unterwaditzer 2014-12-07 14:16:18 +01:00
parent 7f2ccd6b3a
commit 741045c1be
2 changed files with 19 additions and 39 deletions

View file

@ -20,8 +20,7 @@ from tests import EVENT_TEMPLATE, TASK_TEMPLATE, VCARD_TEMPLATE
import vdirsyncer.exceptions as exceptions import vdirsyncer.exceptions as exceptions
from vdirsyncer.storage.base import Item from vdirsyncer.storage.base import Item
from vdirsyncer.storage.dav import CaldavStorage, CarddavStorage, \ from vdirsyncer.storage.dav import CaldavStorage, CarddavStorage
_normalize_href
from .. import StorageTests, format_item from .. import StorageTests, format_item
@ -183,26 +182,3 @@ class TestCarddavStorage(DavStorageTests):
@pytest.fixture @pytest.fixture
def item_template(self, request): def item_template(self, request):
return VCARD_TEMPLATE return VCARD_TEMPLATE
@pytest.mark.parametrize('base,path', [
('http://example.com/', ''),
('http://example.com/L%C3%98/', '/L%C3%98'),
('http://example.com/LØ/', '/L%C3%98'),
])
def test_normalize_href(base, path):
assert _normalize_href(base, 'asdf') == path + '/asdf'
assert _normalize_href(base, 'hahah') == path + '/hahah'
assert _normalize_href(base, 'whoops@vdirsyncer.vcf') == \
path + '/whoops@vdirsyncer.vcf'
assert _normalize_href(base, 'whoops%40vdirsyncer.vcf') == \
path + '/whoops@vdirsyncer.vcf'
assert _normalize_href(base, 'wh%C3%98ops@vdirsyncer.vcf') == \
path + '/wh%C3%98ops@vdirsyncer.vcf'
assert _normalize_href(base, 'whØops@vdirsyncer.vcf') == \
path + '/wh%C3%98ops@vdirsyncer.vcf'

View file

@ -23,21 +23,24 @@ dav_logger = log.get(__name__)
CALDAV_DT_FORMAT = '%Y%m%dT%H%M%SZ' CALDAV_DT_FORMAT = '%Y%m%dT%H%M%SZ'
def _normalize_href(base, href, decoding_rounds=1): def _normalize_href(base, href):
'''Normalize the href to be a path only relative to hostname and '''Normalize the href to be a path only relative to hostname and
schema.''' schema.'''
if not href: if not href:
raise ValueError(href) raise ValueError(href)
x = utils.urlparse.urljoin(base, href) x = utils.urlparse.urljoin(base, href)
x = utils.urlparse.urlsplit(x).path x = utils.urlparse.urlsplit(x).path
for i in range(decoding_rounds):
x = utils.compat.urlunquote(x)
x = utils.compat.urlquote(x, '/@')
return x return x
def _encode_href(x):
return utils.compat.urlquote(x, '/@')
def _decode_href(x):
return utils.compat.urlunquote(x)
class Discover(object): class Discover(object):
_resourcetype = None _resourcetype = None
_homeset_xml = None _homeset_xml = None
@ -305,7 +308,7 @@ class DavStorage(Storage):
for href in hrefs: for href in hrefs:
if href != self._normalize_href(href): if href != self._normalize_href(href):
raise exceptions.NotFoundError(href) raise exceptions.NotFoundError(href)
href_xml.append('<D:href>{}</D:href>'.format(href)) href_xml.append('<D:href>{}</D:href>'.format(_encode_href(href)))
if not href_xml: if not href_xml:
return () return ()
@ -320,8 +323,8 @@ class DavStorage(Storage):
rv = [] rv = []
hrefs_left = set(hrefs) hrefs_left = set(hrefs)
for element in root.iter('{DAV:}response'): for element in root.iter('{DAV:}response'):
href = self._normalize_href( href = self._normalize_href(_decode_href(
element.find('{DAV:}href').text) element.find('{DAV:}href').text))
raw = element \ raw = element \
.find('{DAV:}propstat') \ .find('{DAV:}propstat') \
.find('{DAV:}prop') \ .find('{DAV:}prop') \
@ -359,7 +362,7 @@ class DavStorage(Storage):
response = self.session.request( response = self.session.request(
'PUT', 'PUT',
href, _encode_href(href),
data=item.raw.encode('utf-8'), data=item.raw.encode('utf-8'),
headers=headers headers=headers
) )
@ -389,7 +392,7 @@ class DavStorage(Storage):
self.session.request( self.session.request(
'DELETE', 'DELETE',
href, _encode_href(href),
headers=headers headers=headers
) )
@ -421,7 +424,8 @@ class DavStorage(Storage):
contenttype = prop.find('{DAV:}getcontenttype').text contenttype = prop.find('{DAV:}getcontenttype').text
href = self._normalize_href(element.find('{DAV:}href').text) href = _decode_href(self._normalize_href(
element.find('{DAV:}href').text))
etag = prop.find('{DAV:}getetag').text etag = prop.find('{DAV:}getetag').text
if not etag: if not etag:
raise ValueError('Server did not return an etag for item {}. ' raise ValueError('Server did not return an etag for item {}. '
@ -640,8 +644,8 @@ class CarddavStorage(DavStorage):
# Decode twice because ownCloud encodes twice. # Decode twice because ownCloud encodes twice.
# See https://github.com/owncloud/contacts/issues/581 # See https://github.com/owncloud/contacts/issues/581
href = self._normalize_href(element.find('{DAV:}href').text, href = self._normalize_href(
decoding_rounds=2) _decode_href(_decode_href(element.find('{DAV:}href').text)))
etag = prop.find('{DAV:}getetag').text etag = prop.find('{DAV:}getetag').text
if href in hrefs: if href in hrefs: