cryostat-legacy icon indicating copy to clipboard operation
cryostat-legacy copied to clipboard

[Story] RequestHandlers should implement "produces" for Content-Type negotiation

Open andrewazores opened this issue 2 years ago • 0 comments

Should I be supporting multiple Accept headers like Accept: text/html;application/json;*/* ? I was originally going to but I realized since this is going to be an internally used endpoint, there wouldn't be much reason to if we just document that you must have at most 1 Accept header to each request.

How it is right now, if there are mutliple, 406 is thrown since we are looking at the raw acceptHeader.

And I notice that when I press "View Report" on an ActiveRecording, it will send add an AcceptHeader like text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8 because I'm guessing the default behaviour of Accept headers in the downloadReport function in the cryostat-web Api.service.

What should happen here?

1. Support multiple headers using `ctx.parsedHeaders().accept()` and do a bunch of manual parsing by checking q values and having an implicit order #(text/html always priority over application/json (or something))` (I don't think vert.x has a way of doing this easily)

2. Change the way the downloadReport function works with headers only.

3. something else...?

I think Vert.x does support this:

https://vertx.io/docs/vertx-web/java/#_routing_based_on_mime_types_acceptable_by_the_client

but we would need to set up the router in WebServer to use .produces() on route definitions, so that's out of scope for this particular PR. Ideally we do fully support this, but that's a larger change to make later on.

I'd rather not have requests to this endpoint outright fail if multiple Accepts are given, or if Accept has multiple listed content types. What do you think of the following behaviour?

  1. Check the Accept header(s)
  2. If there is exactly one, and its value is a known supported type, respond with that
  3. In any other scenario, respond with HTML

This way if the client didn't provide an Accept, or provided an invalid value, or provided multiple, or provided the weighted list, we just respond with a default HTML formatted doc (for now). This suits our own frontend's needs if we can set the Accept header there manually/explicitly to only HTML or only JSON, and also allows some reasonable degree of flexibility as a public API with particular constraints. If we have time to come back around and properly support content type routing then we can fix this endpoint up to fully support the content type negotiation semantics and Accept header values.

Originally posted by @andrewazores in https://github.com/cryostatio/cryostat/issues/1009#issuecomment-1175053670

andrewazores avatar Jul 05 '22 20:07 andrewazores