Adding custom headers and HTTP methods to HTTP API

[Michael Nutt]

Hello,

I started a few weeks ago building a desktop-synced file storage app for sandstorm. I have been using OwnCloud for a few years but don't use anything other than its file sharing. Its plugin system also doesn't fit well into the sandstorm model.

However, it has a pretty good desktop client called Mirall. It's written in Qt and is portable across a variety of platforms, and the communication to OwnCloud is webdav-based. So I decided to swap out OwnCloud server, write my own server app, and integrate it into sandstorm. I pretty quickly got the server app (named Davros) built out and running but realized that there was a lot of work to do to make webdav work through the proxy and legacy bridge. The changes that I ended up making include:

* Adding the methods PROPFIND, PROPPATCH, MKCOL, COPY, MOVE, LOCK, UNLOCK, ACL, REPORT, and HEAD
* Adding webdav-specific request headers Depth, Destination and Overwrite, and response headers DAV and ETag
* Adding response codes 204, 206, 207, and 304. The extra noContent codes don't really fit the existing shouldResetForm heuristics so I ended up having to switch them over to just specifying the successCode in the schema.
* Making OPTIONS requests hit the grain, as the "DAV" header can potentially be part of the response and there is no way for the proxy to determine the correct value otherwise.
* I updated the auth to always challenge with a 401 when the authorization header was not presented, before realizing that it opens us up to using browsers + url-based authentication. I'll probably just ask to add Mirall to the list of browsers that can't do bearer auth.

The changes are in a branch here: https://github.com/sandstorm-io/sandstorm/compare/master...mnutt:webdav-bridge
The sandstorm app is here: https://github.com/mnutt/davros

I'd be happy to write some tests for the webdav changes; I ran Litmus against it and it passed all the tests my actual webdav server could pass. Webdav has all kinds of crazy extensions (CalDAV, CardDAV, etc) but I didn't need them.

But lastly, very late in the game I discovered that on large files the Mirall client strays away from webdav and implements its own API using custom http headers OC-Total-Length, OC-Chunk-Size, X-OC-Mtime, OC-FileId, and OC-Chunked. These feel somewhat ugly to hard-code in sandstorm, but if there was a single place it may not be too bad.

What would be the best way to proceed? Should I try to update the branch to have a single list of whitelisted headers? Is it alright to open a pull request for feedback even though it's not ready to be merged?

By the way, here's an end-to-end test of the desktop client syncing with sandstorm:

https://www.youtube.com/watch?v=5aouwWr94jY

Cheers,
Michael

[Kenton Varda]

Holy crap, that's amazing!

I definitely want to get this merged soon -- this is something a lot of people want.

I think it's reasonable to whitelist some headers to be passed through. We have some other use cases for this as well. It seems particularly reasonable to me to support ownCloud headers as a "de-facto standard".

I think the right way forward is to add a new field to `Context` like:

    additionalHeaders @6 :List(Header);

    # Additional headers present in the request. Only whitelisted headers are

    # permitted.

    struct Header {

      name @0 :Text;  # lower-cased name

      value @1 :Text;

    }

    const headerWhitelist :List(Text) = ["oc-total-length", ...];

    # Non-standard request headers which are whitelisted for backwards-compatibility

    # purposes. This whitelist exists to help avoid the need to modify code originally written

    # without Sandstorm in mind -- especially to avoid modifying client apps. Feel free

    # to send us pull requests adding additional headers.

We've actually been meaning to add something like this for a while.

On another note, glancing at the code, I think instead of adding several different "Content" structs all of which contain exactly the same fields, let's just add one new struct "Content" and plan on deprecating PostContent and PutContent in favor of Content when we do an API version bump.

I might also have some thoughts on how best to integrate caching stuff (etag, preconditionFailed) -- I started working on this a while back but got distracted. See the CachePolicy struct (currently defined but not used anywhere).

Unfortunately, this week we are in launch crunch mode. It will probably be a week or two before I can give this the attention it deserves. :/

-Kenton

--
You received this message because you are subscribed to the Google Groups "Sandstorm Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sandstorm-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.