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

Re-write request Content-Type when decoding #24

Merged
merged 2 commits into from Oct 26, 2021

Conversation

florimondmanca
Copy link
Owner

@florimondmanca florimondmanca commented Oct 25, 2021

Refs #23

This pull request makes it so that applications see Content-Type: application/json (instead of Content-Type: application/x-msgpack) in requests.

The motivation is to make content and Content-Type consistent from the point of view of the application, which may do additional consistency checks, eg for security purposes. FastAPI 0.65.2+ has such a check that prevents a CSRF vulnerability when a client sends JSON data with text/plain, which is exempted from CSRF checks.

Still pondering, but I think this might be an acceptable, perhaps necessary option. The idea behind msgpack-asgi is to serve as a "msgpack-to/from-JSON gateway" afterall.

cc @einfachTobi — I'd be happy to hear what you think about this. :-)

@einfachTobi
Copy link

I think it's the best approach. You never can tell, what any application will do with the request. I'm sure changing the content-type after decoding gives best compatibility with any application. At least this is also the consequent way because technically the content-type is application/json after passing the middleware.

@florimondmanca
Copy link
Owner Author

OK, let's roll.

@florimondmanca florimondmanca merged commit 9172176 into master Oct 26, 2021
@florimondmanca florimondmanca deleted the fm/json-content-type branch October 26, 2021 09:44
@florimondmanca florimondmanca mentioned this pull request Oct 26, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants