hi andrew, thanks for that detailed research!
chen, harish, gicmo, can i ask you to comment/take a look at this? tia.

I commented on the macosforge.org track thingy about this. I am afraid but I think Andrew might be wrong with his namespace assumtions. We also don't expect the xml document to have a specific "D:" prefix at all. So Cyrus is wrong here as well. (We shouldn't really be *guessing* about other people's code) Something else must be broken in Evolution. Btw when *adding* stuff to a calendar there is no XML involved at all. It is a simple Http/1.1 PUT.  A detailed trace of the converstaion would be *very, very* helpful. Thanks very much for the research. We need more poeple like this! ;-D
Ohh and if its crashing a stacktrace is the thing to have as well. 

Created attachment 72644 [details]
TCPDump of communication up to crash

This is a tcpdump of a session from the point at which Evolution was started, through to the point at which it crashed.

Note that the last thing that Evolution requested from the CalDAV server was the REPORT request, which does use XML, and not the PUT, which happened in a previous session.

The only item in this calendar was the single event that Evolution had PUT there in the previous session (after which Evolution had crashed, presumably when parsing the REPORT response from Apple's Calendar Server).

Ohh thanks for that fast response. Very nice! Soo the problem is, the XML the server spits out looks totally valid to me. One thing: The server sends relative URLs which I *bet* we don't handle right yet (shouldn't be that hard to fix thouhg). But I still haven't got a clue why we crash. Any chance you can get me a stack trace of evolution-data-server?

Ok, my blind shot in the dark is this piece of code:

		object.href  = g_strdup (e_cal_component_get_href (cache_comp));
		object.etag  = g_strdup (e_cal_component_get_etag (cache_comp));

in modify_object () with one of the get_XXX functions returning NULL. No idea why they could be NULL though. 

Created attachment 72650 [details]
Bug Buddy trace of evolution-data-server-1.6

I have installed evolution-dbg, evolution-data-server-dbg, libglib2.0-0-dbg, libgtk2.0-0-dbg, libgnomevfs2-0-dbg, libc6-dbg and libxml2-dbg before generating this trace.  Hopefully that is enough.

This is happening on Debian Sid, BTW, but I think a workmate has replicated the crash on Gentoo using Evolution 2.8 / Gnome 2.16.

Created attachment 72651 [details]
TCPDump of communication against a different CalDAV server

This is a TCP dump of the process of using Evolution against a different CalDAV server showing that what would normally happen after the REPORT response would be a GET for the desired events.

Note that this CalDAV server is (very much) under development, so it reports an excessive and untrue list in response to the OPTIONS request. I have written this server specifically to work with Evolution at this stage, but it very likely has bugs in respect of specification compliance.

Andrew : Thanks for your involvement and the analysis :-). We do not have an Apple  server in our setup but until then, please do help us with more bug reports.
Gicmo : I will add this bug to the task list on the 2.10 Roadmap on go-evolution.org, that will be chalked out later this week

I can confirm that this problem is related to the way Apple's Calendar Server (legally) provides relative URLs for resources, as Christian Kellner suggests above.

Since posting this bug I have written my own CalDAV server, which is now largely working with Evolution, Sunbird / Lightning and Mulberry.  I have modified this to allow switching the URL output style.

When I output URLs for the REPORT response in the form:

  http://hostname/path/to/resource

everything works and is fine and dandy.

When I output URLs for the REPORT response in the form:

  /path/to/resource

then evolution-data-server crashes after receiving the REPORT response and before requesting the resources, which is normally it's next step.

If someone wants to test a fix for this I am happy to provide a publicly accessible CalDAV server which will produce output as relative URLs.  I have also replicated the problem with Evolution 2.8.

Thanks,
Andrew McMillan.

Adding support for relative URLs shouldn't be that hard, but I seem to be busy the next few weeks. Someone wants to hack that up? I would of course be more then happy to do some reviewing!
The place where we parse href is a centeral one and we should be able to come up with a simple function that checks if a given href starts with a http:// and if not prepends the url of the server (which we already have somewhere in the backend struct)
A second, different approach would be to only store relative urls in the cache (and the object) and always fix up the url when doing requests.

I think I prefer the first approach though. I am also willing to help during the implementation so I am adding the gnome-love keyword.

I've been trawling around in the source to 1.8.1 enough now that I can probably figure out some way to do this.

Always storing them as relative URLs might actually be one way of killing bug #355659 at the same time since the fundamental problem there is that the server and the client can have different ideas as to what the URL is, and the server's choice will eventually win.

Yeah, you might be right about the href issue! 
I am pretty sure you could do it! As mentioned earlier I am also here to help in case you need some. I am on irc.gimp.org as gicmo as well. If you feel like doing it, please go ahead! ;-D 

