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

♻ Assume request bodies contain JSON when no Content-Type header is provided #3456

Merged
merged 2 commits into from Jul 3, 2021

Conversation

tiangolo
Copy link
Owner

@tiangolo tiangolo commented Jul 3, 2021

♻ Assume request bodies contain JSON when no Content-Type header is provided

This is related to #2118 and the CVE GHSA-8h2j-cgx8-6xv7 about CSRF.

Some clients already assume that request bodies are read as JSON, even when no content-type header is set: #2118 (comment)

And the risk of CSRF only applies for specific Content-Type headers, so, no Content-Type header is already protected from CORS preflights: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests

as that doesn't risk CSRF problems that would always have headers, and because many clients already depend on that default behavior
@codecov
Copy link

codecov bot commented Jul 3, 2021

Codecov Report

Merging #3456 (540f19f) into master (00a8420) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##            master     #3456    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          226       248    +22     
  Lines         6783      7717   +934     
==========================================
+ Hits          6783      7717   +934     
Impacted Files Coverage Δ
tests/main.py 100.00% <0.00%> (ø)
fastapi/utils.py 100.00% <0.00%> (ø)
fastapi/params.py 100.00% <0.00%> (ø)
fastapi/routing.py 100.00% <0.00%> (ø)
fastapi/encoders.py 100.00% <0.00%> (ø)
fastapi/requests.py 100.00% <0.00%> (ø)
fastapi/responses.py 100.00% <0.00%> (ø)
fastapi/exceptions.py 100.00% <0.00%> (ø)
fastapi/concurrency.py 100.00% <0.00%> (ø)
fastapi/applications.py 100.00% <0.00%> (ø)
... and 69 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eb17fc...540f19f. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2021

@tiangolo tiangolo merged commit edf6b2d into master Jul 3, 2021
@tiangolo tiangolo deleted the assume-json branch July 3, 2021 16:25
@LironSherk
Copy link

LironSherk commented Jul 6, 2021

Hi,
I'm sending a request with no Content-Type header but got the same error before this fix.
when I put a breakpoint on the line
if not content_type_value

it equals: 'text/plain;charset=UTF-8'

in node-fetch, if developer didn't provide Content-Type the default is : 'text/plain;charset=UTF-8'

so all node servers that sending JSON requests to fastapi without providing Content-Type will fail.

@falkben
Copy link
Sponsor Contributor

falkben commented Jul 6, 2021

Hi,
I'm sending a request with no Content-Type header but got the same error before this fix.
when I put a breakpoint on the line
if not content_type_value

it equals: 'text/plain;charset=UTF-8'

in node-fetch, if developer didn't provide Content-Type the default is : 'text/plain;charset=UTF-8'

so all node servers that sending JSON requests to fastapi without providing Content-Type will fail.

I think you should either change the default you are sending w/ node-fetch or make sure the content-type is specified on every request. Assuming json when the content type was sent as text/plain is not correct. CORS specifically checks text/plain, so to do so we'd be back in the same situation as GHSA-8h2j-cgx8-6xv7.

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

3 participants