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

After we remove 'keep-alive', should the "Subscribe" header change? #101

Closed
canadaduane opened this issue Mar 1, 2021 · 8 comments
Closed

Comments

@canadaduane
Copy link
Member

After PR #93 is merged, we'll have changed the "Subscribe" header from 3 variants:

           Subscribe
      or   Subscribe: keep-alive
      or   Subscribe: keep-alive=<seconds>

to a single variant:

           Subscribe

I like this simplifying change quite a bit; however, this carries several issues forward and introduces some new ones.

  1. I don't think "Subscribe" (i.e. without a colon, ":") is a spec-compliant header. According to the HTTP 1.1 spec, RFC 2616 p. 31, "HTTP Header Fields...follow the same generic format as that given in Section 3.1 of RFC 822 [9]. Each header field consists of a name followed by a colon (":") and the field value." RFC 822 Section 3.1.2 also specifies "Once a field has been unfolded, it may be viewed as being composed of a field-name followed by a colon (":"), followed by a field-body, and terminated by a carriage-return/line-feed."
  2. Spec-conformance aside, it's clear that common tools such as curl expect headers to have a colon and value.

For example, neither curl -H Subscribe nor curl -H Subscribe: will successfully pass "Subscribe" as a request header, but "Subscribe:keep-alive" is passed as a header:

# "Subscribe" with no colon:
$ curl -vH Subscribe https://invisible.college:4545/posts
> GET /posts HTTP/1.1
> Host: invisible.college:4545
> User-Agent: curl/7.54.0
> Accept: */*
>
...

# "Subscribe" with colon but no value:
$ curl -vH Subscribe: https://invisible.college:4545/posts
> GET /posts HTTP/1.1
> Host: invisible.college:4545
> User-Agent: curl/7.54.0
> Accept: */*
>
...

# "Subscribe" with colon and value:
$ curl -vH Subscribe:keep-alive https://invisible.college:4545/posts
> GET /posts HTTP/1.1
> Host: invisible.college:4545
> User-Agent: curl/7.54.0
> Accept: */*
> Subscribe:keep-alive                         <----- SUCCESS
>
...
  1. Finally, if we reduce 3 variants to a single variant, but a header value is required, what should it be? "On", "Yes", "True" etc.?
@josephg
Copy link
Contributor

josephg commented Mar 2, 2021

There's an option we might want to specify that we haven't captured anywhere else in the spec yet, and this might be a good place for it. And that is a server flag specifying whether operations are linearized. (Eg through transform in an OT system).

Lets say the server received these updates:

  A
 / \
B   C

The server can send the updates as-is:

  1. Update A
  2. Update B (with parent A)
  3. Update C (with parent A)

This makes sense in a CRDT / sync9.

Or it can use OT to linearize it to this, and send the linearized version:

  A
 /
B
 \
  C'
  1. Update A
  2. Update B (with parent A)
  3. Update C' (= C transformed by B, with B as the new parent).

The linearized form is how most OT systems (like sharedb / sharejs / google docs) work. It saves a lot of complexity for the client:

  • If the client never intends on editing the data, the client can simply apply the updates it receives in order.
  • If an update reaches the server with an old parent, clients of an OT system don't have enough context to transform the update. (In OT, the client never has to store or interact with old operations at all). It makes sense for transform to happen in the server, to avoid unnecessary fetches for old operations in all the clients.

Anyway, thats all to say, I propose we change the header to be either:

   Subscribe: linear
or Subscribe: dag

Or something like that, so the server can tell the client if it can ignore versions and blindly apply all the changes it sees, or if its expected to make & maintain a DAG of all the operations its seen to be able to interpret incoming updates.

@josephg
Copy link
Contributor

josephg commented Mar 2, 2021

The idea of having this mode switch is being discussed in #92

@canadaduane
Copy link
Member Author

canadaduane commented Mar 8, 2021

Should Subscribe be an HTTP request method instead of a header of the GET method?

e.g.:

SUBSCRIBE /posts HTTP/1.1
Host: invisible.college:4545
User-Agent: curl/7.54.0
Accept: */*

Pros:

  • If we don't need a parameter on Subscribe: maybe it's more naturally expressed as a request method?
  • Its semantics can be clearly differentiated from a one-time "GET"
  • We don't need a lot of things that GET supports, such as caching, etags, etc. We can start from a "clean slate".
  • It stands out more--this may help distinguish what we're doing from other efforts

Cons:

  • A new HTTP request method may be more difficult to support on all libraries (e.g. closed set enums)
  • A new HTTP request method may interact poorly with proxies (?)

@josephg
Copy link
Contributor

josephg commented Mar 8, 2021

We talked to some people at IETF about this and they said politically it’s almost impossible to add new http methods. If it’s going to make the spec harder to ratify, I’m happy to just overload GET.

@toomim
Copy link
Member

toomim commented Apr 1, 2021

Empty headers are allowed in HTTP, although I agree they are ugly:

Curl has a bug report about this:

I do also see the potential merit of creating a separate method, even though I think there are other reasons to keep this with GET that will become more apparent over time as we develop programming APIs on Braid. Overall, I currently lean towards keeping it with GET.

However, I want us to make the best decision, and so let me give a little more ammo for the other side. I believe I was present in the IETF conversation Seph is referring to, but FWIW my memory was people saying that it is much harder to create a new method than a new header— not that it is "almost impossible" to add new HTTP methods. So if we do conclude that a new method would be best, I think it would be worthwhile to propose it to the HTTPWG and let them shoot it down, rather than close the possibility before giving it a shot.

But I think we'll need to explore programming APIs on Braid, and flesh out the different types of subscriptions (e.g. automatically-expiring subscriptions, etc.) before we can make a good final decision here.

@mitar
Copy link
Member

mitar commented Apr 11, 2022

You could define a new method and then one could use almost standard X-Method-Override header to pass it until Braid becomes so popular that everyone implements it.

@toomim
Copy link
Member

toomim commented Apr 11, 2022

Good point. Also, the SUBSCRIBE HTTP method is already defined in some webdav implementations: https://docs.microsoft.com/en-us/previous-versions/office/developer/exchange-server-2003/aa143117(v=exchg.65); for instance it already exists in the nodejs http module:

$ node
Welcome to Node.js v15.9.0.
Type ".help" for more information.
> require('http').METHODS
[
  'ACL',        'BIND',        'CHECKOUT',
  'CONNECT',    'COPY',        'DELETE',
  'GET',        'HEAD',        'LINK',
  'LOCK',       'M-SEARCH',    'MERGE',
  'MKACTIVITY', 'MKCALENDAR',  'MKCOL',
  'MOVE',       'NOTIFY',      'OPTIONS',
  'PATCH',      'POST',        'PRI',
  'PROPFIND',   'PROPPATCH',   'PURGE',
  'PUT',        'REBIND',      'REPORT',
  'SEARCH',     'SOURCE',      'SUBSCRIBE',
  'TRACE',      'UNBIND',      'UNLINK',
  'UNLOCK',     'UNSUBSCRIBE'
]
> 

If we want SUBSCRIBE, we might do a little research into these existing systems, to bring them along for the ride, or at least not break them.

@toomim
Copy link
Member

toomim commented Aug 23, 2023

Closing this issue; discussion of subscription parameters is moving to #123.

@toomim toomim closed this as completed Aug 23, 2023
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

4 participants