diff --git a/radicale/__init__.py b/radicale/__init__.py index 5d3def1..ae53c89 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -788,6 +788,8 @@ class Application: try: items = list(vobject.readComponents(content or "")) + for item in items: + storage.check_item(item) except Exception as e: self.logger.warning( "Bad PUT request on %r: %s", path, e, exc_info=True) @@ -797,13 +799,23 @@ class Application: tag = tags.get(content_type) if write_whole_collection: - new_item = self.Collection.create_collection( - path, items, {"tag": tag}) + try: + new_item = self.Collection.create_collection( + path, items, {"tag": tag}) + except ValueError as e: + self.logger.warning( + "Bad PUT request on %r: %s", path, e, exc_info=True) + return BAD_REQUEST else: if tag: parent_item.set_meta({"tag": tag}) href = posixpath.basename(path.strip("/")) - new_item = parent_item.upload(href, items[0]) + try: + new_item = parent_item.upload(href, items[0]) + except ValueError as e: + self.logger.warning( + "Bad PUT request on %r: %s", path, e, exc_info=True) + return BAD_REQUEST headers = {"ETag": new_item.etag} return client.CREATED, headers, None diff --git a/radicale/storage.py b/radicale/storage.py index 3c3258a..711e7fc 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -27,7 +27,6 @@ entry. import binascii import contextlib -import errno import json import os import pickle @@ -115,6 +114,20 @@ def load(configuration, logger): return CollectionCopy +def check_item(vobject_item): + """Check vobject items for common errors.""" + if vobject_item.name == "VCALENDAR": + for component in vobject_item.components(): + if (component.name in ("VTODO", "VEVENT", "VJOURNAL") and + not get_uid(component)): + raise ValueError("UID in %s is missing" % component.name) + elif vobject_item.name == "VCARD": + if not get_uid(vobject_item): + raise ValueError("UID in VCARD is missing") + else: + raise ValueError("Unknown item type: %r" % vobject_item.name) + + def scandir(path, only_dirs=False, only_files=False): """Iterator for directory elements. (For compatibility with Python < 3.5) @@ -149,7 +162,7 @@ def get_etag(text): def get_uid(item): """UID value of an item if defined.""" - return hasattr(item, "uid") and item.uid.value + return (hasattr(item, "uid") or None) and item.uid.value def sanitize_path(path): @@ -541,13 +554,14 @@ class Collection(BaseCollection): self._sync_directory(directory) @staticmethod - def _find_available_file_name(exists_fn): + def _find_available_file_name(exists_fn, suffix=""): # Prevent infinite loop - for _ in range(10000): - file_name = hex(getrandbits(32))[2:] + for _ in range(1000): + file_name = "%016x" % getrandbits(64) + suffix if not exists_fn(file_name): return file_name - raise FileExistsError(errno.EEXIST, "No usable file name found") + # something is wrong with the PRNG + raise RuntimeError("No unique random sequence found") @classmethod def _fsync(cls, fd): @@ -690,15 +704,19 @@ class Collection(BaseCollection): new_collection = vobject.iCalendar() for item in items: new_collection.add(item) + # href must comply to is_safe_filesystem_path_component + # and no file name collisions must exist between hrefs href = self._find_available_file_name( - vobject_items.get) + vobject_items.get, suffix=".ics") vobject_items[href] = new_collection self.upload_all_nonatomic(vobject_items) elif props.get("tag") == "VCARD": vobject_items = {} for card in collection: + # href must comply to is_safe_filesystem_path_component + # and no file name collisions must exist between hrefs href = self._find_available_file_name( - vobject_items.get) + vobject_items.get, suffix=".vcf") vobject_items[href] = card self.upload_all_nonatomic(vobject_items) @@ -982,8 +1000,13 @@ class Collection(BaseCollection): cinput_hash = cetag = ctext = ctag = cstart = cend = None vobject_item = None if input_hash != cinput_hash: - vobject_item = Item(self, href=href, - text=btext.decode(self.encoding)).item + try: + vobject_item = Item(self, href=href, + text=btext.decode(self.encoding)).item + check_item(vobject_item) + except Exception as e: + raise RuntimeError("Failed to parse item %r from %r: %s" % + (href, self.path, e)) from e # Serialize the object again, to normalize the text representation. # The storage may have been edited externally. ctext = vobject_item.serialize() diff --git a/radicale/tests/static/todo2.ics b/radicale/tests/static/todo2.ics index 0642ef6..32274f7 100644 --- a/radicale/tests/static/todo2.ics +++ b/radicale/tests/static/todo2.ics @@ -23,5 +23,6 @@ BEGIN:VTODO DTSTART;TZID=Europe/Paris:20130901T180000 DUE;TZID=Europe/Paris:20130903T180000 RRULE:FREQ=MONTHLY +UID:todo2 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo3.ics b/radicale/tests/static/todo3.ics index fb29bc1..f9252fd 100644 --- a/radicale/tests/static/todo3.ics +++ b/radicale/tests/static/todo3.ics @@ -21,5 +21,6 @@ END:STANDARD END:VTIMEZONE BEGIN:VTODO DTSTART;TZID=Europe/Paris:20130901T180000 +UID:todo3 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo4.ics b/radicale/tests/static/todo4.ics index eeecb96..1c651dc 100644 --- a/radicale/tests/static/todo4.ics +++ b/radicale/tests/static/todo4.ics @@ -21,5 +21,6 @@ END:STANDARD END:VTIMEZONE BEGIN:VTODO DUE;TZID=Europe/Paris:20130901T180000 +UID:todo4 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo5.ics b/radicale/tests/static/todo5.ics index ae6a629..29c307f 100644 --- a/radicale/tests/static/todo5.ics +++ b/radicale/tests/static/todo5.ics @@ -22,5 +22,6 @@ END:VTIMEZONE BEGIN:VTODO CREATED;TZID=Europe/Paris:20130903T180000 COMPLETED;TZID=Europe/Paris:20130920T180000 +UID:todo5 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo6.ics b/radicale/tests/static/todo6.ics index db9b4b5..805b4cf 100644 --- a/radicale/tests/static/todo6.ics +++ b/radicale/tests/static/todo6.ics @@ -21,5 +21,6 @@ END:STANDARD END:VTIMEZONE BEGIN:VTODO COMPLETED;TZID=Europe/Paris:20130920T180000 +UID:todo6 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo7.ics b/radicale/tests/static/todo7.ics index 1d44c3a..f94b271 100644 --- a/radicale/tests/static/todo7.ics +++ b/radicale/tests/static/todo7.ics @@ -21,5 +21,6 @@ END:STANDARD END:VTIMEZONE BEGIN:VTODO CREATED;TZID=Europe/Paris:20130803T180000 +UID:todo7 END:VTODO END:VCALENDAR diff --git a/radicale/tests/static/todo8.ics b/radicale/tests/static/todo8.ics index 8005238..27d4962 100644 --- a/radicale/tests/static/todo8.ics +++ b/radicale/tests/static/todo8.ics @@ -20,6 +20,6 @@ RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10 END:STANDARD END:VTIMEZONE BEGIN:VTODO - +UID:todo8 END:VTODO END:VCALENDAR diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index 6591ea8..45e9e01 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -133,13 +133,13 @@ class BaseRequestsMixIn: status, headers, answer = self.request("PUT", "/calendar.ics/", event) assert status == 201 status, headers, answer = self.request( - "PUT", "/calendar.ics/event1.ics", event) + "PUT", "/calendar.ics/test_event.ics", event) assert status == 201 # Overwrite status, headers, answer = self.request("PUT", "/calendar.ics/", event) assert status == 201 status, headers, answer = self.request( - "GET", "/calendar.ics/event1.ics") + "GET", "/calendar.ics/test_event.ics") assert status == 404 def test_delete(self): @@ -263,8 +263,9 @@ class BaseRequestsMixIn: filters_text = "".join( "%s" % filter_ for filter_ in filters) self.request("MKCOL", "/calendar.ics/") - self.request( + status, _, _ = self.request( "PUT", "/calendar.ics/", "BEGIN:VCALENDAR\r\nEND:VCALENDAR") + assert status == 201 for i in range(items): filename = "{}{}.ics".format(kind, i + 1) event = get_file_content(filename)