diff --git a/tests/__init__.py b/tests/__init__.py index 4aaa0ef..709e373 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -3,6 +3,8 @@ Test suite for vdirsyncer. ''' +import hypothesis.strategies as st + from vdirsyncer.utils.vobject import normalize_item @@ -66,3 +68,7 @@ UID:{uid} X-SOMETHING:{r} HAHA:YES END:FOO''' + +printable_characters_strategy = st.text( + st.characters(blacklist_categories=('Cc',)) +) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index e0e03a7..6385cf9 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -2,14 +2,17 @@ import random +from hypothesis import given +import hypothesis.strategies as st + import pytest import vdirsyncer.exceptions as exceptions -from vdirsyncer.storage.base import Item +from vdirsyncer.storage.base import Item, normalize_meta_value from vdirsyncer.utils.compat import iteritems, text_type, urlquote, urlunquote from .. import EVENT_TEMPLATE, TASK_TEMPLATE, VCARD_TEMPLATE, \ - assert_item_equals + assert_item_equals, printable_characters_strategy def get_server_mixin(server_name): @@ -269,12 +272,12 @@ class StorageTests(object): pytest.skip('ownCloud is fundamentally broken.') if not getattr(self, 'dav_server', ''): - assert s.get_meta('color') is None - assert s.get_meta('displayname') is None + assert not s.get_meta('color') + assert not s.get_meta('displayname') try: s.set_meta('color', None) - assert s.get_meta('color') is None + assert not s.get_meta('color') s.set_meta('color', u'#ff0000') assert s.get_meta('color') == u'#ff0000' except exceptions.UnsupportedMetadataError: @@ -285,3 +288,17 @@ class StorageTests(object): rv = s.get_meta('displayname') assert rv == x assert isinstance(rv, text_type) + + @given(value=st.one_of( + st.none(), + printable_characters_strategy.filter(lambda x: x.strip() != x) + )) + def test_metadata_normalization(self, requires_metadata, s, value): + x = s.get_meta('displayname') + assert x == normalize_meta_value(x) + + if not getattr(self, 'dav_server', None): + # ownCloud replaces "" with "unnamed" + # Also https://github.com/owncloud/core/issues/18409 + s.set_meta('displayname', value) + assert s.get_meta('displayname') == normalize_meta_value(value) diff --git a/tests/test_metasync.py b/tests/test_metasync.py index 5b86972..7484eb9 100644 --- a/tests/test_metasync.py +++ b/tests/test_metasync.py @@ -1,8 +1,5 @@ # -*- coding: utf-8 -*- -from hypothesis import given -import hypothesis.strategies as st - import pytest from vdirsyncer.metasync import MetaSyncConflict, metasync @@ -42,7 +39,7 @@ def test_basic(monkeypatch): b.set_meta('foo', None) metasync(a, b, status, keys=['foo']) - assert a.get_meta('foo') is b.get_meta('foo') is None + assert not a.get_meta('foo') and not b.get_meta('foo') def test_conflict(): @@ -85,17 +82,3 @@ def test_conflict_x_wins(wins): assert a.get_meta('foo') == b.get_meta('foo') == status['foo'] == ( 'bar' if wins == 'a' else 'baz' ) - - -@given(s=st.text().filter(lambda x: x.strip() == x), key=st.text()) -def test_trailing_newline(s, key): - a = MemoryStorage() - b = MemoryStorage() - status = {} - a.set_meta(key, s + u'\n') - b.set_meta(key, s) - a.set_meta = b.set_meta = blow_up - - metasync(a, b, status, keys=[key]) - - assert status[key] == s diff --git a/vdirsyncer/metasync.py b/vdirsyncer/metasync.py index dbf1575..df53870 100644 --- a/vdirsyncer/metasync.py +++ b/vdirsyncer/metasync.py @@ -1,5 +1,5 @@ from . import exceptions, log -from .utils.compat import text_type +from .storage.base import normalize_meta_value logger = log.get(__name__) @@ -34,9 +34,9 @@ def metasync(storage_a, storage_b, status, keys, conflict_resolution=None): _b_to_a() for key in keys: - a = _normalize_value(storage_a.get_meta(key)) - b = _normalize_value(storage_b.get_meta(key)) - s = status.get(key) + a = storage_a.get_meta(key) + b = storage_b.get_meta(key) + s = normalize_meta_value(status.get(key)) logger.debug(u'Key: {}'.format(key)) logger.debug(u'A: {}'.format(a)) logger.debug(u'B: {}'.format(b)) @@ -53,11 +53,3 @@ def metasync(storage_a, storage_b, status, keys, conflict_resolution=None): for key in set(status) - set(keys): del status[key] - - -def _normalize_value(value): - if value is None: - return value - else: - assert isinstance(value, (bytes, text_type)) - return value.strip() diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index b29ab48..ee8c890 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -5,7 +5,7 @@ import functools from .. import exceptions, sync from ..utils import uniq -from ..utils.compat import to_native, with_metaclass +from ..utils.compat import to_native, to_unicode, with_metaclass from ..utils.vobject import Item # noqa @@ -238,3 +238,7 @@ class Storage(with_metaclass(StorageMeta)): ''' raise NotImplementedError('This storage does not support metadata.') + + +def normalize_meta_value(value): + return to_unicode(value or u'').strip() diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index b37c560..ddd3e43 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -7,11 +7,11 @@ from lxml import etree import requests from requests.exceptions import HTTPError -from .base import Item, Storage +from .base import Item, Storage, normalize_meta_value from .http import HTTP_STORAGE_PARAMETERS, USERAGENT, prepare_auth, \ prepare_client_cert, prepare_verify from .. import exceptions, log, utils -from ..utils.compat import text_type, to_native, to_unicode +from ..utils.compat import text_type, to_native dav_logger = log.get(__name__) @@ -597,9 +597,10 @@ class DavStorage(Storage): root = _parse_xml(response.content) for prop in root.findall('.//' + lxml_selector): - text = getattr(prop, 'text', None) + text = normalize_meta_value(getattr(prop, 'text', None)) if text: - return to_unicode(text) + return text + return u'' def set_meta(self, key, value): try: @@ -609,7 +610,7 @@ class DavStorage(Storage): lxml_selector = '{%s}%s' % (namespace, tagname) element = etree.Element(lxml_selector) - element.text = value + element.text = normalize_meta_value(value) data = ''' diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 19b985a..edbc3cf 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -6,7 +6,7 @@ import subprocess from atomicwrites import atomic_write -from .base import Item, Storage +from .base import Item, Storage, normalize_meta_value from .. import exceptions, log from ..utils import checkdir, expand_path, generate_href, get_etag_from_file, \ get_etag_from_fileobject @@ -183,16 +183,16 @@ class FilesystemStorage(Storage): fpath = os.path.join(self.path, key) try: with open(fpath, 'rb') as f: - return f.read().decode(self.encoding) or None + return normalize_meta_value(f.read().decode(self.encoding)) except IOError as e: if e.errno == errno.ENOENT: - return None + return u'' else: raise def set_meta(self, key, value): - value = value or u'' - assert isinstance(value, text_type) + value = normalize_meta_value(value) + fpath = os.path.join(self.path, key) with atomic_write(fpath, mode='wb', overwrite=True) as f: f.write(value.encode(self.encoding)) diff --git a/vdirsyncer/storage/memory.py b/vdirsyncer/storage/memory.py index 6a01c00..3232d22 100644 --- a/vdirsyncer/storage/memory.py +++ b/vdirsyncer/storage/memory.py @@ -2,8 +2,9 @@ import random -import vdirsyncer.exceptions as exceptions -from vdirsyncer.storage.base import Storage +from .base import Storage, normalize_meta_value + +from .. import exceptions def _random_string(): @@ -66,7 +67,7 @@ class MemoryStorage(Storage): del self.items[href] def get_meta(self, key): - return self.metadata.get(key) + return normalize_meta_value(self.metadata.get(key)) def set_meta(self, key, value): - self.metadata[key] = value + self.metadata[key] = normalize_meta_value(value) diff --git a/vdirsyncer/storage/remotestorage.py b/vdirsyncer/storage/remotestorage.py index 7c60967..2e09fde 100644 --- a/vdirsyncer/storage/remotestorage.py +++ b/vdirsyncer/storage/remotestorage.py @@ -19,7 +19,7 @@ We also use a custom ``data``-URI for the redirect in OAuth: import click -from .base import Item, Storage +from .base import Item, Storage, normalize_meta_value from .http import HTTP_STORAGE_PARAMETERS, prepare_client_cert, \ prepare_verify from .. import exceptions, log, utils @@ -225,15 +225,15 @@ class RemoteStorage(Storage): def get_meta(self, key): try: - return self.session.request('GET', key).text or None + return normalize_meta_value(self.session.request('GET', key).text) except exceptions.NotFoundError: - pass + return u'' def set_meta(self, key, value): self.session.request( 'PUT', key, - data=(value or u'').encode('utf-8'), + data=normalize_meta_value(value).encode('utf-8'), headers={'Content-Type': 'text/plain; charset=utf-8'} )