braid-org / braid-spec

Working area for Braid extensions to HTTP
https://braid.org
232 stars 16 forks source link

Guidance for PUTs to legacy servers #117

Open toomim opened 11 months ago

toomim commented 11 months ago

Legacy servers and proxies won't understand the new headers we're defining for PUT to describe patches:

Legacy servers/proxies ignore headers that they don't understand, and thus will happily clobber the contents at their endpoints with the patches themselves, instead of applying them to their contents.

We need guidance on how to avoid this. I propose the following two approaches:

  1. Clients can do feature detection (TBD) on servers to discover which endpoints are safe to PUT to.
  2. Braid PUTs can be made to self-destruct on legacy servers by omitting transfer-encoding: chunked and content-length
    • Legacy servers then won't know when the PUT has completed, and won't commit the contents to disk or memory.
    • Braid-aware servers will deduce the end of a message from the combination of Patches: N and Patch-Length headers

The second approach eliminates the ability to use transfer-encoding: chunked with Braid PUTs, which has the downside that we can't stream large patch contents within an update, because the length of each patch must be computed ahead of time. However, the patches themselves can be used to implement streaming, by replacing a single patch with multiple patches. In essence, each "chunk" in transfer-encoding: chunked will become a separate patch. So I don't think we'll lose any functionality this way, although we end up with two ways to do the same thing.

Thoughts on this approach?

CxRes commented 11 months ago

I have been thinking about this since meeting 67, and I do not think using Partial PUT is a good idea (it is not backwards compatible even according to HTTP), since the standard expectation is that PUT is idempotent. Where a server ignores the Patches header, it would lead to bad things. The proposed solutions are extremely "hacky" in this regard.

I suggest that we investigate using POST which will typically only append the patch in unsupported systems. Another alternative might be to extend PATCH itself to support multiple patches in one request.

toomim commented 11 months ago

Ah, I'm glad you are bringing up idempotency! Partial PUT is idempotent when it specifies its Version:. We should add this to the spec!

Perhaps we could say:

  1. A server or client that receives an update (whether via PUT request or via GET response) with a Version: should only apply that update if it has not already applied an update for that version.
  2. And then for sending updates via PUT, we should say something like: Any PUT with a Content-Range SHOULD specify a Version: header so that it can be made idempotent.

We could also consider changing that SHOULD to a MUST, but this would actually be a stronger constraint than is in HTTP's Partial PUT spec today, and effectively declare all of today's Partial PUTs to be illegal, because they don't know about Version:.

Using POST or PATCH gives up on idempotency entirely. That does not sound like an improvement to me. There are a lot of advantages to using a method with constrained semantics. Perhaps I should write them up sometime soon.

However, I don't think we need to fight for a "one true way to issue updates." I'm in favor of allowing implementors to choose which method to send their patches over. We can specify the best practices using each of the methods. We have to think them through anyway, so it's not much more work to write down the practices we come up with. I think conventions on which standard to use can develop over time.

When you say "the proposed solutions are hacky", could you perhaps articulate your concerns in more detail? From my perspective, these solutions eliminate the problem quite elegantly.

mitar commented 11 months ago

Mention of idempotency reminded me of this very interesting concept in API design:

toomim commented 11 months ago

Good find!

This is the stuff that people have to do when they don't have proper versioning built into their HTTP architecture.

In Braid, I see a transaction as equivalent to a Branch of time (not specified yet BTW), with a separate HEAD, that either becomes validated or aborted. When you don't have that, you have to implement your own transaction mechanism, and your own mechanism for rolling back, or re-running failed transactions.

That means you have to re-run failed transactions ... without accidentally running them twice. Then you end up trying to add idempotency after the fact, and you create an idempotency key.

However, if you just implement your mutations as PUTs to Versions, you get idempotency for free, along with a real history mechanism, which lets you roll back or commit any branch of versioning at any point, in a standard way, that the whole system can obey.

This also enables you to maintain intermediate mid-transaction states across the whole system, which means you don't have to re-run as much of a transaction if something fails. You can restart from an intermediate state.

CxRes commented 11 months ago

Partial PUT is idempotent when it specifies its Version:

That is unfortunately a big condition which as you yourself recognize most servers don't know about.

I don't think we need to fight for a "one true way to issue updates."

Absolutely!

My only argument is to design defensively. Even if there is a small probability of a problem, let's avoid that as a solution in the first instance (unless there is an extremely strong mitigating argument).

When you say "the proposed solutions are hacky".

I mean that you are using the protocol in a way it is not designed to be used. In case there are implementations in the wild that deviate even slightly or take a shortcut, there is a risk of unwanted operation occurring (which in this particular case has a chance of data loss on legacy systems). A risk that does not sit well with me and feels unnecessary (I can think of multiple design alternatives, if you don't like POST and PATCH, maybe even invent a new method UPDATE which is idempotent).

If a client does incorrect feature detection or sends a Braid request to a non-Braid server, well, it will clobber. If an implementation ignores Content-Length header it will clobber. Do you (application developers that write to Braid and non-Braid servers) really want to take that chance? I think servers should be allowed to and be responsible for ensuring data integrity resulting from PUT (or any method) not a client.

Again, this is my defensive design bias in a wold full of systems that don't speak Braid!

Further, my suspicion is that IETF will not accept these solutions as well.

toomim commented 11 months ago

Ah, thank you for explaining your concerns. I think I understand where you are coming from:

If an implementation ignores Content-Length header it will clobber. Do you (application developers that write to Braid and non-Braid servers) really want to take that chance?

We don't need to take a "chance". We will run tests on a wide range of real internet proxies and servers to ensure that our strategies work. We won't be deploying any changes to HTTP before running all these tests. Consider that the HTTPWG is attended by all the major HTTP implementors, including many proxies. Any proposal will first be tested on their current implementations, and then we'll do an additional round of testing across various samples of real networks, servers, and proxies to validate that our guidance accurately explains their behavior.

As for your specific concern about Content-Length: the only way for a server to know the end of a PUT body is via Content-Length or Content-Encoding: chunked. If a server ignores those frame markers, then it is already broken: the only other option is to use heuristics (such as the end of the TCP connection, or a timeout) to determine the end of the PUT body. This means if you internet drops partway through a PUT, the server will already clobber its data with the partial representation.

But again, we will validate any designs that we like with extensive testing.

Thanks!

toomim commented 11 months ago

Also, an understanding of the utility of POST in Braid is currently needed for issue #118.

toomim commented 11 months ago

Finally, keep in mind that feature detection makes Partial PUT safe 100% of the time. It just requires a priori knowledge of the server, or a round-trip to OPTIONS.

CxRes commented 11 months ago

https://twitter.com/fielding/status/275471320685367296