From b52bfb77e3ed5fdaea8396718f895aaa3273284d Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 16:27:50 +0400 Subject: [PATCH 1/9] feat: make max_part_size customizable. Addresses issue reported in #2785 --- starlette/formparsers.py | 7 +++--- starlette/requests.py | 24 +++++++++++++++++--- tests/test_formparsers.py | 48 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/starlette/formparsers.py b/starlette/formparsers.py index 82e2ad195..bfcd1f473 100644 --- a/starlette/formparsers.py +++ b/starlette/formparsers.py @@ -122,9 +122,6 @@ async def parse(self) -> FormData: class MultiPartParser: - max_file_size = 1024 * 1024 # 1MB - max_part_size = 1024 * 1024 # 1MB - def __init__( self, headers: Headers, @@ -132,6 +129,8 @@ def __init__( *, max_files: int | float = 1000, max_fields: int | float = 1000, + max_file_size: int = 1024 * 1024, # 1MB + max_part_size: int | float = 1024 * 1024, # 1MB ) -> None: assert multipart is not None, "The `python-multipart` library must be installed to use form parsing." self.headers = headers @@ -148,6 +147,8 @@ def __init__( self._file_parts_to_write: list[tuple[MultipartPart, bytes]] = [] self._file_parts_to_finish: list[MultipartPart] = [] self._files_to_close_on_error: list[SpooledTemporaryFile[bytes]] = [] + self.max_file_size = max_file_size + self.max_part_size = max_part_size def on_part_begin(self) -> None: self._current_part = MultipartPart() diff --git a/starlette/requests.py b/starlette/requests.py index a29164d0d..1c9da85f2 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -249,7 +249,14 @@ async def json(self) -> typing.Any: self._json = json.loads(body) return self._json - async def _get_form(self, *, max_files: int | float = 1000, max_fields: int | float = 1000) -> FormData: + async def _get_form( + self, + *, + max_files: int | float = 1000, + max_fields: int | float = 1000, + max_file_size: int = 1024 * 1024, + max_part_size: int | float = 1024 * 1024, + ) -> FormData: if self._form is None: assert ( parse_options_header is not None @@ -264,6 +271,8 @@ async def _get_form(self, *, max_files: int | float = 1000, max_fields: int | fl self.stream(), max_files=max_files, max_fields=max_fields, + max_file_size=max_file_size, + max_part_size=max_part_size, ) self._form = await multipart_parser.parse() except MultiPartException as exc: @@ -278,9 +287,18 @@ async def _get_form(self, *, max_files: int | float = 1000, max_fields: int | fl return self._form def form( - self, *, max_files: int | float = 1000, max_fields: int | float = 1000 + self, + *, + max_files: int | float = 1000, + max_fields: int | float = 1000, + max_file_size: int = 1024 * 1024, + max_part_size: int | float = 1024 * 1024, ) -> AwaitableOrContextManager[FormData]: - return AwaitableOrContextManagerWrapper(self._get_form(max_files=max_files, max_fields=max_fields)) + return AwaitableOrContextManagerWrapper( + self._get_form( + max_files=max_files, max_fields=max_fields, max_file_size=max_file_size, max_part_size=max_part_size + ) + ) async def close(self) -> None: if self._form is not None: # pragma: no branch diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index 64efdbdbc..a7e4352bd 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -104,10 +104,14 @@ async def app_read_body(scope: Scope, receive: Receive, send: Send) -> None: await response(scope, receive, send) -def make_app_max_parts(max_files: int = 1000, max_fields: int = 1000) -> ASGIApp: +def make_app_max_parts( + max_files: int = 1000, max_fields: int = 1000, max_file_size: int = 1024 * 1024, max_part_size: int = 1024 * 1024 +) -> ASGIApp: async def app(scope: Scope, receive: Receive, send: Send) -> None: request = Request(scope, receive) - data = await request.form(max_files=max_files, max_fields=max_fields) + data = await request.form( + max_files=max_files, max_fields=max_fields, max_file_size=max_file_size, max_part_size=max_part_size + ) output: dict[str, typing.Any] = {} for key, value in data.items(): if isinstance(value, UploadFile): @@ -699,3 +703,43 @@ def test_max_part_size_exceeds_limit( response = client.post("/", data=multipart_data, headers=headers) # type: ignore assert response.status_code == 400 assert response.text == "Part exceeded maximum size of 1024KB." + + +@pytest.mark.parametrize( + "app,expectation", + [ + (make_app_max_parts(max_part_size=1024 * 10), pytest.raises(MultiPartException)), + ( + Starlette(routes=[Mount("/", app=make_app_max_parts(max_part_size=1024 * 10))]), + does_not_raise(), + ), + ], +) +def test_max_part_size_exceeds_custom_limit( + app: ASGIApp, + expectation: typing.ContextManager[Exception], + test_client_factory: TestClientFactory, +) -> None: + client = test_client_factory(app) + boundary = "------------------------4K1ON9fZkj9uCUmqLHRbbR" + + multipart_data = ( + f"--{boundary}\r\n" + f'Content-Disposition: form-data; name="small"\r\n\r\n' + "small content\r\n" + f"--{boundary}\r\n" + f'Content-Disposition: form-data; name="large"\r\n\r\n' + + ("x" * 1024 * 10 + "x") # 1MB + 1 byte of data + + "\r\n" + f"--{boundary}--\r\n" + ).encode("utf-8") + + headers = { + "Content-Type": f"multipart/form-data; boundary={boundary}", + "Transfer-Encoding": "chunked", + } + + with expectation: + response = client.post("/", data=multipart_data, headers=headers) # type: ignore + assert response.status_code == 400 + assert response.text == "Part exceeded maximum size of 10KB." From daa8d1d5d3da62361f06ae2cb4e9ee03f71181ed Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 16:55:59 +0400 Subject: [PATCH 2/9] revert: make only `max_part_size` configurable. --- starlette/formparsers.py | 4 ++-- starlette/requests.py | 7 +------ tests/test_formparsers.py | 4 +--- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/starlette/formparsers.py b/starlette/formparsers.py index bfcd1f473..fc508765f 100644 --- a/starlette/formparsers.py +++ b/starlette/formparsers.py @@ -122,6 +122,8 @@ async def parse(self) -> FormData: class MultiPartParser: + max_file_size = 1024 * 1024 # 1MB + def __init__( self, headers: Headers, @@ -129,7 +131,6 @@ def __init__( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_file_size: int = 1024 * 1024, # 1MB max_part_size: int | float = 1024 * 1024, # 1MB ) -> None: assert multipart is not None, "The `python-multipart` library must be installed to use form parsing." @@ -147,7 +148,6 @@ def __init__( self._file_parts_to_write: list[tuple[MultipartPart, bytes]] = [] self._file_parts_to_finish: list[MultipartPart] = [] self._files_to_close_on_error: list[SpooledTemporaryFile[bytes]] = [] - self.max_file_size = max_file_size self.max_part_size = max_part_size def on_part_begin(self) -> None: diff --git a/starlette/requests.py b/starlette/requests.py index 1c9da85f2..19ecca350 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -254,7 +254,6 @@ async def _get_form( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_file_size: int = 1024 * 1024, max_part_size: int | float = 1024 * 1024, ) -> FormData: if self._form is None: @@ -271,7 +270,6 @@ async def _get_form( self.stream(), max_files=max_files, max_fields=max_fields, - max_file_size=max_file_size, max_part_size=max_part_size, ) self._form = await multipart_parser.parse() @@ -291,13 +289,10 @@ def form( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_file_size: int = 1024 * 1024, max_part_size: int | float = 1024 * 1024, ) -> AwaitableOrContextManager[FormData]: return AwaitableOrContextManagerWrapper( - self._get_form( - max_files=max_files, max_fields=max_fields, max_file_size=max_file_size, max_part_size=max_part_size - ) + self._get_form(max_files=max_files, max_fields=max_fields, max_part_size=max_part_size) ) async def close(self) -> None: diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index a7e4352bd..219c7e0d2 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -109,9 +109,7 @@ def make_app_max_parts( ) -> ASGIApp: async def app(scope: Scope, receive: Receive, send: Send) -> None: request = Request(scope, receive) - data = await request.form( - max_files=max_files, max_fields=max_fields, max_file_size=max_file_size, max_part_size=max_part_size - ) + data = await request.form(max_files=max_files, max_fields=max_fields, max_part_size=max_part_size) output: dict[str, typing.Any] = {} for key, value in data.items(): if isinstance(value, UploadFile): From cd66fe30ed5e4dbcc636a9a1a37ab6290bd20cf3 Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:01:36 +0400 Subject: [PATCH 3/9] docs: add docs to explain new parameter --- docs/requests.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/requests.md b/docs/requests.md index 9140bb964..368755f13 100644 --- a/docs/requests.md +++ b/docs/requests.md @@ -113,12 +113,12 @@ state with `disconnected = await request.is_disconnected()`. Request files are normally sent as multipart form data (`multipart/form-data`). -Signature: `request.form(max_files=1000, max_fields=1000)` +Signature: `request.form(max_files=1000, max_fields=1000, max_part_size=1024*1024)` -You can configure the number of maximum fields or files with the parameters `max_files` and `max_fields`: +You can configure the number of maximum fields or files with the parameters `max_files` and `max_fields`; and part size using `max_part_size`: ```python -async with request.form(max_files=1000, max_fields=1000): +async with request.form(max_files=1000, max_fields=1000, max_part_size=1024*1024): ... ``` From 2952aa8cc1db7a325c97177160be5c22b73b4d0e Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:34:01 +0400 Subject: [PATCH 4/9] refactor: remove `float` type --- starlette/formparsers.py | 2 +- starlette/requests.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/starlette/formparsers.py b/starlette/formparsers.py index fc508765f..87d262c1d 100644 --- a/starlette/formparsers.py +++ b/starlette/formparsers.py @@ -131,7 +131,7 @@ def __init__( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_part_size: int | float = 1024 * 1024, # 1MB + max_part_size: int = 1024 * 1024, # 1MB ) -> None: assert multipart is not None, "The `python-multipart` library must be installed to use form parsing." self.headers = headers diff --git a/starlette/requests.py b/starlette/requests.py index 19ecca350..16b60a2a1 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -254,7 +254,7 @@ async def _get_form( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_part_size: int | float = 1024 * 1024, + max_part_size: int = 1024 * 1024, ) -> FormData: if self._form is None: assert ( @@ -289,7 +289,7 @@ def form( *, max_files: int | float = 1000, max_fields: int | float = 1000, - max_part_size: int | float = 1024 * 1024, + max_part_size: int = 1024 * 1024, ) -> AwaitableOrContextManager[FormData]: return AwaitableOrContextManagerWrapper( self._get_form(max_files=max_files, max_fields=max_fields, max_part_size=max_part_size) From 143d9d43e58257a1b7968562897e437e996bf061 Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:36:04 +0400 Subject: [PATCH 5/9] refactor: fix type for other paramters in `MultiPart` --- starlette/formparsers.py | 6 +++--- starlette/requests.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/starlette/formparsers.py b/starlette/formparsers.py index 87d262c1d..097523747 100644 --- a/starlette/formparsers.py +++ b/starlette/formparsers.py @@ -122,15 +122,15 @@ async def parse(self) -> FormData: class MultiPartParser: - max_file_size = 1024 * 1024 # 1MB + max_file_size: int = 1024 * 1024 # 1MB def __init__( self, headers: Headers, stream: typing.AsyncGenerator[bytes, None], *, - max_files: int | float = 1000, - max_fields: int | float = 1000, + max_files: int = 1000, + max_fields: int = 1000, max_part_size: int = 1024 * 1024, # 1MB ) -> None: assert multipart is not None, "The `python-multipart` library must be installed to use form parsing." diff --git a/starlette/requests.py b/starlette/requests.py index 16b60a2a1..9fcd40d21 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -252,8 +252,8 @@ async def json(self) -> typing.Any: async def _get_form( self, *, - max_files: int | float = 1000, - max_fields: int | float = 1000, + max_files: int = 1000, + max_fields: int = 1000, max_part_size: int = 1024 * 1024, ) -> FormData: if self._form is None: @@ -287,8 +287,8 @@ async def _get_form( def form( self, *, - max_files: int | float = 1000, - max_fields: int | float = 1000, + max_files: int = 1000, + max_fields: int = 1000, max_part_size: int = 1024 * 1024, ) -> AwaitableOrContextManager[FormData]: return AwaitableOrContextManagerWrapper( From 7b869bbe01bfc0804c795ccbbcb6d786655ec16e Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:44:39 +0400 Subject: [PATCH 6/9] Revert "refactor: fix type for other paramters in `MultiPart`" This reverts commit 143d9d43e58257a1b7968562897e437e996bf061. --- starlette/formparsers.py | 6 +++--- starlette/requests.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/starlette/formparsers.py b/starlette/formparsers.py index 097523747..87d262c1d 100644 --- a/starlette/formparsers.py +++ b/starlette/formparsers.py @@ -122,15 +122,15 @@ async def parse(self) -> FormData: class MultiPartParser: - max_file_size: int = 1024 * 1024 # 1MB + max_file_size = 1024 * 1024 # 1MB def __init__( self, headers: Headers, stream: typing.AsyncGenerator[bytes, None], *, - max_files: int = 1000, - max_fields: int = 1000, + max_files: int | float = 1000, + max_fields: int | float = 1000, max_part_size: int = 1024 * 1024, # 1MB ) -> None: assert multipart is not None, "The `python-multipart` library must be installed to use form parsing." diff --git a/starlette/requests.py b/starlette/requests.py index 9fcd40d21..16b60a2a1 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -252,8 +252,8 @@ async def json(self) -> typing.Any: async def _get_form( self, *, - max_files: int = 1000, - max_fields: int = 1000, + max_files: int | float = 1000, + max_fields: int | float = 1000, max_part_size: int = 1024 * 1024, ) -> FormData: if self._form is None: @@ -287,8 +287,8 @@ async def _get_form( def form( self, *, - max_files: int = 1000, - max_fields: int = 1000, + max_files: int | float = 1000, + max_fields: int | float = 1000, max_part_size: int = 1024 * 1024, ) -> AwaitableOrContextManager[FormData]: return AwaitableOrContextManagerWrapper( From ded09ca5973c86bb6ff785d62aced4fc5b831145 Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:46:39 +0400 Subject: [PATCH 7/9] Update tests/test_formparsers.py --- tests/test_formparsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index 219c7e0d2..81842ec46 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -105,7 +105,7 @@ async def app_read_body(scope: Scope, receive: Receive, send: Send) -> None: def make_app_max_parts( - max_files: int = 1000, max_fields: int = 1000, max_file_size: int = 1024 * 1024, max_part_size: int = 1024 * 1024 + max_files: int = 1000, max_fields: int = 1000, max_part_size: int = 1024 * 1024 ) -> ASGIApp: async def app(scope: Scope, receive: Receive, send: Send) -> None: request = Request(scope, receive) From c4136e6a0ccef2e63e9fed577d2721bd84b600e1 Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Thu, 26 Dec 2024 17:49:27 +0400 Subject: [PATCH 8/9] style: lint --- tests/test_formparsers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index 81842ec46..97d83ad60 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -104,9 +104,7 @@ async def app_read_body(scope: Scope, receive: Receive, send: Send) -> None: await response(scope, receive, send) -def make_app_max_parts( - max_files: int = 1000, max_fields: int = 1000, max_part_size: int = 1024 * 1024 -) -> ASGIApp: +def make_app_max_parts(max_files: int = 1000, max_fields: int = 1000, max_part_size: int = 1024 * 1024) -> ASGIApp: async def app(scope: Scope, receive: Receive, send: Send) -> None: request = Request(scope, receive) data = await request.form(max_files=max_files, max_fields=max_fields, max_part_size=max_part_size) From 70fdf28a0908056b1fe4004d450590d860992630 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Sat, 28 Dec 2024 05:54:12 +0100 Subject: [PATCH 9/9] Use content instead of data --- tests/test_formparsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index 97d83ad60..f781a9ecb 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -736,6 +736,6 @@ def test_max_part_size_exceeds_custom_limit( } with expectation: - response = client.post("/", data=multipart_data, headers=headers) # type: ignore + response = client.post("/", content=multipart_data, headers=headers) assert response.status_code == 400 assert response.text == "Part exceeded maximum size of 10KB."