这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@kemingy
Copy link
Member

@kemingy kemingy commented Aug 22, 2025

Signed-off-by: Keming <kemingy94@gmail.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes form validation handling in the Falcon plugin to properly support application/x-www-form-urlencoded content type. The changes differentiate between form-encoded data and multipart form data, ensuring appropriate parsing for each content type.

  • Adds type hint for FalconASGIRequest import and parameter
  • Implements content-type-specific form parsing logic for both sync and async validation methods
  • Ensures form validation only occurs for supported MIME types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Signed-off-by: Keming <kemingy94@gmail.com>
@tfluehmann
Copy link

tfluehmann commented Aug 22, 2025

Works for me!

>>> import requests
>>> url = 'http://localhost:8000/api/file_upload'
>>> files = {'file': open('example.bin', 'rb')}
>>> data = {'uid': 'bla'}
>>> response = requests.post(url, files=files, data=data)
>>> print(response.text)
{"filename": "example.bin", "type": "text/plain", "uid": "bla"}
    @spec.validate(resp=Response(HTTP_200=FileResp), tags=["file-upload"])
    def on_post(self, req, resp, form: File):
        """
        post multipart/form-data demo

        demo for 'form'
        """
        file: BodyPart = form.file
        response = {}
        if file:
            response = {"filename": file.secure_filename, "type": file.content_type}
        if form.uid:
            response.update({"uid": form.uid})
        resp.media = response

Signed-off-by: Keming <kemingy94@gmail.com>
@kemingy kemingy merged commit 2c2e484 into 0b01001001:main Aug 22, 2025
9 checks passed
@kemingy kemingy deleted the falcon_form_header branch August 22, 2025 10:09
Copy link

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

Just a couple of late comments. 😈

if form and req.content_type:
try:
form_data = await req.get_media()
except HTTPError as err:
Copy link

Choose a reason for hiding this comment

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

This works, although it would be more idiomatic to catch the media-specific exceptions: MediaNotFoundError (irrelevant if you are sure you handling forms here) and, the most important, MediaMalformedError.

req_form = {}
async for part in form_data:
if part.filename is None:
req_form[part.name] = await part.data
Copy link

Choose a reason for hiding this comment

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

These async properties look a bit awkward and provided only for compatibility. I would prefer part.get_data().

# pass the `falcon.BodyPart` if it's attached as a file
req_form[part.name] = part
# try to consume the file data, otherwise it will be lost
_ = part.data
Copy link

Choose a reason for hiding this comment

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

As above, just part.get_data() should look cleaner and more obvious it performs I/O.

# pass the `falcon.BodyPart` if it's attached as a file
req_form[part.name] = part
# try to consume the file data, otherwise it will be lost
await part.data
Copy link

Choose a reason for hiding this comment

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

Note that Falcon doesn't spool data to temporary files or anything like that out of the box. part.get_text() and part.get_data() are meant as convenience methods for getting short form fields, not files. They are limited to (by default, configurable) 1 MiB.

Although maybe you are not intending to handle large files here in the OpenAPI context?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @vytas7, thanks so much for your review. I created a new PR to address those issues. Needs your help there #435 if you have time.

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.

bug: Falcon demo file upload - AttributeError: 'bytes' object has no attribute 'filename'

3 participants