377489 - Incorrect result passed to listener in getitems() calDav provider

Status: RESOLVED INVALID

(Calendar :: Provider: CalDAV, defect)

Description

[Tim]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9a1) Gecko/20070213 Sunbird/0.3.1

As mentioned in bug 355270, I open and fill a new bug report.
I have made some tries with Cosmo 0.6 and I found out the following problem
when subscribing with a simple calendar with one event. When calling getItems()
in calDavCalendar.js, the Cosmo server returns a code 207 and not 200. This
code is considered as a multi-status code and I suppose that it can be returned
if the request is successful. Am I wrong ?
At least, the calendar is properly displayed in the month view. But the problem
is that the listener is called with Components.results.NS_ERROR_FAILURE which
reports an error. Note that the error code 207 is not accessible in the
listener.
Another weird thing in the code is that Components.results.NS_ERROR_FAILURE is
also passed to the listener even if the result is 200 (OK). So I wonder if
there is not a slight code mistake: "rv" variable should be used instead of
Components.results.NS_ERROR_FAILURE

Here is the code part I am talking about in calDavCalendar.js:

// parse aStatusCode
var rv;
var errString;
if (aStatusCode == 200) { // XXX better error checking       
    rv = Components.results.NS_OK;
} else {               <== 207 return code is considered as an error
    rv = Components.results.NS_ERROR_FAILURE;
    errString = "XXX something bad happened";
}
// call back the listener
try {
    if (aListener) {
        aListener.onOperationComplete(thisCalendar,
            Components.results.
            NS_ERROR_FAILURE,                 <== rv instead ?
            aListener.GET, null,
            errString);
    }


Reproducible: Always

Steps to Reproduce:
Install Cosmo 0.6, launch it and subscribe to the demo calendar with a calDav URL in Sunbird/Lightning.
Actual Results:  
In getitems(), the HTTP request to get the events returns a 207 result code and the unique event is properly displayed in calendar view. But the listener is called with a Components.results.NS_ERROR_FAILURE error code.

Expected Results:  
The listener should be called with a Components.results.NS_OK.

Comment 1

[Bruno Browning]

Confirming. RFC4791 says that the response to a successful REPORT request MUST be a 207 multistatus (though I suspect that the CalDAV draft did not say that when the code in question was written). We need to test for 207 rather than 200, return rv, and provide UI with a reasonable, localized error in the case of failure.

Comment 2

[Bruno Browning]

My apologies for confirming this: I was simply wrong, as, I'm afraid, is this bug. The RFC does specify a 207 Multistatus response, and Cosmo does give one, but that is parsed out by nsIWebDAVService and is gone before the CalDAV provider ever sees it. The 200 OK which the CalDAV provider checks for in the code referenced above is in reference to one of the responses contained within the multistatus. So the code is correct insofar as it goes. I am therefore closing this bug. Reporter, if you think I'm wrong (again!) feel free to reopen.

Comment 3

[Tim]

The problem is that even when there is no error returned, in real use, the aStatusCode checked by CalDAV is 207 and is considered as an error. So maybe it is not normal to receive a 207 since this result code is only a way to signal that a set of status code should be checked, which is performed, based on what you say, in the nsIWebDAVService. So maybe the problem is that we should never receive a 207 error code in the CalDAV provider. But actually we do. So there is still very likely a problem... What do you think of it ?

Note also that I think that my last remark is still valid regarding the replacement of "NS_ERROR_FAILURE" with rv, since without it, an error is always returned independently of the previous return code.

If I am not mistaken, I think that this bug should be reopened for the NS_ERROR_FAILURE/rv problem. And regarding the first problem, it you agree it is still valid, maybe it should be transfered to another component. Let me know what you think about it.