Skip to content

Improve encodings.normalize_encoding behaviour or docs #136702

Open

Description

@StanFromIreland

Bug report

Bug description:

This is to wait for #55531 / #136643

  • 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 :-)

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    stdlibStandard Library Python modules in the Lib/ directory
    on Jul 16, 2025
  2. changed the title [-]Improve `encodings.normalize` behaviour or docs[/-] [+]Improve `encodings.normalize_encoding` behaviour or docs[/+] on Oct 2, 2025
  3. added a commit that references this issue on Nov 9, 2025
  4. encukou commented on Nov 10, 2025

    @encukou
    Member

    The test fails when run repeatedly, see e.g. here: https://buildbot.python.org/#/builders/320/builds/3283

  5. encukou commented on Nov 10, 2025

    @encukou
    Member

    Also, is there a reason to not use the preferred deprecation period of 5 years?

  6. malemburg commented on Nov 10, 2025

    @malemburg
    Member

    The test fails when run repeatedly, see e.g. here: https://buildbot.python.org/#/builders/320/builds/3283

    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.

    Also, is there a reason to not use the preferred deprecation period of 5 years?

    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.

  7. encukou commented on Nov 10, 2025

    @encukou
    Member

    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?

    >>> 'foo'.encode('utf☃8')
    b'foo'
  8. added a commit that references this issue on Nov 10, 2025
  9. malemburg commented on Nov 10, 2025

    @malemburg
    Member

    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.

  10. encukou commented on Nov 10, 2025

    @encukou
    Member

    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?

  11. added a commit that references this issue on Nov 10, 2025
  12. malemburg commented on Nov 10, 2025

    @malemburg
    Member

    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.

    Thoughts ?

  13. encukou commented on Nov 11, 2025

    @encukou
    Member

    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:

    >>> import unicodedata
    >>> import encodings
    >>> weird_name = 'asčií'
    >>> encodings.normalize_encoding(weird_name)
    'asc_ii'
    >>> encodings.normalize_encoding(unicodedata.normalize('NFKC', weird_name))
    'asi'

    Not to mention that it could open the original security issue:

    >>> unicodedata.normalize('NFKC', '\N{TWO DOT LEADER}\N{FULLWIDTH SOLIDUS}')
    '../'

    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.

  14. malemburg commented on Nov 11, 2025

    @malemburg
    Member

    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.

  15. added 2 commits that reference this issue on Dec 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

stdlibStandard Library Python modules in the Lib/ directorytopic-unicodetype-bugAn unexpected behavior, bug, or error

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions