Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DavMail connection not working with python-caldav, requires patched version of 0.7.0 #189

Closed
joneuhauser opened this issue May 23, 2022 · 11 comments

Comments

@joneuhauser
Copy link
Contributor

Hi, I'm connecting to a DavMail 6.0.1-3009 client. I'm attempting to connect to a caldav url like so:

https://caldav.example.org/users/[email protected]/calendar/

With python-caldav 0.9.0, I get the following error:

  File "C:\Users\jonat\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 478, in calendars
    return self.calendar_home_set.calendars()
  File "C:\Users\jonat\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 445, in calendar_home_set
    if '@' in calendar_home_set_url and not '://' in calendar_home_set_url:
TypeError: argument of type 'NoneType' is not iterable

This seems to be the same error as #172, however unlike with icloud, it also doesn't work to connect to https://caldav.example.org/ directly.

With python-caldav 0.7.0, I can connect (and read everything just fine), but only after removing the check here:

https://github.com/python-caldav/caldav/blob/master/caldav/objects.py#L120

Otherwise, zero calendars are detected.

@tobixen
Copy link
Member

tobixen commented May 23, 2022

Interesting, I've never heard of DAVMail before. It would be very useful to have some kind of test account that I could use for reproducing the error and/or run the test suite against, do you have any possibility to help me with that? Or would it be better to reach out to the DAVMail community?

@tobixen
Copy link
Member

tobixen commented May 23, 2022

I've sent an email to @mguessan

Alternatively, you would have to dig more into it yourself. Obviously calendar_home_set_url is set to None here, while a string is expected, which means self.get_property(cdav.CalendarHomeSet()) returned None rather than a string. It seems vaguely related to #180. Perhaps it's possible to work around it by just setting self._calendar_home_set to None or to a blank string and return if the server doesn't yield a calendar-home-set URL.

@joneuhauser
Copy link
Contributor Author

joneuhauser commented May 24, 2022

Thanks for your quick response! I don't think I can provide you with a test account. We use DAVMail to provide a CalDAV interface to MS Exchange. Maybe another relevant bit of info: the [email protected] is an Outlook shared mailbox.

Here's the response for the calendar-home-set PROPFIND request when connecting to https://caldav.example.org/users/[email protected]/calendar/:

<?xml version="1.0" encoding="UTF-8"?>
<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav" xmlns:E="urn:ietf:params:xml:ns:carddav">
    <D:response>
        <D:href>/users/[email protected]/calendar/</D:href>
        <D:propstat>
            <D:prop></D:prop>
            <D:status>HTTP/1.1 200 OK</D:status>
        </D:propstat>
    </D:response>
</D:multistatus>

Removing the workaround for OwnCloud (see 0e224be) seems to work for this case. Then, the calendar-home-set is set by

caldav/caldav/objects.py

Lines 419 to 420 in 529073d

self._calendar_home_set = CalendarSet(
self.client, self.client.url.join(sanitized_url))
.

When connecting to https://caldav.example.org/, I get (same credentials)

<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav" xmlns:E="urn:ietf:params:xml:ns:carddav">
    <D:response>
        <D:href>/principals/users/[email protected]</D:href>
        <D:propstat>
            <D:prop>
                <C:calendar-home-set>
                    <D:href>/users/[email protected]/calendar/</D:href>
                </C:calendar-home-set>
            </D:prop>
            <D:status>HTTP/1.1 200 OK</D:status>
        </D:propstat>
    </D:response>
</D:multistatus>'

