gh-75223: Reject undotted extensions in mimetypes.add_type()#2895
gh-75223: Reject undotted extensions in mimetypes.add_type()#2895OddBloke wants to merge 16 commits intopython:mainfrom
mimetypes.add_type()#2895Conversation
|
I've opened this now to get feedback on whether this is an appropriate change to make. Does it need a warning first, before it becomes the default behaviour? (At a minimum, I would expect to need to add to documentation.) |
|
As said in https://bugs.python.org/issue31040, please use There is one case where it works without a dot: And someone may have used it to distinguish between emptyness and type not found. What I propose: Emit a warning in case the ext is empty, else ValueError in case the type does not start with a dot, to ensure we're not breaking any code. |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Extensions that don't start with a dot will never be found by methods/functions that act on the registry, so we should stop users from mistakenly adding them. The one exception is the empty string extension. This could be in use to detect absent extensions, so instead of raising a ValueError we emit a warning.
b92d271 to
1485be9
Compare
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @JulienPalard: please review the changes made to this pull request. |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Dismissing my review, I've updated the PR
This comment was marked as outdated.
This comment was marked as outdated.
|
IMO, this is a backwards-incompatible change and should be deprecated with a warning, and documented. (For example, if |
|
Thanks, please see #128638 to deprecate it first, and I've marked this as a draft until it's ready for merge. |
This comment was marked as outdated.
This comment was marked as outdated.
It was deprecated and scheduled for 3.16, so it's time! |
Documentation build overview
|
mimetypes.add_type()
|
|
||
| .. function:: add_type(type, ext, strict=True) | ||
|
|
||
| Add a mapping from the MIME type *type* to the extension *ext*. When the |
There was a problem hiding this comment.
This should have the same "Valid extensions start with a '.' or are empty." note IMO, but it wasn't updated before?
Extensions that don't start with a dot will never be found by
methods/functions that act on the registry, so we should stop users from
mistakenly adding them.
https://bugs.python.org/issue31040