337712 - Multiple CalDAV violations and misbehaviour of Lightning and Sunbird - both latest versions

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Jakub Klos]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
Build Identifier: 0.3a2

1. Improper handling of the URL:

- Remote calendar is set to url: http://server/webdav
- If the CalDAV server returns in the REPORT response href of item as http://server/webdav/folder/itemid the CalDAV client has to repeat the same folder and itemid in every action on this item (DELETE, PUT). Currently it uses only the itemid. Also the DELETE always appends ".ics" although the server did not return it in its response

Both reproducable in Lightning and Sunbird

2. Lightning (latest version March) does not display tasks (TODO)

- The server sends them but Lightning fails in calDAVCalendar.js in the icalString assignment:

                debug("item result = \n" + calData + "\n");
                // XXX try-catch
                item.icalString = calData;
                item.calendar = thisCalendar;

Always reproducable. Events work fine. Error returned:

Error: [Exception... "'Illegal value' when calling method: [calIEvent::icalString]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: file:///C:/Documents%20and%20Settings/Jakub/Application%20Data/Thunderbird/Profiles/3ikp5zav.default/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calDavCalendar.js :: anonymous :: line 554"  data: no]
Source File: file:///C:/Documents%20and%20Settings/Jakub/Application%20Data/Thunderbird/Profiles/3ikp5zav.default/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calDavCalendar.js
Line: 554

The VTODO has correct format and is identical to the one Lightning uses when PUTting the item to the server.

3. No items (Events and Todos) displayed in Sunbird 0.3a2

Items are properly sent from the CalDAV server but Sunbird does not display them. Previous version 0.3a1 worked fine. I'm aware of the calendar-data CalDAV draft change and the CalDAV server supports both formats (the old and the new one). It is not in the format. Notice, Lightning is working also using the latest CalDAV draft.

The console reports these 2 problems:

Error: [Exception... "'Component does not have requested interface' when calling method: [nsIInterfaceRequestor::getInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "<unknown>"  data: no]

Security Error: Content at moz-nullprincipal:{fe8ea01a-023f-470a-bae2-b50af3416f62} may not load or link to about:blank.

Always reproducable.

Reproducible: Always




I am sorry for submitting all of them as 1 bug. I feel they are all related

Comment 1

[Joey Minta]

Item 2 can be fixed by Bug 322932.  Item 3 is bug 337790.  Can you perhaps tidy up this bug report for item 1 so that it can be properly dealt with?

Comment 2

[Jakub Klos]

Ok, tested the latest nightly build and these issues I still found and all of them can be fixed in calDavCalendar.js

- DELETE - does not issue the href id the server returned - check the code
In the deleteitem function there is:

         eventUri.spec = eventUri.spec + aItem.id + ".ics";

Replace it with:

        try {
            eventUri.spec = this.mCalendarUri.spec + aItem.getProperty("X-MOZ-LOCATIONPATH");
            LOG("using X-MOZ-LOCATIONPATH: " + eventUri.spec);
        } catch (ex) {
            // XXX how are we REALLY supposed to figure this out?
            eventUri.spec = eventUri.spec + aItem.id + ".ics";
        }

It will start working.

- PUT - Location: should be checked and itemid updated as in groupdav
This is more of an issue. The CalDAV client should consider that the CalDAV server might return the Location: header which contains the new href id of the item added (not all caldav servers can accept the id created by clients and change them). This behavior is suggested in the GroupDAV draft. So for the listener of the put item add the Location: header check and update the X-MOZ-LOCATIONPATH. That should do it.

Let me know if this helps and I'll test the changes right away.
Thank you

Comment 3

[Matthew (lilmatt) Willis]

Attachment: Patch implementing Jakub Klos' fix

Implements the fix for DELETE in comment 2.

Comment 4

[Matthew (lilmatt) Willis]

(In reply to comment #2)
> - PUT - Location: should be checked and itemid updated as in groupdav...

Jakub,
Are you referring to this?
http://lxr.mozilla.org/mozilla/source/calendar/providers/caldav/calDavCalendar.js#217

I want to make sure before attempting to fix this as I can't really replicate it at the moment using Apple's CalDAV server.

Comment 5

[Jakub Klos]

Hi Matthew
Yes I am. The onPutComplete listener should somehow read the
header "Location: " of the response from the CalDAV server. If it is there (the server sent it) it should set the content of the header to the property

aItem.setProperty("X-MOZ-LOCATIONPATH", locationheadercontent);

If I knew how the listener parameters worked I could have fixed it myself and told you but I don't :).

The behavior is explained here:

http://www.groupdav.org/draft-hess-groupdav-01.html

Section "5.4. Creating Items" - see the response from the server.

Thank you
Jakub

Comment 6

[Matthew (lilmatt) Willis]

(In reply to comment #5)
> Yes I am. The onPutComplete listener should somehow read the
> header "Location: " of the response from the CalDAV server. If it is there (the
> server sent it) it should set the content of the header to the property

Unless I'm misreading the spec (which is entirely possible), it looks like this behaviour was removed from the CalDAV spec:
   C.10.  Changes in -06
      c.  Removed ability for a server to return the Location header on a
          successful PUT request.

http://ietf.osafoundation.org/caldav/draft-dusseault-caldav.txt

Am I misreading something?

Comment 7

[Jakub Klos]

You are right. But what is the reason for them to remove it? Not all caldav servers allow you to specify your own href... What is the proper way for the server to return the href then? Any ideas?
Or is it strictly required for the sever to accept any href for the PUT command?
Thank you

Comment 8

[Bruno Browning]

I think we're going to need to query after PUT using item.id to find the href of the item just added. We'll probably need to be doing that query anyway in order to make sure we have the correct etag for the item. And doing so will let us deal better with two issues that Jakub points out: items that are not located in the 'root' of the calendar URI, and items whose filename is not item.id + .ics

I'd question as well the usefulness of LOCATIONPATH as an X-prop. It seems to me that, in a scenario such Jakub describes, a non-Mozilla client could easily move itemid from folder to otherfolder without modifying X-MOZ-LOCATIONPATH - which would then be incorrect the next time Sunbird/Lightning loaded the item. Seems to me we'd be better off with something along the lines of an array this.mLocationPaths, keyed on item ids and populated in getItems and adoptItem with hrefs. Thoughts?

Comment 9

[Bruno Browning]

Hmm. I can't replicate #1 with Cosmo, either. Jakub, could you tell us a little more about your CalDAV server and the arrangement of 'folders' on it?

Comment 10

[Jakub Klos]

I believe that the current functionality is fine. You query the server and it responds. The hrefs from server you do not know you add them. The ones you know they are ok. If an item is moved by some other client on the server it will appear as a new item to your client. I guess this is ok.

Querying the server after PUT would work but I don't think it is the best effort. Is it really a problem to check the returned Location: header? Although the CalDAV draft dropped it, GroupDAV still supports it.

As for the CalDAV server we are using. It is a server written by me which connects to our Groupware server. I believe all other servers let you specify your own href and then they keep it. However our server supports folders and if you place an item to the root of the caldav uri it will be placed under the proper folder. PUT /webdav/newitem.cs our server will place it based on the item type to /webdav/Events/newid.ics

I could set it up for you to test the server but I din't think it's required. If you consider supporting the Location: header all would be fixed. Or we could address the CalDAV people to explain why the officially dropped the support...

Comment 11

[Matthew (lilmatt) Willis]

(In reply to comment #10)
> If you consider supporting the Location: header all would be fixed. Or we could
> address the CalDAV people to explain why the officially dropped the support...

I just spoke with Cyrus Daboo, one of the authors of the CalDAV spec, about this. This was dropped from the spec because doing this would apply specific semantics to the Location header that would not be true for other uses of HTTP. Since we're implementing the CalDAV spec (as opposed to GroupDAV), we don't want to check this header. 

Anyone who is interested in implementing GroupDAV is welcome to do so in an extension.  If someone is interested in writing the code and supporting GroupDAV in the default CalDAV provider, they're certainly welcome to try and convince us that this is the right thing to do.  That said, for reasons of code path simplicity, testing simplicity, and a preference for standards-track protocols, I suspect that would be an uphill battle.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 239968 [details] [diff] [review]
Patch implementing Jakub Klos' fix

Looks good; r2=dmose.

Comment 13

[cmtalbert]

Comment on attachment 239968 [details] [diff] [review]
Patch implementing Jakub Klos' fix

r+

Comment 14

[Matthew (lilmatt) Willis]

Delete patch checked in on MOZILLA_1_8_BRANCH and trunk

-> FIXED

Comment 15

[Jakub Klos]

Ok, I understand what you mean. Thank you for the DELETE fix.
How about a special handling of the PUT command then?

1. You add a new item

Comment 16

[Jakub Klos]

I'm sorry (for some reason my browser submitted the form)
2. You check via GET and the original href if the item is on the server (if the server responded with success).
3. If not you should refresh/reload the items from the server (probably the current view of the calendar and also the items on the left (upcoming events)).

How about that? Please, let me know
Thank you