Closed
Bug 575782
Opened 14 years ago
Closed 14 years ago
Lightning 1.0 beta 2 does not work with non-ASCII characters in event names
Categories
(Calendar :: Provider: CalDAV, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b3
People
(Reporter: bkuemmer, Assigned: nomisvai)
References
Details
(Whiteboard: [needed beta][no l10n impact][CalDAV server: DavMail])
Attachments
(1 file, 1 obsolete file)
34.06 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.4) Gecko/20100608 Lightning/1.0b2 Thunderbird/3.1 I have updated from Thunderbird 3.0 with Lightning 1.0b1 to Thunderbird 3.1 with Lightning 1.0b2, using a CalDav provider (DavMail 3.6.6-1032 talking to an Exchange 2003 server). Since the update, not all calendar entries are displayed anymore. CalDav logs "HTTP/1.1 404 Not Found" for some calendar entries. The entries all seem to contain Umlaut characters or the € sign. CalDav log: <D:response><D:href>/users/bernd.kuemmerlen@xxx.de/calendar/Osterferien%20(Baden-W%3Frttemberg)-16.EML</D:href><D:propstat><D:status>HTTP/1.1 404 Not Found</D:status></D:propstat></D:response> Lightning log: Warning: CalDAV: Unexpected response, status: undefined, href: /users/bernd.kuemmerlen@xxx.de/calendar/Osterferien (Baden-W?rttemberg)-16.EML calendar-data: There are about 10 Warnings like this in the Lightning log, followed by a (very long) "CalDAC: recv" section, then the following error: Error: Assert failed: unexepcted endBatch! 2: [file:///C:/Users/bkuemmerlen/AppData/Roaming/Thunderbird/Profiles/ba9sao7p.Empty%20Test/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/modules/calProviderUtils.jsm:647] cPB_endBatch 3: [file:///C:/Users/bkuemmerlen/AppData/Roaming/Thunderbird/Profiles/ba9sao7p.Empty%20Test/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/modules/calUtils.jsm -> file:///C:/Users/bkuemmerlen/AppData/Roaming/Thunderbird/Profiles/ba9sao7p.Empty%20Test/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calDavRequestHandlers.js:797] mg_onStopRequest Source File: file:///C:/Users/bkuemmerlen/AppData/Roaming/Thunderbird/Profiles/ba9sao7p.Empty%20Test/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/modules/calUtils.jsm -> file:///C:/Users/bkuemmerlen/AppData/Roaming/Thunderbird/Profiles/ba9sao7p.Empty%20Test/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calUtils.js Line: 975 Warning: CalDAV: Get failed: multiget error Warning: There has been an error reading data for calendar: MMS. However, this error is believed to be minor, so the program will attempt to continue. Error code: 0x80004005. Description: multiget error Warning: There has been an error reading data for calendar: MMS. However, this error is believed to be minor, so the program will attempt to continue. Error code: READ_FAILED. Description: After this, the calendar displays a few entries, but a lot of entries are not displayed (as confirmed through the Outlook Web Interface). Next to the Calendar the icon for "The calendar XXX is momentarily not available" is displayed. Reproducible: Always Steps to Reproduce: 1. Create a CalDav calendar connection to a local DavMail server 2. Open Calendar Actual Results: The Calendar does not display all events on the server, only a subset. Warnings and errors are displayed in the Error Console. Expected Results: All events should be displayed in the Calendar. This has been tested with a completely new clean profile. It is 100% reproducible (i.e. I can not get it to work) with Lightning 1.0 beta2. Using Lightning 1.0 beta 1 on Thunderbird 3.1, all Calendar entries load fine, but after restarting Thunderbird I get a message that the Calendar could not be loaded, and I have to delete and recreate the CalDav calendar.
Reporter | ||
Comment 1•14 years ago
|
||
Changing a few of the calendar entries to not contain Umlauts anymore removes them from the Warning log in Lightning, but others appear instead.
Reporter | ||
Comment 2•14 years ago
|
||
The same issue can be reproduced in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.4) Gecko/20100610 Sunbird/1.0b2
Assignee | ||
Comment 3•14 years ago
|
||
Is it possible to have a test account on a DavMail server so I can reproduce the issue? You can email me the credentials, thanks.
Assignee: nobody → simon.at.orcl
Reporter | ||
Comment 4•14 years ago
|
||
Sorry, since this is an Exchange 2003 server over which I have no control, I can not provide a test account.
Assignee | ||
Comment 5•14 years ago
|
||
In that case I think the full logs would be useful: 1) Go in tools->options->advanced (General tab) and "Config Editor..." 2) Add or set these preference names to "true": calendar.debug.log calendar.debug.log.verbose 3) Install the Console2 addon(To copy all the logs at step 5) 4) Reproduce the issue 5) Tools->Error console->Right click->Select All, Right click->Copy 6) Either mail the logs to me or add them as attachment to this bug (if your meetings contain no sensitive info) You can wait a week or so before doing that as I think there are other similar bugs I have to look at that might also resolve this issue. Thanks.
Updated•14 years ago
|
Whiteboard: [CalDAV server: DavMail]
Version: unspecified → Lightning 1.0b2
Reporter | ||
Comment 6•14 years ago
|
||
Logs have been sent by private e-mail
Assignee | ||
Updated•14 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Reporter | ||
Updated•14 years ago
|
Summary: Lightning 1.0 beta 2 does not display all calendar entries → Lightning 1.0 beta 2 does not work with non-ASCII characters in event names
Updated•14 years ago
|
Flags: blocking-calendar1.0?
Assignee | ||
Comment 8•14 years ago
|
||
Make sure the path is encoded when building the multiget request. I could not test this on a DavMail server since I don't have a test account, so please test on as many different servers as you can.
Attachment #456281 -
Flags: review?(philipp)
Updated•14 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Reporter | ||
Comment 9•14 years ago
|
||
I have tested this patch against Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.4) Gecko/20100610 Sunbird/1.0b2 and it does indeed fix the issue for me! Thanks a lot!
Reporter | ||
Comment 10•14 years ago
|
||
Now also tested successfully in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.4) Gecko/20100608 Lightning/1.0b2 Thunderbird/3.1
Comment 11•14 years ago
|
||
This seems like a quite critical bug, we should consider doing a chemspill release for this. I've seen quite a few caldav regressions after the release, we should get as many of them fixed as possible.
Severity: major → critical
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Whiteboard: [CalDAV server: DavMail] → [needed beta][CalDAV server: DavMail]
Updated•14 years ago
|
Whiteboard: [needed beta][CalDAV server: DavMail] → [needed chemspill][CalDAV server: DavMail]
Comment 12•14 years ago
|
||
Comment on attachment 456281 [details] [diff] [review] Encode path before sending multiget request >- let locpath = this.itemsNeedFetching.pop(); >+ // Rebuild the path to make sure it is encoded properly >+ let locpath = this.calendar.ensurePath(this.itemsNeedFetching.pop()); >+ locpath = makeURL(this.baseUri.prePath + locpath).path; > queryXml.D::prop += <D:href xmlns:D={D}>{locpath}</D:href>; Can it be the case that whatever is in this.itemsNeedFetching.pop() is already a full uri? (i.e the server advertises full uris rather than relative) In this case locpath will probably be http://.../http://..../... r=philipp if I am mistaking, but I really think we should be more robust here.
Attachment #456281 -
Flags: review?(philipp) → review+
Comment 13•14 years ago
|
||
This patch made events with non-ascii characters downloaded. However, there is a similar problem, when dismissing such event. E.g. PUT /users/martin.balint@xxx.net/calendar/test%20+%C4%9B%C5%A1%C4%8D.EML but event name is actually "test +ěšč", so correctly urlencoding is PUT /users/martin.balint@xxx.net/calendar/test%20%2B%C4%9B%C5%A1%C4%8D.EML
Comment 14•14 years ago
|
||
If characters + and # are present in event name, such event cannot be dismissed. In DavMail logs, I can see 404.
Updated•14 years ago
|
Whiteboard: [needed chemspill][CalDAV server: DavMail] → [needed beta][CalDAV server: DavMail]
Updated•14 years ago
|
Whiteboard: [needed beta][CalDAV server: DavMail] → [needed beta][no l10n impact][CalDAV server: DavMail]
Assignee | ||
Comment 15•14 years ago
|
||
@Philipp: whether the server returns a full uri or not does not matter because we call ensurePath() on the uri so we "extract" only the path information from it and we then rebuild it to make sure it is properly encoded. @Martin: The encoding is done by nsIURI which follows the rules of rfc2396/3986(Section 2.2), even if + and # are described as "reserved" characters they do not need to be encoded since they are not delimiters in the context described. I believe this specific case is a server bug: The server should unescape this request path properly before matching it with a server resource. In other words, it shouldn't matter if the client percent encodes + or # in the requests, the server should be able to match it anyways.
Comment 16•14 years ago
|
||
(In reply to comment #15) > @Philipp: whether the server returns a full uri or not does not matter because > we call ensurePath() on the uri so we "extract" only the path information from > it and we then rebuild it to make sure it is properly encoded. Yes, I've taken another look at the code and ensurePath is called as you've mentioned. > > @Martin: The encoding is done by nsIURI which follows the rules of > rfc2396/3986(Section 2.2), even if + and # are described as "reserved" I've talked with Simon, and we've come to the conclusion this is a server bug. As Simon mentioned, # and + are not delimiter characters in the caldav context, so they don't need to be escaped. The workaround is easy for now, so please file a bug with davmail, feel free to reference this bug.
Assignee | ||
Comment 17•14 years ago
|
||
Actually after re-reading the RFCs with second set of eyes(Thanks Bern!) I've come to the conclusion that we should encode the # and + since their value is not equivalent to their % encoding.. and also it is stated elsewhere that their values must be encoded IF they have no delimiter role in the URL, I will look at the available mozilla functions and might end up writing my own. 3.2.3 URI Comparison http://tools.ietf.org/html/rfc2616#section-3.2.3 Characters other than those in the "reserved" and "unsafe" sets (see RFC 2396 [42]) are equivalent to their ""%" HEX HEX" encoding. http://tools.ietf.org/html/rfc3986 reserved = gen-delims / sub-delims gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@" sub-delims = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="
Assignee | ||
Comment 18•14 years ago
|
||
This patch makes the caldav provider encode all "unsafe" characters. I've also changed the webdav sync token name property in order to trigger a refresh so that paths that are not encoded differently will get picked up correctly. This change affects pretty much all requests done by the caldav provider so please test on as many servers as you can.
Attachment #456281 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Attachment #492906 -
Flags: review?(philipp)
Comment 19•14 years ago
|
||
Comment on attachment 492906 [details] [diff] [review] patch v2 > > makeUri: function caldav_makeUri(aInsertString, aBaseUri) { > let baseUri = aBaseUri || this.calendarUri; >- let spec = baseUri.spec + (aInsertString || ""); >- if (this.mUriParams) { >- spec += this.mUriParams; >- } >- return makeURL(spec); >+ let decodedSpec = baseUri.prePath + this.ensureDecodedPath(baseUri.path) + (aInsertString || ""); >+ let url = cal.makeURL(baseUri.prePath + this.ensureEncodedPath(decodedSpec) + (this.mUriParams || "")); I don't quite understand: the decodedSpec contains baseUri.prePath, then the url again prepends baseUri.prePath? Doesn't this cause a double prePath ? >+ cal.LOG("CalDAV: makeUri: aInsertString=" + aInsertString + ", baseUri=" + (baseUri ? baseUri.spec : "undefined")+", final url=" + url.spec); We might want to get rid of this before checkin, this might happen potentially very often. Or at least move it to verbose caldav logging. >+ cal.WARN("Spec could not be parsed, returning as-is: "+ aSpec); This should be prefixed with CalDAV: >+ var uriComponents = aString.split("/"); >+ for (var i = 0 ; i < uriComponents.length ; i++ ) { >+ uriComponents[i] = encodeURIComponent(uriComponents[i]); >+ } uriComponents = uriComponents.map(encodeURIComponent); or even: return uriComponents.map(encodeURIComponent).join("/"); I've tested this with sogo and it seems to work. I unfortunately don't have other servers at hand. r=philipp with comments considered.
Attachment #492906 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 20•14 years ago
|
||
pushed to comm-central: http://hg.mozilla.org/comm-central/rev/42371756f234
Assignee | ||
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Target Milestone: --- → Trunk
Assignee | ||
Comment 21•14 years ago
|
||
backported to comm-1.9.2: http://hg.mozilla.org/releases/comm-1.9.2/rev/678d78f024ca
Updated•14 years ago
|
Target Milestone: Trunk → 1.0b3
You need to log in
before you can comment on or make changes to this bug.
Description
•