You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Improve encodings.normalize_encoding behaviour or docs #136702
This function accepts bytes since 98297ee. This is however undocumented, and untested. I propose deprecating (& later removing) it, otherwise it should be documented and tested properly.
This function is documented as
encoding should be ASCII only.
However, this is only enforced for bytes (see point 1) with a ValueError, and for strings the characters are simply removed. We should be consistent and not depend on input type. I propose enforcing this for strings too with a ValueError.
cc @malemburg (Note, absolutely no rush for this one, we should get the performance issue/C implementation sorted out first :-)
Are warning tests expected to work when tests are run again in the same process ? Background: Warnings are normally only shown once per process invokation.
Yes, but since we don't have any encodings with non-ASCII names, the extra warning won't cause much harm. The warnings only come in addition to the LookupError.
Are warning tests expected to work when tests are run again in the same process?
Yes. The warnings_helper.check_warnings helper will reset the filters appropriately; not sure about unittest's assertWarns.
But this one is about other process-wide caches. The test suite has some magic for cases like this; I'll send a PR (after a while as I'm running refleak tests).
Yes, but since we don't have any encodings with non-ASCII names, the extra warning won't cause much harm. The warnings only come in addition to the LookupError.
We don't have any normalized non-ASCII names, but isn't the point of normalization to coerce them ASCII alphanumerics?
We don't have any normalized non-ASCII names, but isn't the point of normalization to coerce them ASCII alphanumerics?
Not really, no. The function in the encodings package has always mentioned that "encoding names should be ASCII only; if they do use non-ASCII characters, these must be Latin-1 compatible." The Latin-1 part was later removed to only leave ASCII as permitted encoding for encoding names.
The purpose of the normalization is to remove special symbols from the names, since encoding names often use e.g. hyphens, spaces or dots which need to be coerced to a standard form to prepare for lookup. It was never intended to be used as a general purpose cleanup function 🙂
Allowing such names might even result in security issues, since this could be used to bypass filters in other (non-Python) applications dealing with the data before passing it to Python.
Regarding the deprecation phase: We can bump this to 5 years, no problem.
The function in the encodings package has always mentioned that "encoding names should be ASCII only; if they do use non-ASCII characters, these must be Latin-1 compatible." The Latin-1 part was later removed to only leave ASCII as permitted encoding for encoding names.
Yes, but all that time it was a only docstring in a helper function. I'd be as careful here as with any breaking change.
Allowing such names might even result in security issues, since this could be used to bypass filters in other (non-Python) applications dealing with the data before passing it to Python.
I'm pretty sure that is not an original design consideration :)
Shouldn't we disallow control characters here, too?
The function in the encodings package has always mentioned that "encoding names should be ASCII only; if they do use non-ASCII characters, these must be Latin-1 compatible." The Latin-1 part was later removed to only leave ASCII as permitted encoding for encoding names.
Yes, but all that time it was a only docstring in a helper function. I'd be as careful here as with any breaking change.
Allowing such names might even result in security issues, since this could be used to bypass filters in other (non-Python) applications dealing with the data before passing it to Python.
I'm pretty sure that is not an original design consideration :) Shouldn't we disallow control characters here, too?
True. The original design was for Python 2 when encoding name strings were 8-bit strings and not Unicode as they are now.
Security concerns were only related to possibly tricking the system into importing modules outside the encoding package.
Thinking about this some more:
The logic of the search function is to try a lookup of an encoding name, using normalization to provide a bit more flexibility in writing those names. If the lookup fails for any reason, it returns None. The codec registry then tries the next search function. If none succeed, you get a LookupError.
Having the search function raise an exception due to the normalization function failing, would break the search function chain. This is not really desired behavior.
Given that and the fact that the ASCII restriction is somewhat arbitrary (like you say, control characters are still accepted), I think it's better to revert the PR and instead not restrict the range of accepted encoding names at all.
The doc string and other docs would then need to be updated accordingly.
That was my original thinking, but I'm not so sure any more.
The other option would be to disallow control characters along with non-ASCII ones, as an explicit breaking change with a deprecation period. While an ASCII restriction does feel arbitrary, a [\x20-\x7e]* restriction sounds more reasonable given backwards compatibility and security concerns, and the fact that all encodings seem to have a commonly used ASCII name.
(There's no good reason to allow utf^8 for example, but it also has no additional security/confusion concerns over allowing both utf-8 and utf_8.)
If we do allow non-ASCII names, it would be proper to treat them as identifiers, and at least normalize (if not do a full UAX#31 dance). That would also technacally be a breaking change:
Having the search function raise an exception due to the normalization function failing, would break the search function chain. This is not really desired behavior.
The search function gets the normalized name. The codec registry should be able to raise LookupError on failed normalization before getting to the chain.
The search function gets the normalized name. The codec registry should be able to raise LookupError on failed normalization before getting to the chain.
Good catch ! But perhaps not in the way you intended 🙂
I'll have to do a more thorough review of what we have at the moment, since it seems that the normalize function which is exposed to the encodings package search function is in fact not the one which is used by the codecs registry.
Even though we now only have two such normalization functions in CPython, this is still one too many. We have one in Python/codecs.c:normalizestring() and one in Objects/unicodeobject.c:_Py_normalize_encoding(). They are very similar, but not quite the same. Both are implemented in terms of char *, so essentially receive UTF-8 encoded versions of whatever was passed in to the corresponding functions exposed at the Python level.
Regarding the deprecation of passing in non-ASCII names to the encoding package's search function, I'm leaning more and more towards just removing this restriction, since an exception (after the deprecation period) would do more harm than good.
The encodings package search function should continue to work as it does now: convert all unexpected characters to lowercase ASCII or underscores. It should return None for unknown encodings and not raise exceptions when passing in non-ASCII encoding names to no disrupt the chain of search functions.
I'll ping again, once I have completed the review.
BTW: The encodings package search function will ignore any encoding name with embedded dots (after passing through aliasing), so it's not possible to trick the import into going into other directories.
Activity
[-]Improve `encodings.normalize` behaviour or docs[/-][+]Improve `encodings.normalize_encoding` behaviour or docs[/+]pythongh-136702: Deprecate passing non-ascii *encoding* (str) to `enc…
encukou commentedon Nov 10, 2025
The test fails when run repeatedly, see e.g. here: https://buildbot.python.org/#/builders/320/builds/3283
encukou commentedon Nov 10, 2025
Also, is there a reason to not use the preferred deprecation period of 5 years?
malemburg commentedon Nov 10, 2025
Are warning tests expected to work when tests are run again in the same process ? Background: Warnings are normally only shown once per process invokation.
Yes, but since we don't have any encodings with non-ASCII names, the extra warning won't cause much harm. The warnings only come in addition to the
LookupError.encukou commentedon Nov 10, 2025
Yes. The
warnings_helper.check_warningshelper will reset the filters appropriately; not sure about unittest'sassertWarns.But this one is about other process-wide caches. The test suite has some magic for cases like this; I'll send a PR (after a while as I'm running refleak tests).
We don't have any normalized non-ASCII names, but isn't the point of normalization to coerce them ASCII alphanumerics?
pythongh-136702: Clear codec caches for refleak tests; use test.suppo…
malemburg commentedon Nov 10, 2025
Not really, no. The function in the encodings package has always mentioned that "encoding names should be ASCII only; if they do use non-ASCII characters, these must be Latin-1 compatible." The Latin-1 part was later removed to only leave ASCII as permitted encoding for encoding names.
The purpose of the normalization is to remove special symbols from the names, since encoding names often use e.g. hyphens, spaces or dots which need to be coerced to a standard form to prepare for lookup. It was never intended to be used as a general purpose cleanup function 🙂
Allowing such names might even result in security issues, since this could be used to bypass filters in other (non-Python) applications dealing with the data before passing it to Python.
Regarding the deprecation phase: We can bump this to 5 years, no problem.
encukou commentedon Nov 10, 2025
Yes, but all that time it was a only docstring in a helper function. I'd be as careful here as with any breaking change.
I'm pretty sure that is not an original design consideration :)
Shouldn't we disallow control characters here, too?
gh-136702: Clear codec caches for refleak tests; use test.support hel…
malemburg commentedon Nov 10, 2025
True. The original design was for Python 2 when encoding name strings were 8-bit strings and not Unicode as they are now.
Security concerns were only related to possibly tricking the system into importing modules outside the encoding package.
Thinking about this some more:
The logic of the search function is to try a lookup of an encoding name, using normalization to provide a bit more flexibility in writing those names. If the lookup fails for any reason, it returns
None. The codec registry then tries the next search function. If none succeed, you get aLookupError.Having the search function raise an exception due to the normalization function failing, would break the search function chain. This is not really desired behavior.
Given that and the fact that the ASCII restriction is somewhat arbitrary (like you say, control characters are still accepted), I think it's better to revert the PR and instead not restrict the range of accepted encoding names at all.
The doc string and other docs would then need to be updated accordingly.
Thoughts ?
encukou commentedon Nov 11, 2025
That was my original thinking, but I'm not so sure any more.
The other option would be to disallow control characters along with non-ASCII ones, as an explicit breaking change with a deprecation period. While an ASCII restriction does feel arbitrary, a
[\x20-\x7e]*restriction sounds more reasonable given backwards compatibility and security concerns, and the fact that all encodings seem to have a commonly used ASCII name.(There's no good reason to allow
utf^8for example, but it also has no additional security/confusion concerns over allowing bothutf-8andutf_8.)If we do allow non-ASCII names, it would be proper to treat them as identifiers, and at least normalize (if not do a full UAX#31 dance). That would also technacally be a breaking change:
Not to mention that it could open the original security issue:
The search function gets the normalized name. The codec registry should be able to raise
LookupErroron failed normalization before getting to the chain.malemburg commentedon Nov 11, 2025
Good catch ! But perhaps not in the way you intended 🙂
I'll have to do a more thorough review of what we have at the moment, since it seems that the normalize function which is exposed to the encodings package search function is in fact not the one which is used by the codecs registry.
Even though we now only have two such normalization functions in CPython, this is still one too many. We have one in
Python/codecs.c:normalizestring()and one inObjects/unicodeobject.c:_Py_normalize_encoding(). They are very similar, but not quite the same. Both are implemented in terms ofchar *, so essentially receive UTF-8 encoded versions of whatever was passed in to the corresponding functions exposed at the Python level.Regarding the deprecation of passing in non-ASCII names to the encoding package's search function, I'm leaning more and more towards just removing this restriction, since an exception (after the deprecation period) would do more harm than good.
The encodings package search function should continue to work as it does now: convert all unexpected characters to lowercase ASCII or underscores. It should return
Nonefor unknown encodings and not raise exceptions when passing in non-ASCII encoding names to no disrupt the chain of search functions.I'll ping again, once I have completed the review.
BTW: The encodings package search function will ignore any encoding name with embedded dots (after passing through aliasing), so it's not possible to trick the import into going into other directories.
pythongh-136702: Deprecate passing non-ascii *encoding* (str) to `enc…
pythongh-136702: Clear codec caches for refleak tests; use test.suppo…