vobject patch inconsistent #327

Closed
macosforgebot opened this Issue Feb 21, 2011 · 9 comments

Projects

None yet

3 participants

@macosforgebot

mail@… originally submitted this as ticket:419


There is a patch to vobject at the moment.

source://CalendarServer/trunk/lib-patches/vobject/vobject.icalendar.patch

It claims to just add the : but at the same time it removes " too.

By reading RFC2445 I am not sure what is supposed to happen.

But Lightning escapes the " as \" and therefore the server gives:

2011-02-21 17:19:58+0100 [-] [caldav-0]  [PooledMemCacheProtocol,client] [twistedcaldav.memcachepool.MemCachePool#debug] Clients #free: 5, #busy: 0, #pending: 0, #queued: 0
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-] [twistedcaldav.method.put_common#error] u'error: illegal escape sequence: \'\\"\''
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-] Traceback (most recent call last):
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twisted/internet/defer.py", line 441, in _runCallbacks
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     self.result = callback(self.result, *args, **kw)
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twisted/internet/defer.py", line 949, in gotResult
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     _inlineCallbacks(r, g, deferred)
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twisted/internet/defer.py", line 891, in _inlineCallbacks
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     result = result.throwExceptionIntoGenerator(g)
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twisted/python/failure.py", line 338, in throwExceptionIntoGenerator
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     return g.throw(self.type, self.value, self.tb)
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-] --- <exception caught here> ---
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twistedcaldav/method/put_common.py", line 937, in run
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     yield self.fullValidation()
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twisted/internet/defer.py", line 893, in _inlineCallbacks
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     result = g.send(result)
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]   File "/usr/lib/python2.6/site-packages/twistedcaldav/method/put_common.py", line 272, in fullValidation
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-]     description="Can't parse calendar data"
2011-02-21 17:19:58+0100 [-] [caldav-0]  [-] twext.web2.http.HTTPError: <ErrorResponse 403 {urn:ietf:params:xml:ns:caldav}valid-calendar-data>
@macosforgebot

mail@… originally submitted this as comment:1:⁠ticket:419


As lightning is based on libical I had a look at that.

It seems like they allow \".

Have a look at https://freeassociation.svn.sourceforge.net/svnroot/freeassociation/trunk/libical/src/test/regression.c line 3383.

@macosforgebot

mail@… originally submitted this as comment:2:⁠ticket:419


Looking at the DAViCal-Server implementation it seems like they accept \" too.

http://repo.or.cz/w/awl.git/blob/HEAD:/inc/iCalendar.php#l126

@macosforgebot

mail@… originally submitted this as comment:3:⁠ticket:419


On IRC I heard from dre that\ Cyrus\ is\ of\ the\ opinion,\ this\ is\ not\ a\ bug\ in\ the\ calendar\ server.

I have opened a lightning bug to see https://bugzilla.mozilla.org/show_bug.cgi?id=635758 .

@macosforgebot

@cyrusdaboo originally submitted this as comment:4:⁠ticket:419


To be clear: iCalendar explicitly lists the set of escape characters that it defines:

text = *(TSAFE-CHAR / ":" / DQUOTE / ESCAPED-CHAR)

; Folded according to description above

ESCAPED-CHAR = ("
" / "\;" / "\," / "\N" / "\n")

;
encodes \, \N or \n encodes newline ; \; encodes ;, \, encodes ,

It does not say any arbitrary character can be escaped, nor does it say they must not be escaped. What this really means is that for the best interoperability, clients must only escape the set of characters defined in the iCalendar syntax. Anything else can legitimately be treated as wrong.

Note I am not totally opposed to accepting \", but if we do the server must convert that into a plain " character and return that to clients, because there could be clients out there that (legitimately) reject iCalendar data with \" so the server must only return valid data to all clients. That said, I do believe the client in this case needs to be fixed. Arguably the spec also needs to be clearer on this point.

@macosforgebot

mail@… originally submitted this as comment:5:⁠ticket:419


How relevant is the vobject patch after the merge r7207 ?

@macosforgebot

mail@… originally submitted this as comment:6:⁠ticket:419


just as people reading here will be interested in it, to patch current calendar server one needs the following:

root@thinkpad:/home/fm/rpmbuild # cat SOURCES/pycalendar-escape.patch 
Index: src/pycalendar/utils.py
===================================================================
--- src/pycalendar/utils.py     (Revision 144)
+++ src/pycalendar/utils.py     (Arbeitskopie)
@@ -170,7 +170,7 @@
     return os.getvalue()

 # Use these to control the parsing of illegal escape characters
-decodeTextValue.raiseOnInvalidEscape = True
+decodeTextValue.raiseOnInvalidEscape = False
 decodeTextValue.fixInvalidEscape = True

 # Date/time calcs
@macosforgebot

@cyrusdaboo originally submitted this as comment:7:⁠ticket:419


vobject is still being used for vcard data so the patch is still relevant.

As far as the options to change behavior in pycalendar - the current state is that there are some module level attributes that should be set by the application using the library to determine the behavior the app wants. We are still in the process of fine tuning that and will likely not have it finalized until after the vcard parsing is re-worked.

@macosforgebot

mail@… originally submitted this as comment:8:⁠ticket:419


since r7242 the patch from comment:6 is not needed anymore

@macosforgebot

@wsanchez originally submitted this as comment:9:⁠ticket:419

  • Status changed from new to closed
  • Priority changed from 5: Not set to 3: Important
  • Milestone set to CalendarServer-3.0
  • Resolution changed from to Software changed
@macosforgebot macosforgebot added this to the CalendarServer-3.0 milestone Aug 4, 2016
@wsanchez wsanchez was unassigned by tomerd Aug 4, 2016
@mocomoc mocomoc assigned wsanchez and cyrusdaboo and unassigned wsanchez and cyrusdaboo Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment