Skip to content

Commit

Permalink
Fix reading FastAPI request body twice. (#1724)
Browse files Browse the repository at this point in the history
Starlette/FastAPI is internally caching the request body if read via request.json() or request.body() but NOT when using request.form(). This leads to a problem when our Sentry Starlette integration wants to read the body data and also the users code wants to read the same data.

Solution:
Force caching of request body for .form() calls too, to prevent error when body is read twice.

The tests where mocking .stream() and thus hiding this problem. So the tests have been refactored to mock the underlying ._receive() function instead.

Co-authored-by: hasier <hasier@users.noreply.github.com>
  • Loading branch information
antonpirker and hasier committed Nov 10, 2022
1 parent 0923d03 commit f222c9d
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 160 deletions.
98 changes: 45 additions & 53 deletions sentry_sdk/integrations/starlette.py
Expand Up @@ -22,7 +22,7 @@
)

if MYPY:
from typing import Any, Awaitable, Callable, Dict, Optional, Union
from typing import Any, Awaitable, Callable, Dict, Optional

from sentry_sdk._types import Event

Expand Down Expand Up @@ -367,10 +367,10 @@ def _make_request_event_processor(req, integration):
def event_processor(event, hint):
# type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any]

# Extract information from request
# Add info from request to event
request_info = event.get("request", {})
if info:
if "cookies" in info and _should_send_default_pii():
if "cookies" in info:
request_info["cookies"] = info["cookies"]
if "data" in info:
request_info["data"] = info["data"]
Expand Down Expand Up @@ -473,30 +473,46 @@ async def extract_request_info(self):
request_info = {} # type: Dict[str, Any]

with capture_internal_exceptions():
# Add cookies
if _should_send_default_pii():
request_info["cookies"] = self.cookies()

# If there is no body, just return the cookies
content_length = await self.content_length()

if content_length:
data = None # type: Union[Dict[str, Any], AnnotatedValue, None]

if not request_body_within_bounds(client, content_length):
data = AnnotatedValue.removed_because_over_size_limit()

else:
parsed_body = await self.parsed_body()
if parsed_body is not None:
data = parsed_body
elif await self.raw_data():
data = AnnotatedValue.removed_because_raw_data()
else:
data = None

if data is not None:
request_info["data"] = data

return request_info
if not content_length:
return request_info

# Add annotation if body is too big
if content_length and not request_body_within_bounds(
client, content_length
):
request_info["data"] = AnnotatedValue.removed_because_over_size_limit()
return request_info

# Add JSON body, if it is a JSON request
json = await self.json()
if json:
request_info["data"] = json
return request_info

# Add form as key/value pairs, if request has form data
form = await self.form()
if form:
form_data = {}
for key, val in iteritems(form):
is_file = isinstance(val, UploadFile)
form_data[key] = (
val
if not is_file
else AnnotatedValue.removed_because_raw_data()
)

request_info["data"] = form_data
return request_info

# Raw data, do not add body just an annotation
request_info["data"] = AnnotatedValue.removed_because_raw_data()
return request_info

async def content_length(self):
# type: (StarletteRequestExtractor) -> Optional[int]
Expand All @@ -509,19 +525,17 @@ def cookies(self):
# type: (StarletteRequestExtractor) -> Dict[str, Any]
return self.request.cookies

async def raw_data(self):
# type: (StarletteRequestExtractor) -> Any
return await self.request.body()

async def form(self):
# type: (StarletteRequestExtractor) -> Any
"""
curl -X POST http://localhost:8000/upload/somethign -H "Content-Type: application/x-www-form-urlencoded" -d "username=kevin&password=welcome123"
curl -X POST http://localhost:8000/upload/somethign -F username=Julian -F password=hello123
"""
if multipart is None:
return None

# Parse the body first to get it cached, as Starlette does not cache form() as it
# does with body() and json() https://github.com/encode/starlette/discussions/1933
# Calling `.form()` without calling `.body()` first will
# potentially break the users project.
await self.request.body()

return await self.request.form()

def is_json(self):
Expand All @@ -530,33 +544,11 @@ def is_json(self):

async def json(self):
# type: (StarletteRequestExtractor) -> Optional[Dict[str, Any]]
"""
curl -X POST localhost:8000/upload/something -H 'Content-Type: application/json' -d '{"login":"my_login","password":"my_password"}'
"""
if not self.is_json():
return None

return await self.request.json()

async def parsed_body(self):
# type: (StarletteRequestExtractor) -> Any
"""
curl -X POST http://localhost:8000/upload/somethign -F username=Julian -F password=hello123 -F photo=@photo.jpg
"""
form = await self.form()
if form:
data = {}
for key, val in iteritems(form):
if isinstance(val, UploadFile):
data[key] = AnnotatedValue.removed_because_raw_data()
else:
data[key] = val

return data

json_data = await self.json()
return json_data


def _set_transaction_name_and_source(event, transaction_style, request):
# type: (Event, str, Any) -> None
Expand Down

1 comment on commit f222c9d

@vercel
Copy link

@vercel vercel bot commented on f222c9d Nov 10, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.