So the href is different and the calendar-home-set is found. But even then, I don't get any calendar unless I:

  • remove quoting the '@':

    caldav/caldav/objects.py

    Lines 445 to 446 in 99494d1

    if '@' in calendar_home_set_url and not '://' in calendar_home_set_url:
    calendar_home_set_url = quote(calendar_home_set_url)
    OR
  • remove the check here:

    caldav/caldav/objects.py

    Lines 120 to 122 in 99494d1

    if (self.url.strip_trailing_slash() !=
    self.url.join(path).strip_trailing_slash()):
    c.append((self.url.join(path), resource_types,

The responses are identical in both cases, the both contain

    <D:response>
        <D:href>/users/[email protected]/calendar/</D:href>
        <D:propstat>
            <D:prop>
                <D:resourcetype>
                    <D:collection />
                    <C:calendar />
                </D:resourcetype>
                <D:displayname>Calendar</D:displayname>
            </D:prop>
            <D:status>HTTP/1.1 200 OK</D:status>
        </D:propstat>
    </D:response>
    <!--- and a lot of calendar events (probably?) afterwards -->

but of course '%40' != '@', so the (correctly found) calendar is not discarded. So it seems like the "hacky workaround" for owncloud is making problems here. Maybe the code could perform this substitution ONLY if the server response contains owncloud?

So personal and shared mailboxes have slightly different issues. Both work fine if

caldav/caldav/objects.py

Lines 445 to 446 in 99494d1

if '@' in calendar_home_set_url and not '://' in calendar_home_set_url:
calendar_home_set_url = quote(calendar_home_set_url)
is completely removed.

@tobixen
Copy link
Member

tobixen commented May 24, 2022

Thanks for your quick response! I don't think I can provide you with a test account. We use DAVMail to provide a CalDAV interface to MS Exchange. Maybe another relevant bit of info: the [email protected] is an Outlook shared mailbox.

Do you connect as the user [email protected], or do you connect as [email protected]?

Here's the response for the calendar-home-set PROPFIND request when connecting to https://caldav.example.org/users/[email protected]/calendar/:

I guess that is a calendar URL. I'm not sure if it's very well defined what the calendar-home-set should be for a calendar - the parent directory of the calendar, the calendar home set for the currently logged-in-user or undef. This is the first time I see the latter, but that does not necessarily mean it's a breach of standards.

            <D:prop></D:prop>
            <D:status>HTTP/1.1 200 OK</D:status>

200 OK, but an empty prop response. Perhaps 404 would have been more appropriate.

Removing the workaround for OwnCloud (see 0e224be) seems to work for this case. Then, the calendar-home-set is set by

Except for the risk of a None-value causing problems further on ... it seems like a no-brainer to include that, I've cherry-picked it into my master branch. Perhaps the "correct" would be to continue probing and searching for a valid calendar-home-set if none is found. At the other hand, there may be situations where the calendar-home-set is not needed. And I wouldn't add any complexity without having evidence that it will solve real-world problems.

So it seems like the "hacky workaround" for owncloud is making problems here. Maybe the code could perform this substitution ONLY if the server response contains owncloud?

No, I don't think so. The at-sign in an URL should be used only for URLs containing username and password, and should only be used in the beginning of the URL to separate those two. @ is not a valid character in the path in an URL. Actually both owncloud and DAVMail seems to be doing things wrongly - they should return %40 rather than @ when presenting the href. Either lines 120-122 should be modified to consider that URLs may be quoted, or work should be done to make sure self.url is always quoted in some cannonical form.

@joneuhauser
Copy link
Contributor Author

Do you connect as the user [email protected], or do you connect as [email protected]?

In both cases, I connect at [email protected].

If you want me to test any further updates against our davmail instance, I'll gladly do so. Thanks for your work!

@tobixen
Copy link
Member

tobixen commented May 24, 2022

I have a commit coming up now that I'd like you to test ...

tobixen added a commit that referenced this issue May 24, 2022
@joneuhauser
Copy link
Contributor Author

joneuhauser commented May 25, 2022

With this fix, I still get zero calendars, both for https://caldav.example.org/users/[email protected]/calendar/ and https://caldav.example.org/, the latter of which should yield my personal calendar.

In objects.py:105, properties always includes exactly one entry of the "{urn:ietf:params:xml:ns:caldav}calendar" type. It is discarded because its url is the same as self.url.

For the personal calendar, this is because the calendar_home_set is returned by the server as users/email%40example.org/calendar/ (and not users/email%40example.org/, which is presumably what other servers return), so it's the same as the current calendar url. For the function calendar, self.client.url is already the full path of the calendar, so calendar_home_set is just the calendar's url (calendar_home_set is returned as undefined by the server, as discussed earlier).

Maybe it's enough to check if the url already contains "calendar" at the end? With this change, everything works for me:

diff --git a/caldav/objects.py b/caldav/objects.py
index a051f79..016e1f1 100644
--- a/caldav/objects.py
+++ b/caldav/objects.py
@@ -117,8 +117,9 @@ class DAVObject(object):
                 # And why is the strip_trailing_slash-method needed?
                 # The collection URL should always end with a slash according
                 # to RFC 2518, section 5.2.
-                if (self.url.canonical().strip_trailing_slash() !=
-                        self.url.join(path).canonical().strip_trailing_slash()):
+                if (self.url.canonical().strip_trailing_slash().endswith("calendar")
+                    or (self.url.canonical().strip_trailing_slash() !=
+                        self.url.join(path).canonical().strip_trailing_slash())):
                     c.append((self.url.join(path), resource_types,
                               resource_name))

Edit: The comment in the code block says investigate the RFCs thoroughly - why does a "get members of this collection"-request also return the collection URL itself?. This confuses me - shouldn't be the type (tag) of the collection member be different than the one of the collection (e.g. calendar <-> calendar-home-set)? Why checking for url equality instead?

@tobixen
Copy link
Member

tobixen commented May 25, 2022

Hmmm ... what you are saying is that the calendar-home-set URL and the calendar URL is the same? You may be right that it may make sense to check the resource-type property, and not only the URL.

This is the DAVObject class, so the children method is a generic method for fetching children of a resource - it could be finding the calendar events on a calendar (yes, there are other methods for finding events on a calendar, but not all of them works for all servers, I think I've used calendar.children() as a fallback somewhere). I don't like the idea of .endswith("calendar") there. And yes, I've had problems that some servers would report the resource itself and not only the children resources when doing the request - so the lines are there for a reason, they shouldn't be removed :-)

@joneuhauser
Copy link
Contributor Author

Hmmm ... what you are saying is that the calendar-home-set URL and the calendar URL is the same?

Yes, for both private and shared mailboxes.

Afaics, .children() is only used very few times, so the following check should be safe:

diff --git a/caldav/objects.py b/caldav/objects.py
index a051f79..75cb86c 100644
--- a/caldav/objects.py
+++ b/caldav/objects.py
@@ -117,8 +117,9 @@ class DAVObject(object):
                 # And why is the strip_trailing_slash-method needed?
                 # The collection URL should always end with a slash according
                 # to RFC 2518, section 5.2.
-                if (self.url.canonical().strip_trailing_slash() !=
-                        self.url.join(path).canonical().strip_trailing_slash()):
+                if ((isinstance(self, CalendarSet) and type == cdav.Calendar.tag)
+                    or (self.url.canonical().strip_trailing_slash() !=
+                        self.url.join(path).canonical().strip_trailing_slash())):
                     c.append((self.url.join(path), resource_types,
                               resource_name))
 

@tobixen
Copy link
Member

tobixen commented May 25, 2022

That seems good. Can you prepare it as a commit or a pull request?

@joneuhauser
Copy link
Contributor Author

Thanks for the quick response :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants