foca icon indicating copy to clipboard operation
foca copied to clipboard

Check for content types accepted by client

Open uniqueg opened this issue 5 years ago • 16 comments

API endpoints can serve different content types, e.g., application/json, text/plain etc. Clients can ask a service to respond with one or more specific accepted content types.

It would be nice if FOCA implemented a solution (e.g., a decorator) that compares the client's desired content type(s) against the ones that the service offers for the respective endpoint (as defined in the API specs). An appropriate error response should be returned if a client requests a content type that the endpoint cannot deliver. If the client does not explicitly ask for a specific content type (i.e., if the Accept: <content-type> header is missing), the default response should be given.

Other important points to think about:

uniqueg avatar Sep 17 '20 17:09 uniqueg

Can I take up this issue next?

vipulchhabra99 avatar Jan 31 '21 15:01 vipulchhabra99

@vipulchhabra99 are you working on this? Or may I please take the issue.

anuragxxd avatar Feb 17 '21 12:02 anuragxxd

Oh, I didn't see @vipulchhabra99's comment, sorry about that. From my side it's okay for either of you to take up that issue

uniqueg avatar Feb 17 '21 12:02 uniqueg

I am working on it.:)

anuragxxd avatar Feb 17 '21 13:02 anuragxxd

It's fine @uniqueg, I haven't started working on it.

vipulchhabra99 avatar Feb 17 '21 13:02 vipulchhabra99

@uniqueg In this we have to take user input for each specific route or the whole app. I think it's for the whole app.

anuragxxd avatar Feb 19 '21 09:02 anuragxxd

I'm not sure I understand your question, but the goal here would be to create a generic solution for a relatively common problem - content negotiation between client and server. If the client requests the response to be of one or more specified content types (via the Accept: <content-type> header), the server SHOULD return the response in one of the requested content types, if it can, or else return an appropriate error response, probably 406 Not Acceptable.

