Add MIME content type info to File #143
Conversation
Makes all headers lower case, fixing case sensitivity issues. Exposes jheaders property in Files and Fields.
jhnstrk
force-pushed
the
file_content_type
branch
from
4304002 to
f1e28d6
Compare
| @@ -68,12 +68,14 @@ def finalize(self) -> None: ... | |||
| def close(self) -> None: ... | |||
|
|
|||
| class FieldProtocol(_FormProtocol, Protocol): | |||
| def __init__(self, name: bytes | None) -> None: ... | |||
| def __init__(self, name: bytes, content_type: str | None = None) -> None: ... | |||
There was a problem hiding this comment.
Yes, but not frequently used. Examples:
- Passing json and specifying "application/json"
- Embedding "multipart/mixed" inside ( https://www.ietf.org/rfc/rfc2388.txt ).
- Specifying a charset for a field
There was a problem hiding this comment.
Specifying a charset for a field
This is enough to convince me. Thanks.
jhnstrk
force-pushed
the
file_content_type
branch
from
0fc5e2d to
f1e28d6
Compare
python_multipart/multipart.py
Outdated
| disp, options = parse_options_header(content_disp) | ||
|
|
||
| # Get the field and filename. | ||
| field_name = options.get(b"name") | ||
| field_name = options.get(b"name", b"") |
There was a problem hiding this comment.
Is this needed? Isn't the field_name supposed to always be present?
There was a problem hiding this comment.
I added this for typing. Without it, we'll need to raise or assert it's not None. Which makes sense actually.
Will probably need a test to cover it. too.
There was a problem hiding this comment.
Removed the default value and added a check for None.
| @@ -1656,7 +1678,7 @@ def on_header_value(data: bytes, start: int, end: int) -> None: | |||
| header_value.append(data[start:end]) | |||
|
|
|||
| def on_header_end() -> None: | |||
| headers[b"".join(header_name)] = b"".join(header_value) | |||
| headers[b"".join(header_name).decode().lower()] = b"".join(header_value) | |||
There was a problem hiding this comment.
I think the headers dict is easier to use if the keys are strings rather than bytes, so I used decode to do the transformation. The string is guaranteed to be valid since it can only contain a subset of ASCII characters (it's filtered as it's built). Can be bytes if you prefer, in which case decode isn't needed.
Co-authored-by: Marcelo Trylesinski <[email protected]>
Co-authored-by: Marcelo Trylesinski <[email protected]>
Co-authored-by: Marcelo Trylesinski <[email protected]>
| """ | ||
|
|
||
| def __init__(self, name: bytes | None) -> None: | ||
| def __init__(self, name: bytes, content_type: str | None = None) -> None: |
There was a problem hiding this comment.
Why did the name type changed here? Now we have a mismatch between this and the File.field_name. 🤔
Can we revert this bit?
| @@ -1540,7 +1562,7 @@ def __init__( | |||
|
|
|||
| def on_start() -> None: | |||
| nonlocal file | |||
| file = FileClass(file_name, None, config=cast("FileConfig", self.config)) | |||
| file = FileClass(file_name, b"", config=cast("FileConfig", self.config)) | |||
There was a problem hiding this comment.
| file = FileClass(file_name, b"", config=cast("FileConfig", self.config)) | |
| file = FileClass(file_name, config=cast("FileConfig", self.config)) |
| @@ -1656,7 +1678,7 @@ def on_header_value(data: bytes, start: int, end: int) -> None: | |||
| header_value.append(data[start:end]) | |||
|
|
|||
| def on_header_end() -> None: | |||
| headers[b"".join(header_name)] = b"".join(header_value) | |||
| headers[b"".join(header_name).decode().lower()] = b"".join(header_value) | |||
Adds MIME type information as
File.content_typeas requested in issue #58.Tests have been updated and a case-sensitive header issue was fixed too.
The part headers are also exposed as a dict, with keys as strings, lower-cased as
File.headers.