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.

Sign up for GitHub

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

Jump to bottom

Add RFC 006: Format Negotiation Protocol #115

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add RFC 006: Format Negotiation Protocol #115

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b8ced49
Add RFC 006: Format Negotiation Protocol
Oct 4, 2018
9bc2218
code review
Oct 4, 2018
82df676
allow hyphen
Oct 4, 2018
f4bffe6
use conjure=<version> instead of v=<version>
Oct 8, 2018
cd241b0
typo
Oct 8, 2018
e338def
add default version
Oct 8, 2018
2724321
rfc review adjustments
Oct 17, 2018
84c3e14
add example for binary
Oct 17, 2018
d37ec2d
clarify complex negotiation scope
Oct 19, 2018
580a84d
deutsch
Oct 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 38 additions & 10 deletions docs/rfc/006-format-negotiation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ We have the following requirements for the format negotiation protocol:

### Definitions

- A *version* is a string of the form `x.y` where x and y are positive integers
- A *Conjure format identifier* is a string of the `application/<format>+conjure; v=<version>` where `<format>` is a
non-empty string over `[a-z]` and `<version>` is a version string (as above)
- A *version* is a non-zero integer
- A *Conjure format identifier* is a string of the `application/<format>; v=<version>` where `<format>` is a
Copy link
Contributor

@jkozlowski jkozlowski Oct 5, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering if we could maybe use "conjure_version" or "conjure_protocol" instead of just "v"? Or is this a standardized version parameter (I didn't read the standard). It could be more discoverable that way if someone bumps into this in the wild.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meant to change this to conjure=<version> as per above comment. pushed now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you push? I don't see the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now I did

non-empty string over `[a-z]` (e.g., `json`) and `<version>` is a version string (as above)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include hyphen in the format specification. application/x-jackson-smile for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

- A *Conjure format list* is a comma-separated, ordered list of Conjure format identifiers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use q (quality factory), according to RFC2616, instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think the version is related to quality?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As using it to describe the priority order of the conjure format identifiers instead of assuming that this list is ordered, which the spec doesn't explicitly call out?

But according to this, order of the list doesn't seem to matter if no q being defined.

If there is no priority defined for the first two values, the order in the list is irrelevant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I've seen the quality parameter used in practice, most implementations I've worked with use order.

If the quality of types matches we would use order to break ties. Given that, I don't see any benefit that the quality parameter provides.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it technically harder to support the quality factor? If not, then regardless of other people's use of it I would argue it's probably beneficial to stick to the advertised standard, even if not just for discoverability (e.g. 'search-engine of choice'ing Accept header format will get you a generic description from Mozilla which matches our use, rather than something which is subtly different, being close enough you will think you know how it works, but actually don't).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it means we'd eat an extra parse and sort pass for each request. probably doesn't matter much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd just cache results; doesn't matter at all :)

- The `Accept` and `Content-Type` HTTP headers are defined as per the
[HTTP 1.1 spec](https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html). Note that `Accept: <format list>` is a
Expand All @@ -25,7 +25,7 @@ We have the following requirements for the format negotiation protocol:
### Wire format versioning

We propose that every revision of the Conjure wire format be labelled with a format identifier.
The current PLAIN+JSON format shall be labelled `application/json+conjure; v=1.0`.
The current PLAIN+JSON format shall be labelled `application/json; v=1`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is this versioning? JSON bit is covered by application/json, so really this is kinda conjure DSL (IR?) format version, not "PLAIN+JSON" format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if PLAIN should be considered part of the format here.
It only refers to the encoding of parameters NOT in the body (headers, query and path params), things that are orthogonal to whatever the content-type is.

@jkozlowski I think it's fine to say it's application/json since every response is JSON compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should version the PLAIN and the structured format together.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we do want that. It just seemed weird to requisition content-type for the format of things outside the body, but can't think of a better place that would explicitly cover the format of custom headers etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also probably just put down that "application/json" is also fine for backcompat.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added.


### Format capabilities

Expand All @@ -38,12 +38,13 @@ We assume that clients and servers implement both serialization and deserializat

**Requests.**
Clients send the format identifier used to encode the request as a Content-Type header, and a format list as an Accept
header. The format indicates the preference-ordered list of formats that the client supports.
header. The format indicates the preference-ordered list of formats that the client supports. The supported formats
must include the format use to encode the request, i.e., the format specified in the Content-Type request header.

