search

sabre-io / dav

sabre/dav is a CalDAV, CardDAV and WebDAV framework for PHP
http://sabre.io
BSD 3-Clause "New" or "Revised" License
1.51k stars 344 forks source link

Send ETag after a sucessful PUT request #172

Closed evert closed 11 years ago

evert commented 11 years ago

Original author: evert...@gmail.com (September 20, 2010 18:45:10)

Not required, but useful in some cases.

Original issue: http://code.google.com/p/sabredav/issues/detail?id=81

evert commented 11 years ago

From lars.kne...@gmail.com on September 08, 2011 18:04:32: I'm currently in the process of making sabredav working with SOGO connector (carddav).

One thing that breaks this client is the absent of the ETag header.

What would be the best place to add the ETag header?

evert commented 11 years ago

From evert...@gmail.com on September 08, 2011 18:14:10: Add it to Sabre_DAV_Server::httpPut, there's a Content-Length header added already, so you could add it there. If you make it a patch or a mercurial clone, I'd happily add it to the source.

evert commented 11 years ago

From lars.kne...@gmail.com on September 08, 2011 18:52:25: See my attached patch.

If Sabre_DAV_Server::createFile could return the created node, we could save an extra roundtrip.

evert commented 11 years ago

From evert...@gmail.com on September 09, 2011 12:59:36: I'm a little scared of this change.

The problem lies in heavily modified backends. If a new file gets created and stored in a database, but then the etag is requested, the etag may no longer represent the exact content as it was specified in the PUT body, as it may reflect the representation after subsequent GET's.

I know in some of the implementations I've done, I'm parsing iCalendar files to a database model. If a GET is done after, the file will often look slightly different (but contain most of the data). In those cases CalDAV clients will not pick up on the new data, and the mechanism is technically broken.

evert commented 11 years ago

From lars.kne...@gmail.com on September 10, 2011 05:59:09: I also discovered this problem in the current implementation of the CardDAV_* classes, while implementing the carddav backend for Tine 2.0.

I would propose following minor change. When storing a card in Sabre_CardDAV_Card::put just reread the stored card and repopulate $this->cardData. This way we have the latest data stored in the backend and can calculate the correct ETag.

I'll attach the patch. With this patch always the correct ETag get's calculated.

evert commented 11 years ago

From lars.kne...@gmail.com on September 10, 2011 06:25:41: While we are at this topic. I would also propose that etag can be a property of Sabre_CardDAV_Card::cardData.

Then the backend could provide an etag. It's very likely that the application behind the backend already generates some kind of etags also for other usage.

If no etag is provided in cardData we could fallback to the current solution, by calculating the md5 hash.

evert commented 11 years ago

From evert...@gmail.com on September 10, 2011 09:51:28: This still doesn't fully solve the following scenario:

  1. PUT request is done, ETag is returned (for example an MD5 of the PUT body)
  2. Data is stored, but modified because of how the backend works
  3. ETag must now be different for subsequent GET's

I'm thinking the easiest way to go about a true fix for this, is to allow put() and createFile() methods to return an etag after. If it's null, we don't do anything.

This is a bit of a bigger change though, and something I want to spend some time thinking about before implementing.

The other option is to return a 'weak etag', but I don't know how clients respond to this.

evert commented 11 years ago

From lars.kne...@gmail.com on September 14, 2011 06:04:02: A minor update on this problem.

While implementing a carddav backend for the addressbook of Tine 2.0, I discovered that we can't use the UID provided in the vcard. Because of that also the ETag logic does not work as expected.

I have changed the logic to always return "1" as ETag for new cards. During the next sync the clients fetches the real ETag and updates the vcard again. After that everything is in sync.

Not yet perfect, but we are moving forward...

evert commented 11 years ago

From lars.kne...@gmail.com on September 14, 2011 07:26:24: And yet another problem to solve with same solution.

When uploading a new card or event, we also need to return the location header, if we did not stored the card under the url requested by the client.

Which means we don't need to the ETag only, but also the Location header to point to the new URL.

See: http://greenbytes.de/tech/webdav/draft-dusseault-caldav-04.html#creating-components

Do you have an idea how to implement this already? I could help you implementing it.

evert commented 11 years ago

From lars.kne...@gmail.com on September 14, 2011 21:26:13: Just to let you know. I changed the logic in my code.

I reverted my changes in Server.php and set the needed header directly in the card handling class.

As I don't have access to the Server class there, I just set the headers via the header function.

I know that this is not the "right" way to do this, but it works.

evert commented 11 years ago

From evert...@gmail.com on March 05, 2012 16:13:38: This is implemented in 1.6.1. Now any createFile() or put() method can optionally return an etag.