Closed Bug 1670415 Opened 1 year ago Closed 7 months ago

Discovery does not work, when the principal has two calendar-home-sets

Categories

(Calendar :: Provider: CalDAV, defect)

Thunderbird 82
defect

Tracking

(thunderbird_esr78 unaffected)

RESOLVED FIXED
88 Branch
Tracking Status
thunderbird_esr78 --- unaffected

People

(Reporter: dpa-mozilla, Assigned: darktrojan)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:81.0) Gecko/20100101 Firefox/81.0

Steps to reproduce:

With Thunderbird 82.0b2 I click "+ Add Calendar" in the calendar view and enter as server https://mail.aegee.org, without username. Then click on Find Calendars. TB calls

PROPFIND https://mail.aegee.org/dav/principals/user/anonymous/

<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D='DAV:' xmlns:C='urn:ietf:params:xml:ns:caldav'>
<D:prop>
<D:resourcetype/><C:calendar-home-set/>
</D:prop>
</D:propfind>

and the answer is:
207 Multi-Status
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
<D:response>
<D:href>/dav/principals/user/anonymous/</D:href>
<D:propstat>
<D:prop>
<D:resourcetype><D:principal/></D:resourcetype>
<C:calendar-home-set><D:href>/dav/calendars/user/chair/</D:href><D:href>/dav/calendars/user/k/</D:href></C:calendar-home-set>
</D:prop>
<D:status>HTTP/1.1 200 OK</D:status>
</D:propstat>
</D:response>
</D:multistatus>

essentially two home sets.

For anonymous access the server has anyway always to return two home-sets, otherwise https://bugzilla.mozilla.org/show_bug.cgi?id=1599602 triggers (for anonymous CalDAV access Scheduling INBOX makes no sense).

As next Thunderbird calls “PROPFIND https://mail.aegee.org/dav/calendars/user/chair/,/dav/calendars/user/k/” with comma, which obviously is not going to work.

Thunderbird shall instead do a separate PROPFIND call during the discovery for each calendar-home-set.

I use Thunderbird 78.8.0. I unzipped omni.ja, modified as follows,

--- modules/CalDavCalendar.jsm.orig	2021-03-03 22:55:03.992193064 +0200
+++ modules/CalDavCalendar.jsm	2021-03-03 23:24:06.598429490 +0200
@@ -1778,6 +1778,7 @@
           return normalized == chs.path || normalized == chs.spec;
         };
         let createBoxUrl = path => {
+          if (!path) return
           let newPath = this.ensureDecodedPath(path);
           // Make sure the uri has a / at the end, as we do with the calendarUri.
           if (newPath.charAt(newPath.length - 1) != "/") {
@@ -1811,7 +1812,7 @@
           this.mInboxUrl = createBoxUrl(response.firstProps["C:schedule-inbox-URL"]);
           this.mOutboxUrl = createBoxUrl(response.firstProps["C:schedule-outbox-URL"]);
 
-          if (this.calendarUri.spec == this.mInboxUrl.spec) {
+          if (!this.mInboxUrl || this.calendarUri.spec == this.mInboxUrl.spec) {
             // If the inbox matches the calendar uri (i.e SOGo), then we
             // don't need to poll the inbox.
             this.mShouldPollInbox = false;

zipped again omni.ja and now it works.

Rationale: my system offers unauthenticated CalDAV access (no WWW-Authorization headers are set). The unauthenticated user gets one calendar-home-set in the answer, one schedule-outbox-URL (as the unauthenticated user is in theory entitled to query the free busy status of some users), but no schedule-inbox-URL. This lead to calling “this.mInboxUrl = createBoxUrl(null)” which threw an exception.

  • Handle the case, when a CalDAV user has no scheduling-inbox
  • e.g. by setting mShouldPollInbox to false

I will appreciate very much if the fix in included in the next minor release.

Attached patch Fixes null pointer exceptions (obsolete) — Splinter Review
Attachment #9206751 - Flags: feedback?(geoff)

The attachment is in fact a fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1670415 .

Assignee: nobody → dpa-mozilla

This cannot be assigned to me, since I have provided a patch and there is nothing more I can do.

It's been assigned to you because you provided a patch. You were not being asked to do anything more.

That said I'm going to handle this patch in bug 1599602 where it was originally posted.

I'll take over this bug because I think I've already figured it out. (However after fixing the code the next PROPFIND request to https://mail.aegee.org/dav/calendars/user/<whatever>/ returns an HTTP 500 error.)

Assignee: dpa-mozilla → geoff

Comment on attachment 9206751 [details] [diff] [review]
Fixes null pointer exceptions

Making this patch obsolete in favour of the one in bug 1599602.

Attachment #9206751 - Attachment is obsolete: true
Attachment #9206751 - Flags: feedback?(geoff)

response.firstProps returns an array of strings, but we're handling it as a string, which works in most cases but fails when there's more than one value.

In TB 87b2 I apply the updated patch to omni.ja, then I open the Calendar, add as Network calendar https://mail.aegee.org:444/dav/calendars/user/chair/agenda/, select “Does not require credentials” → Subscribe. No 500 error: it works.

The TBSync/DAV-4-TbSync addons are not available for TB 87b2.

When I add https://mail.aegee.org as source (Calendar), TB calls

PROPFIND /dav/principals/user/anonymous/

<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D='DAV:' xmlns:C='urn:ietf:params:xml:ns:caldav'>
  <D:prop>
      <D:resourcetype/><C:calendar-home-set/>
  </D:prop>
</D:propfind>

the server replies:

<?xml version="1.0" encoding="utf-8"?>
<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
  <D:response>
    <D:href>/dav/principals/user/anonymous/</D:href>
    <D:propstat>
      <D:prop>
      <D:resourcetype><D:principal/></D:resourcetype>
      <C:calendar-home-set>
        <D:href>/dav/calendars/user/aaa/</D:href>
        <D:href>/dav/calendars/user/bbb/</D:href>
        <D:href>/dav/calendars/user/chair/</D:href>
        <D:href>/dav/calendars/user/k/</D:href>
      </C:calendar-home-set></D:prop>
      <D:status>HTTP/1.1 200 OK</D:status>
    </D:propstat>
  </D:response>
</D:multistatus>

and then TB calls literally:

PROPFIND /dav/calendars/user/aaa/,/dav/calendars/user/bbb/,/dav/calendars/user/chair/,/dav/calendars/user/k/

Can you please clarify which call returned 500 and with which TB version?

This patch was created for the current development tip (ie. 88) but is very likely to work on 87 too. If you must edit files in omni.ja, make sure you start Thunderbird with the --purgecaches argument or it will continue to use the unedited version.

Whatever I do I cannot reproduce the 500 result. Can you please provide the call, including headers and payload, which triggered 500 answer?

Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

I think I've just discovered why. We're sending an Authorization: Basic Og== header (basic auth without a username or password) which shouldn't be happening. Without that header a 401 response is received, which I guess makes sense.

Anyway here's the request that we're sending, which is just like it was before except the URL is fixed:

PROPFIND /dav/calendars/user/aaa/ HTTP/1.1

<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D='DAV:' xmlns:A='http://apple.com/ns/ical/'><D:prop><D:resourcetype/><D:displayname/><A:calendar-color/></D:prop></D:propfind>

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d062fa9d4a25
Handle multiple calendar home sets found in CalDAV discovery. r=lasana

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

Is the sending of the Authorization: Basic Og== header resolved by now? ⇔ Shall I open a ticket for it?

Regressions: 1701296
You need to log in before you can comment on or make changes to this bug.