-
-
Notifications
You must be signed in to change notification settings - Fork 948
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
feat(multipart): add max_length to secure_filename() #2421
base: master
Are you sure you want to change the base?
Changes from all commits
fe1a82a
626da2d
0c61a5e
c37a6b8
96ec114
a323dea
03e0d10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -592,6 +592,15 @@ class MultipartParseOptions: | |
If the body part headers size exceeds this value, an instance of | ||
:class:`.MultipartParseError` will be raised. | ||
""" | ||
max_filename_length: Optional[int] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new length is not passed to the actual invocation of Also add tests (make sure to parametrize on both WSGI/ASGI) for actually modifying this parsing option, and verifying that it is respected when referencing the said property. |
||
"""The maximum length of filenames in multipart forms (default ``None``). | ||
|
||
If the filename exceeds this value, it will be truncated while preserving | ||
the file extension. Defaults to ``None`` for backward compatibility. | ||
|
||
.. note:: | ||
Starting from Falcon 5.0, this will default to 255 characters. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest a smaller default value. On some systems, e.g., Windows, the max possible path length is around 255-260, so likely we will want to make the file name part of the path shorter than that. Maybe 64 could be a sane default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 64 may be a bit small as default, but 96/128 is likely ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I think 64 ought to be enough for the basename, since |
||
""" | ||
media_handlers: Handlers | ||
"""A dict-like object for configuring the media-types to handle. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add a newsfragment for this improvement.