**Responses.**
Servers that do not support the request format respond with a suitable error code. Otherwise, they use the
most-preferred (as per Accept request header) format to encode the response and advertise the chosen format in the
response Content-Type header.
Servers that do not support the request format respond with Conjure error UNSUPPORTED/415. Otherwise, if the server
does supported the request format, it uses the most-preferred (as per Accept request header) format to encode the
Copy link
Contributor

@jkozlowski jkozlowski Oct 5, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "does support the"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

response and advertise the chosen format in the response Content-Type header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this means I can send "application/json" in my first request, but actually put CBOR as my preference, so I'll get CBOR request? Neat, and then I can followup all my other ones with CBOR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... you'd get a CBOR response, yes.


Every response (including non-success responses) must send a preference-ordered format list of supported formats as
Accept response header. (Note that this is a non-standard header for HTTP 1.1 responses.)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth switching from non-standard use of the Accept header to using the proposed Accept-Post response header (https://tools.ietf.org/id/draft-wilde-accept-post-00.html).

There is also some question around including the servers Accepted types on every response which I think encourages negotiation to potentially give rise to requests and responses both using different formats. If the response is not a 415 response then the response Content-Type be taken as the agreed upon type for all future communication.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for Accept-Post rather than misusing Accept.

We could leave this part off in favor of using the response Content-Type for simplicity for the time being. Failure case is a service which only returns binary or no content, preventing the client from upgrading to a preferred media type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The draft seems abandoned (GitHub repo dead), or at least it never made it into an RFC. So I think 'Accept' and 'Accept-Post' are equally 'non-standard'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think letting the server choose the format based on the preference list sent by the client should be roughly equivalent. Thoughts on that simpler protocol, @markelliot ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me; I think the server returning what it supports is a nice-to-have. If we find a lot of problems in this space. There's an interesting thing here around what content type should be used for errors, I'd propose all errors are sent as JSON format, and thus we can spec the details of server support into the error body as Conjure error params, if we want.

Expand All @@ -55,8 +56,35 @@ responses with non-error responses.

At the beginning of a session, clients have no knowledge of the list of formats supported by the server. Clients may
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this negotiation done per IP? So if I have many nodes of my service, each of them could potentially be talking different wire formats?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the case of a blue/green deploy behind a proxy? Does Conjure version become part of rolling-upgrade state (as in, the server cannot advertise the new conjure protocol until all nodes have upgraded)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to explicitly handle this complexity. Clients receiving a 415 error can choose to retry the request with a more compatible format.

update the list of formats supported by a given server or service as per the formats advertised in the Accept headers of
responses in the session. To bootstrap the negotiation, client shall assume that servers support a "recent" variant of
the `application/json+conjure` format and thus choose the latest supported format from this family.
responses in the session. Typically, a client will pick its most preferred format that is advertised by the server
in the previous response of the session. To bootstrap the negotiation, clients shall assume that servers support the
most recent variant of the `application/json` format known to the client, and use this version for the first request of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that works, but maybe it's not so bad to always send application/json; v=1 on the first request than to fail the request because you jumped the gun on upgrading your client before everyone else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both are OK.

every session.

**Example.** The following sequence of two requests and corresponding responses are between a client that supports
CBOR v=2, CBOR v=1, and JSON v=1, and that prefers formats in that order, and a server that supports CBOR v=1 and
JSON v=1. To bootstrap the session, the client encodes the first request with the latest known JSON format, v=1. The
servers encodes the response with the format most preferred by the client that it also supports itself, CBOR v=1. The
second request is encoded with the format most preferred by the client that the server supports, CBOR v=1.


```text
Client ---------> Server
Content-Type: application/json; v=1
Accept: application/cbor; v=2, application/cbor; v=1, application/json; v=1

Client <--------- Server
Content-Type: application/cbor; v=1
Accept: application/cbor; v=1, application/json; v=1

Client ---------> Server
Content-Type: application/cbor; v=1
Accept: application/cbor; v=2, application/cbor; v=1, application/json; v=1

Client <--------- Server
Content-Type: application/cbor; v=1
Accept: application/cbor; v=1, application/json; v=1
```

In the rare case that server does not support the bootstrap format, the error response will carry a list of supported
formats (see above) from which the client can choose when retrying the request.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we think errors will indeed be rare then we could always fallback to use of an additional OPTIONS request to find out what the acceptable kinds of Content-Type are if we wanted to adhere to HTTP/1.1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, that's also an option