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

Add support for new Starlette request.form() parameters max_files #9640

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wfranceys
Copy link

@wfranceys wfranceys commented Jun 8, 2023

Context:

  • Starlette added max_files to request.form() parameters and a default limit to limiting max files. See the changes here due to this advisory
  • This means when sending multipart-form requests to FastAPI with more than 1000 files will break, see the reported issue here
  • This PR adds an option to configure this setting through FastAPI

Changes:

  • Added an option max_files to the route decorators to set this
@app.post("/", max_files=2000)
def files(files: list[UploadFile]):
    return files

Tests:

  • Added a test to check the default behaviour remains the same and also checks that the new setting works

Questions and TODOs:

  • Is this the right approach? Is there a standard FastAPI pattern for setting this type of configuration?
  • Update docs if this is the right approach
  • Include max_fields option
  • Make test assert the error detail as well

@wfranceys wfranceys force-pushed the add-starlette-request-form-max-files branch from 5c25967 to c2680db Compare June 8, 2023 10:41
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

📝 Docs preview for commit c2680db at: https://6481b17137b13e10e928e328--fastapi.netlify.app

@wfranceys wfranceys force-pushed the add-starlette-request-form-max-files branch 2 times, most recently from dd5934e to 2d889ca Compare June 8, 2023 10:59
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

📝 Docs preview for commit 2d889ca at: https://6481b5a0e9cd8a1455412467--fastapi.netlify.app

files = [("files", ("foo.txt", "foo"))] * 1500
response = client.post("/route-with-default-limit", files=files)
# Fails with {'detail': 'Too many files. Maximum number of files is 1000.'}
assert response.status_code == 400

Choose a reason for hiding this comment

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

Why don't you assert against error details ? Like this is too broad.

Copy link
Author

@wfranceys wfranceys Jun 11, 2023

Choose a reason for hiding this comment

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

Sound sensible, will include! It is annoying to couple against the text in starlette

@adriangb
Copy link
Contributor

adriangb commented Jun 11, 2023

How about using annotated-types' MaxLen: https://github.com/annotated-types/annotated-types/#minlen-maxlen-len.

Usage would be async def endoint(files: Annotated[list[UploadFile], MaxLen(10)]) which is nicely analogous to async def endpoint(users: Annotated[list[User], MaxLen(10)])

@wfranceys
Copy link
Author

How about using annotated-types' MaxLen: https://github.com/annotated-types/annotated-types/#minlen-maxlen-len.

Usage would be async def endoint(files: Annotated[list[UploadFile], MaxLen(10)]) which is nicely analogous to async def endpoint(users: Annotated[list[User], MaxLen(10)])

@adriangb Interesting! Will take a look at that approach

From what I can see I believe this would be the first usage of the annotated-types library in FastAPI - I would feel hesitant to put forward such a dependency without it being the recommended approach

@adriangb
Copy link
Contributor

It’s going to be a dependency anyway via Pydantic V2

@wfranceys wfranceys force-pushed the add-starlette-request-form-max-files branch from 2d889ca to 29ad27f Compare June 11, 2023 19:34
@wfranceys
Copy link
Author

It’s going to be a dependency anyway via Pydantic V2

@adriangb Ah great, that's good to know thanks!

Do you know at all if FastAPI will start using them as a common pattern?

@adriangb
Copy link
Contributor

Yes it will be, Sebastián is working on but not sure when he’ll be merging the PR with Pydantic V2, but I’m guessing before this PR

@tiangolo
Copy link
Member

📝 Docs preview for commit 6212e1f at: https://6486235a6038ce493d4f630f--fastapi.netlify.app

@gungjodi
Copy link

gungjodi commented Aug 1, 2023

hi @tiangolo @adriangb any updates on this?

@cardoso-neto
Copy link

Having the ability to do this would be useful for many usecases. Looking forward for updates.

@tiangolo tiangolo added investigate feature New feature or request p4 and removed investigate labels Jan 13, 2024
@Themis3000
Copy link

For my specific use case this is pretty important to me. Anyone aware of any workarounds? Any updates on this?

@aaoobd3
Copy link

aaoobd3 commented Jul 26, 2024

any updates on this

@gungjodi
Copy link

gungjodi commented Jul 26, 2024

Hi,
I found a workaround on this issue. As pointed on this StackOverflow thread

Basically, you need to set the max_fields & max_files in request.form on Starlette's Request object as below :

Set max files / max fields

async def get_body(request: Request):
    try:
        ## adjust max_fields & max_files based on your needs
        return await request.form(max_fields=1000000, max_files=100000)
    except Exception as exc:
        raise HTTPException(status_code=400, detail='Invalid Form data') from exc

then on your route, use above function as Depend on body

@app.post("/send-files")
async def send_files(body = Depends(get_body)):
    ## get your files using getlist('<files field name>')
    if isinstance(body, FormData):
        files = body.getlist('files')

    ## 'files' variable will be instance of List[UploadFile]
    ....process your files here
    
    return {
        'totalFiles': len(files)
    }

image

Hope this helps

@Yrij-Zhavoronkov
Copy link

Yrij-Zhavoronkov commented Jan 21, 2025

Hi, I found a workaround on this issue. As pointed on this StackOverflow thread

Basically, you need to set the max_fields & max_files in request.form on Starlette's Request object as below :

Set max files / max fields

async def get_body(request: Request):
    try:
        ## adjust max_fields & max_files based on your needs
        return await request.form(max_fields=1000000, max_files=100000)
    except Exception as exc:
        raise HTTPException(status_code=400, detail='Invalid Form data') from exc

then on your route, use above function as Depend on body

@app.post("/send-files")
async def send_files(body = Depends(get_body)):
    ## get your files using getlist('<files field name>')
    if isinstance(body, FormData):
        files = body.getlist('files')

    ## 'files' variable will be instance of List[UploadFile]
    ....process your files here
    
    return {
        'totalFiles': len(files)
    }

image

Hope this helps

Your solution looks to work, but it’s not clear how to combine it with this:

@app.post("/images")
async def upload_images(
   field_0: Annotated[str, Body(...)],
   ...
   field_k: Annotated[str, Body(...)],
   images: Annotated[List[UploadFile], File()]):

without losing the default behavior (pydantic type validations, auto documentation, etc..). So far the only option that comes to mind is: parse the form, create a pydantic model, perform validation, pass the fields directly to the method upload_images.

async def workaround(self, request: Request):
    try:
        form_body = await request.form(max_fields=4, max_files=5000)
        params = AddImagesParams(**{**form_body, 'images': form_body.getlist("images")})
    except ValidationError as ve:
        raise HTTPException(status_code=422, detail=ve.errors())
    except Exception as e:
        raise HTTPException(status_code=400, detail=str(e))
    return await self.add_images(task_id=params.task_id, class_name=params.class_name, images=params.images)

But in my mind it looks cumbersome and ugly. Maybe I'm complicating things and there is a more correct and simpler way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request p4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants