Skip to content
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.

Sign up for GitHub

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

Jump to bottom

codecs.lookup() ignores non-ASCII characters, whereas encodings.normalize_encoding() copies them #83518

Closed
vstinner opened this issue Jan 14, 2020 · 11 comments
Closed

codecs.lookup() ignores non-ASCII characters, whereas encodings.normalize_encoding() copies them #83518

vstinner opened this issue Jan 14, 2020 · 11 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 39337
Nosy @malemburg, @vstinner, @serhiy-storchaka, @shihai1991
PRs
  • [WIP] bpo-39337: calling encodings.normalize_encoding() in codecs.normalizestring() to keep same behavior #18845
  • [WIP]bpo-39337: Adding a _Py_normalize_unicode_encoding() in unicodeobject.c #18987
  • bpo-39337: Add a test case for normalizing of codec names #19069
  • bpo-39337: encodings.normalize_encoding() now ignores non-ASCII characters #22219
  • bpo-41842: Add a unregister function in _codecs module #22360
  • bpo-37751: Document the change in What's New in Python 3.9 #17997
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2020-10-14.15:46:55.467>
created_at = <Date 2020-01-14.21:54:42.124>
labels = ['library', '3.10']
title = 'codecs.lookup() ignores non-ASCII characters, whereas encodings.normalize_encoding() copies them'
updated_at = <Date 2020-10-14.15:55:50.301>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-10-14.15:55:50.301>
actor = 'shihai1991'
assignee = 'none'
closed = True
closed_date = <Date 2020-10-14.15:46:55.467>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2020-01-14.21:54:42.124>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39337
keywords = ['patch']
message_count = 11.0
messages = ['360004', '363645', '364180', '364449', '364452', '364456', '377772', '378281', '378626', '378628', '378630']
nosy_count = 4.0
nosy_names = ['lemburg', 'vstinner', 'serhiy.storchaka', 'shihai1991']
pr_nums = ['18845', '18987', '19069', '22219', '22360', '17997']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39337'
versions = ['Python 3.10']
    @vstinner
    Copy link
    Member Author

    bpo-37751 changed codecs.lookup() in a subtle way: non-ASCII characters are now ignored, whereas they were copied unmodified previously.

    I would prefer that codecs.lookup() and encodings.normalize_encoding() behave the same. Either always ignore or always copy.

    Moreover, it seems like there is no test on how the encoding names are normalized in codecs.register(). I recall that using codecs.register() in an unit test causes troubles since there is no API to unregister a search function. Maybe we should just add a private function for test in _testcapi.

    Serhiy Storchaka wrote an example on my PR:
    https://github.com/python/cpython/pull/17997/files

    There are other differences. For example, normalize_encoding("КОИ-8") returns "кои_8", but codecs.lookup normalizes it to "8".

    The comment in the sources is also not correct.

    @vstinner vstinner added 3.9 only security fixes stdlib Python modules in the Lib dir labels Jan 14, 2020
    @shihai1991
    Copy link
    Member

    I would prefer that codecs.lookup() and encodings.normalize_encoding() behave the same. Either always ignore or always copy.

    How about calling encodings.normalize_encoding() in codecs.normalizestring() to keep same behavior?(I create PR18845)

    Maybe we should just add a private function for test in _testcapi

    I can try to add some test cases in next weekend ;)

    @shihai1991
    Copy link
    Member

    How about calling encodings.normalize_encoding() in codecs.normalizestring() to keep same behavior?(I create PR18845)

    I have try this idea, but it make the testcase of test_io.py failed because some object will call codecs.Lookup() in __del__().-->extension module will be cleaned before calling __del__().

    I would prefer that codecs.lookup() and encodings.normalize_encoding() behave the same. Either always ignore or always copy.

    I try to add a _Py_normalize_unicode_encoding() in unicodeobject.c to support non-ASCII encoding names' normalization(PR18987), but this PR caused many testcases failed.

    For example:

    In master:
    python3.9 -c "print('a\xac\u1234\u20ac\u8000\U0010ffff'.encode('iso-8859-15', 'namereplace'))"
    result:
    b'a\xac\\N{ETHIOPIC SYLLABLE SEE}\xa4\\N{CJK UNIFIED IDEOGRAPH-8000}\\U0010ffff'

    after PR18987:
    ./python -c "print('a\xac\u1234\u20ac\u8000\U0010ffff'.encode('iso-8859-15', 'namereplace'))"
    result:
    b'a\xac\\N{ETHIOPIC SYLLABLE SEE}\\N{EURO SIGN}\\N{CJK UNIFIED IDEOGRAPH-8000}\\U0010ffff'

    @shihai1991
    Copy link
    Member

    Maybe we should just add a private function for test in _testcapi.

    Oh, there have a problem with this idea:
    struct _is is defined in internal/pycore_pystate.h.
    _testcapi must test the public Python C API, not CPython internal C API

    @vstinner
    Copy link
    Member Author

    _testcapi must test the public Python C API, not CPython internal C API

    Use _testinternalcapi in this case.

    @shihai1991
    Copy link
    Member

    Use _testinternalcapi in this case.
    oh, forgive me. I don't atttention this extension module before :(

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 1, 2020

    See also bpo-37751 and my PR 17997.

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 8, 2020

    New changeset 3f34237 by Hai Shi in branch 'master':
    bpo-39337: Add a test case for normalizing of codec names (GH-19069)
    3f34237

    @vstinner
    Copy link
    Member Author

    New changeset c5b049b by Hai Shi in branch 'master':
    bpo-39337: encodings.normalize_encoding() now ignores non-ASCII characters (GH-22219)
    c5b049b

    @vstinner
    Copy link
    Member Author

    Thanks Hai Shi for the change and for new codecs and encodings tests.

    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Oct 14, 2020
    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Oct 14, 2020
    @shihai1991
    Copy link
    Member

    Thanks for everyone's continus review :)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.10 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @shihai1991