373370 - Send If-Match / If-None-Match headers on PUT & DELETE

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Andrew McMillan]

Etags should be used by Mozilla when communicating with CalDAV servers to ensure assist with collision avoidance of events on creation, modification and deletion.

Typical use is as follows:

PUT (Create)  sends  If-None-Match: *

PUT (Replace) sends  If-Match: <existing etag>

DELETE        sends  If-Match: <existing etag>

In the event of a match / non-match the CalDAV server should return a "412" Precondition Failed response as per RFC2068, 14.25.

Given that this is RFC2068, rather than CalDAV, it may well apply to WebDAV calendars and WCAP requests as well.

Comment 1

[Bruno Browning]

Yes, ICS/WebDAV (at least) will benefit from this as well. 

For the CalDAV provider, basic etag support is (proposed to be) provided in bug 362698; using these HTTP headers is something we want to do, but it's a performance win (saves a roundtrip to the server) rather than an added feature. Using them is going to require some improvements to the mozilla webdav extension, likely something along the lines of allowing one to do

nsIWebDAVService.setRequestHeader("If-None-Match", "cached_etag", true);

before doing the PUT via the nsIWebDAVService

I'm going to reassign the bug to default rather than leaving it assigned to me: I haven't started working on it yet, don't know when I will, and don't want to discourage anyone else from taking it on. ;-)

Comment 2

[Bruno Browning]

Attachment: use if(-none)-match

This patch moves etag-checking server-side with if(-none)-match headers. Relies on query-by-uid. I think the patch is solid but should not be committed yet, because testing shows:
* patch works on Chandler Server
* patch works on Apple CalendarServer
* patch fails on Bedework, which seems to ignore the headers. This is a regression from the current client-side etag checking, which works with Bedework
* patch breaks our interop with RSCDS, which does not yet support query-by-uid

I'm most concerned with RSCDS here, and will request review once its query-by-uid is at least near release.

Comment 3

[Bruno Browning]

Still broken (server-side) with the new Bedework 3.4. Patch is expected to work RSN with the upcoming RSCDS 0.81 release.

Comment 4

[Bruno Browning]

Attachment: de-bitrot patch

de-bitrotting patch and requesting review. Tested and working with DAViCal, CalendarServer, and Oracle; issues remain with Bedework 3.4 and Chandler Server 0.8.1 due to server-side etag-handling issues in those products. Those issues have been reported; I think the level of breakage is acceptable in nightlies and expect the server-side bugs to be fixed before our next release.

Comment 5

[Daniel Boelzle [:dbo]]

Comment on attachment 287369 [details] [diff] [review]
de-bitrot patch

> 
>-        var listener = new WebDavListener();
>+        addListener: function addListener() {}

Why don't you use an ordinary object like

          var addListener = {};

?

Comment 6

[Daniel Boelzle [:dbo]]

Comment on attachment 287369 [details] [diff] [review]
de-bitrot patch

other than that, the patch looks good; r=dbo

Comment 7

[Bruno Browning]

Attachment: final tweaks

I was told that my cvs access was activated, but the repository is not letting me commit this change. So I'm posting the final version of the patch here in hopes someone can commit it for me while I sort out what's wrong with my cvs access.

Comment 8

[Daniel Boelzle [:dbo]]

Checked in on HEAD and MOZILLA_1_8_BRANCH.