To implement that, I had thought of defining a decorator that handles the content negotation and that could be added to the controllers of every specific route. But I can see that at least in most cases (I currently can't think of examples that would not want to have content negotation activated, but cannot rule out that there are not) it would be useful to have this for all routes. But I'm not sure how to implement that, given that FOCA doesn't have direct access to the routes. If you find a way (e.g., via Connexion) to do that globally, for the entire app, that would be awesome! In that case, I think we should go for such a solution and only implement a route-based solution if anybody actually asks for that.

As a starting point, please have a look at the issues linked in the issue description to find out what Connexion already has supported, what the limitations are etc. There may be other relevant issues in Connexion as well. Please post any findings, with relevant links (ideally to definite information such as code, specs etc) in this issue so that we can review them together before deciding on an implementation strategy.

uniqueg avatar Feb 19 '21 10:02 uniqueg

In order to test if the connexion validation works with Accept: <content-type> header or not, we tried running DRS-filer. DRS-filer always sends an application/json response. In the request when Accept: application/zip was sent, the server was not throwing an error and giving a 200 - ok response.

Thus connexion validation is not working as expected.

This class is the place where response validation happens in connexion.

One solution is to create a similar class in foca and override it something like this

app = connexion.FlaskApp(__name__)
app.add_api('api.yaml', ..., validator_map=validator_map)

Ref: https://connexion.readthedocs.io/en/latest/response.html#custom-validator

This will ensure validation at an app-level rather than on an individual API endpoint level. Had some trouble understanding the code in the class, but if this is the right approach, we will dig deeper into this.

sarthakgupta072 avatar Apr 05 '21 13:04 sarthakgupta072

Thanks @sarthakgupta072. I think content negotiation/validation should always be performed, so it's fine to implement it at an app level.

But to see how (or rather where) to best implement it, I tnink a couple more tests would be useful to decide how to proceed:

For requests to a test endpoint, please set up those test cases:

  1. The Accept header contains at least one content type that is listed in the specs for that endpoint
  2. The Accept header does not contain any of the content types in the specs for that endpoint (desired behaviror: 406 Not Acceptable)
  3. The Accept header is not set (desired behaviror: 406 Not Acceptable)

The second set of tests are also for requests, but only if the request actually sends some data along (e.g., in a POST request): 4. The value of the Content-Type header is listed among the accepted content types in the specs for that endpoint 5. The value of the Content-Type header is not listed among the accepted content types in the specs for that endpoint (desired behavior: 415 Unsupported Media Type) 6. The Content-Type header is not set (desired behavior: 415 Unsupported Media Type)

For responses from a test endpoint, please set up those test cases (note that any errors here are the developer's fault, not the client's):

  1. The value of the Content-Type header is listed in the specs for that endpoint
  2. The value of the Content-Type header is not listed in the specs for that endpoint (should fail with a 500 Internal Server Error)
  3. The Content-Type header is not set (should fail with a 500 Internal Server Error)

Some more considerations:

  • For robustness and to check if Connexion has any special policies in place for application/json, try all test cases with content type application/json as well as at least one other content type.
  • To check if Connexion actually does some content type introspection or just passes on whatever passes on the content with whatever content type is declared, implement also a test case each where a (1) request or (2) response content type is declared (e.g., application/json) but content of another type (e.g., application/zip) is actually sent to / returned by the endpoint.

These tests also pretty much dictate how to implement content negotiation (and in which order):

  1. Check whether the client asks for a response content type that the server can actually provide
  2. Check whether any media sent along with the request, if any, is of a content type that the server can process
  3. Check whether the content type set for the response is compliant with the specs

If we have all of these tests in place, we know if and to what extent Connexion itself is handling any kind of content negotation / validation, and, based on that, we can decide whether we prefer to patch existing Connexion code or just write the FOCA code on top of Connexion. We can then also use those tests for test-driven development of the actual logic (which should actually be rather simple compared to coming up with all those tests). What's important eventually is that a list of content types accepted by the client and supported by the endpoint specs are passed to the endpoint so that the developer can decide which content type to deliver. The order of the content types in the Accept header should be preserved for that, as the usual behavior, I would assume, is that an endpoint returns content of the first accepted and supported type. However, as there may be other considerations in certain cases, it is important to retain other accepted and supported content types as well and thus give the developer a chance to implement more specific logic in the endpoint's controller.

Does this approach make sense to you?

uniqueg avatar Apr 05 '21 15:04 uniqueg

@uniqueg we actually tried only the basic test case. This approach definitely seems like worth trying. We'll get back to you with our further findings by the earliest.

kushagra189 avatar Apr 05 '21 16:04 kushagra189

@sarthakgupta072: any update on this? Just in case - I have removed the v1.0.0 milestone, as I think we should go forward with releasing that version even if that features doesn't make it in that release. Of course it's still a "nice-to-have" and there's still some time to get it in (writing the documentation and implementing/testing FOCA in at least one or two of the apps that don't currently fully use it will take a bit of time still)

uniqueg avatar May 02 '21 14:05 uniqueg

Hi @uniqueg, I was looking into connexion documentation. I found out that in their latest version, they are providing support for various content types which we can use directly to solve this issue.(link) We need to migrate to Connexion v3 for this.

Also @alohamora let me know when you are done with connexion migration to v3, I will implement this after that, or if you have not started working on migration, even I can start working on that. Do let me know what you prefer :)

Rahuljagwani avatar Jan 20 '24 20:01 Rahuljagwani

Fantastic - thanks a lot @Rahuljagwani - great find!

Yes, @alohamora, please tell @Rahuljagwani if he should go ahead with Connexion v3 migration.

uniqueg avatar Jan 22 '24 11:01 uniqueg

Also, @kushagra189 - please have a look here as well, as Connexion v3 migration also came up in our discussion. Might make sense to migrate first and then you could implement the type stuff from #144, #201 and #200? And @Rahuljagwani could take care of this one. What do you think?

uniqueg avatar Jan 22 '24 11:01 uniqueg

@Rahuljagwani @kushagra189 says it's okay, so please go ahead

uniqueg avatar Feb 05 '24 15:02 uniqueg

Currently blocked by #188

uniqueg avatar May 20 '24 10:05 uniqueg