Thinking about this some more, it seems to me that the cache needs the fully expanded URLs (you wouldn't want to have a namespace collision between two sites, or maybe even between two different logons on the same site) and so the best solution to both problems would be to write a small function which could be used to normalise any server-supplied URL into a form that Evolution can always use.

A function something like this (excuse my pseudocode - it's maybe 20 years since I did any substantial C programming :-)


char * normalise_href( char * server_url, char * username, char * in_href ) {

  Split the server_url up into useful parts:
    protocol, hostname, portnum, root_relative_url

  if ( in_href is a relative URL not from the root ) {
    Re-base in_href from the root.
  }

  if ( in_href is not a relative URL ) {
    Remove any "protocol://hostname:portnum".
  }

  Allocate a buffer

  snprintf( buffer, size, "%s://%s@%s:%d%s", protocol, username, hostname, portnum, root_relative_url );

  return buffer;

}



We can then call this function on any href we receive from a remote server before we see if we have it in out cache.


This approach seems to me one that will have minimal impact and which will solve both problems identified here (and possibly a couple of others that aren't!).


I'm away at a conference for the next few days, but if this seems sound I will try and write something for review in the next week or two.

Thanks,
Andrew McMillan.

Created attachment 76400 [details] [review]
Patch that creates full URIs from server listings

Here's a patch that fixes the problem for me.
Not as elaborate as the suggestion in comment #13, but it gets the job done. :-)

I've just rebased the URIs that we receive when getting a server listing, it seems like this is enough. Is there any other place that we get URIs from the server?
The error checking at the end of the patch would have caught this error if it had been there before, but I don't know if there is any point in having it there now.

Opinions? Reviews?

While the patch does fix this bug, it doesn't fix the problem where the server thinks a URL is one thing, and Evolution thinks it is something else, covered by bug #355659.

It looks to me though as if that 'soup_uri_new_with_base' function does kind of what I was looking for anyway, and that maybe both bugs could be solved if we could do the same thing in synchronize_object or caldav_server_get_object so that the URLs are always reliably constructed in the same way.

Unfortunately my fiddling with it doesn't seem to achieve that goal, so I dunno.

I guess this is a minimal enough solution that it should go in, and we should look for a different solution to the other problem, since this bug is a crasher whereas the other one just makes people go "Huh?! WTF!".

So that's a "Thank you. Please apply this!" from me :-)

I don't see the behaviour described in bug #355659 with Apple's CalendarServer. So it's probably something specific to Cosmo, as you suggests in that bug, even though it may be Evolutions fault.
I'm not going to do anything with bug #355659 for now (unless I run into it myself), as my main goal at the moment is to implement basic CalDAV support in Horde.

No, you won't see bug 355659 with Apple's Calendar Server because your patch forces the start of the URL to be generated by Evolution, which will necessarily agree with other URLs generated by Evolution.  That bug required the client and server to come up with two URLs to the same resource which were not the same strings, such as by using "caldav://" for the protocol, or by including a port number which was a default port.

I have posted a possible patch for bug 355659 now in any case, although I'm not entirely happy with it.

Patch is mailed to the evolution-patches mailing list.

*** Bug 394575 has been marked as a duplicate of this bug. ***

Created attachment 81193 [details] [review]
Patch from Jari Urpalainen

Copied over form the dup'ed bug.

What I don't like about both patches is how we decide if the url is relative or not. One way would be to steal gnome-vfs code:

----
static gboolean
is_uri_relative (const char *uri)
{
	const char *current;

	/* RFC 2396 section 3.1 */
	for (current = uri ; 
		*current
		&& 	((*current >= 'a' && *current <= 'z')
			 || (*current >= 'A' && *current <= 'Z')
			 || (*current >= '0' && *current <= '9')
			 || ('-' == *current)
			 || ('+' == *current)
			 || ('.' == *current)) ;
	     current++);

	return  !(':' == *current);
}
----

But actually doing a strncmp (uri, "http://", 7) == 0 could be totally sufficient, I guess. What do people think about that?

<nag>
Any chance on getting *one* of these patches for this committed at some point?
</nag>

Christian, any progress? ;)

Regarding both CalDAV as well as gnome-vfs you are the right guy to ask, IMHO. So whatever patch *you* are happy with will be a really good way to handle it. Feel free to ping me on IRC, if you need approval. :)

So, ok, some more thinking lead me to the question: why not just store the filename (i.e. strip path and host, well everything but the filename). And always build up the correct path when we store it. So we would be safe no matter what. Opinions and/or comments?

Yes, storing the path relative to the calendar base would work OK, but it would be a lot more work than any of these patches so far, I think.

I guess it is also possible that <DAV:href> value from the server might not be related to the calendar base path in any intuitive way, but I suppose that could be handled by falling back to storing the whole thing.

What would be gained by storing the relative path, other than a fix for this bug (which AFAICS we already have).

Cheers,
Andrew McMillan.

I'm sorry that I haven't updated my patch (from the mini-review in comment 21), and I don't have any time for trying a new patch before August at the earliest.

But the main problem with this bug isn't really how e-d-s stores the path, but that it will crash with any CalDAV server that responds with relative URIs, as e-d-s tries to construct a new URI from the relative path, and doesn't do any error checking on the result (see http://bugzilla.gnome.org/attachment.cgi?id=76400&action=diff#e-cal-backend-caldav.c_sec3)

Gicmo: Can you review the patches and take charge? Thanks :)

So, ok this bug should be fixed with the patched that also fixes bug #326316. The small story is: we only store the last part of the href (i.e. the filename) in the  local cache (and the component) and reconstruct the full uri on every server interaction. I will attach the patch here, and I would highly encourage everybody to look over it and shout and cry on any issue you can find. Thanks

Created attachment 94203 [details] [review]
Store relative urls

Please review it as good as you can! ;-)

The other relevant bug is bug #355659 not bug #26316. Sorry for the spam.