Skip to content

UTF-16 and UTF-32 codecs should reject (lone) surrogates #57101

Closed

Description

@ezio-melotti
BPO 12892
Nosy @malemburg, @gvanrossum, @loewis, @mjpieters, @pitrou, @vstinner, @ezio-melotti, @serhiy-storchaka
Files
  • utf_16_32_surrogates.patch
  • utf-16&32_reject_surrogates.patch: rejects surrogate and calls the error handlers
  • surrogatepass_for_utf-16&32.patch: surrogatepass erro handler for utf-16/32
  • utf_16_32_surrogates_3.patch
  • utf_16_32_surrogates_4.patch
  • utf_16_32_surrogates_5.patch
  • utf_16_32_surrogates_6.patch
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2013-11-19.10:31:20.466>
    created_at = <Date 2011-09-04.10:49:30.301>
    labels = ['type-bug', 'expert-unicode']
    title = 'UTF-16 and UTF-32 codecs should reject (lone) surrogates'
    updated_at = <Date 2015-08-11.11:28:28.010>
    user = 'https://github.com/ezio-melotti'

    bugs.python.org fields:

    activity = <Date 2015-08-11.11:28:28.010>
    actor = 'tmp12342'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2013-11-19.10:31:20.466>
    closer = 'serhiy.storchaka'
    components = ['Unicode']
    creation = <Date 2011-09-04.10:49:30.301>
    creator = 'ezio.melotti'
    dependencies = []
    files = ['23810', '24368', '24384', '31557', '31984', '32047', '32202']
    hgrepos = []
    issue_num = 12892
    keywords = ['patch']
    message_count = 41.0
    messages = ['143490', '143495', '148603', '148613', '148614', '152310', '152316', '152420', '159129', '196785', '196788', '196790', '196798', '196803', '198803', '199151', '199179', '199183', '199184', '199186', '199187', '199188', '199189', '199190', '199191', '199192', '199193', '199194', '199195', '199197', '199371', '199375', '199500', '200233', '203344', '203353', '203375', '203591', '248362', '248392', '248404']
    nosy_count = 12.0
    nosy_names = ['lemburg', 'gvanrossum', 'loewis', 'mjpieters', 'pitrou', 'vstinner', 'ezio.melotti', 'python-dev', 'tchrist', 'kennyluck', 'serhiy.storchaka', 'tmp12342']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue12892'
    versions = ['Python 3.4']

    Activity

    1. malemburg commented on Sep 2, 2013

      @malemburg
      Member

      On 02.09.2013 18:56, Serhiy Storchaka wrote:

      Oh, I were blind. Thank you Marc-Andre. Here is corrected patch. Unfortunately it 1.4-1.5 times slower on UTF-16 encoding UCS2 strings than previous wrong patch.

      I think it would be faster to do this in two steps:

      1. check the ranges of the input

      2. do a memcpy() if there are no lone surrogates

      Part 1 can be further optimized by adding a simple range
      check (ch >= 0xd800).

    2. serhiy-storchaka commented on Sep 2, 2013

      @serhiy-storchaka
      Member

      No, it isn't faster. I tested this variant, it is 1.5x slower.

      And simple range checking actually is slower.

    3. serhiy-storchaka commented on Oct 1, 2013

      @serhiy-storchaka
      Member

      Could you please make a review Ezio?

    4. serhiy-storchaka commented on Oct 7, 2013

      @serhiy-storchaka
      Member

      Updated whatsnew and Misc/ files.

    5. pitrou commented on Oct 8, 2013

      @pitrou
      Member

      utf-16 isn't that widely used, so it's probably fine if it becomes a bit slower.

    6. malemburg commented on Oct 8, 2013

      @malemburg
      Member

      On 08.10.2013 10:46, Antoine Pitrou wrote:

      utf-16 isn't that widely used, so it's probably fine if it becomes a bit slower.

      It's the default encoding for Unicode text files and APIs on Windows,
      so I'd say it *is* widely used :-)

      http://en.wikipedia.org/wiki/UTF-16#Use_in_major_operating_systems_and_environments

    7. pitrou commented on Oct 8, 2013

      @pitrou
      Member

      On 08.10.2013 10:46, Antoine Pitrou wrote:
      >
      > utf-16 isn't that widely used, so it's probably fine if it becomes
      > a bit slower.

      It's the default encoding for Unicode text files and APIs on Windows,
      so I'd say it *is* widely used :-)

      I've never seen any UTF-16 text files. Do you have other data?

      APIs are irrelevant. You only pass very small strings to then (e.g.
      file paths).

    8. malemburg commented on Oct 8, 2013

      @malemburg
      Member
      On 08.10.2013 11:03, Antoine Pitrou wrote:
      > 
      >>> utf-16 isn't that widely used, so it's probably fine if it becomes
      >>> a bit slower.
      >>
      >> It's the default encoding for Unicode text files and APIs on Windows,
      >> so I'd say it *is* widely used :-)
      > 
      > I've never seen any UTF-16 text files. Do you have other data?

      See the link I posted.

      MS Notepad and MS Office save Unicode text files in UTF-16-LE,
      unless you explicitly specify UTF-8, just like many other Windows
      applications that support Unicode text files:

      http://msdn.microsoft.com/en-us/library/windows/desktop/dd374101%28v=vs.85%29.aspx
      http://superuser.com/questions/294219/what-are-the-differences-between-linux-and-windows-txt-files-unicode-encoding

      This is simply due to the fact that MS introduced Unicode plain
      text files as UTF-16-LE files and only later added the possibility
      to also use UTF-8 with BOM versions.

      APIs are irrelevant. You only pass very small strings to then (e.g.
      file paths).

      You are forgetting that wchar_t is UTF-16 on Windows, so UTF-16
      is all around you when working on Windows, not only in the OS APIs,
      but also in most other Unicode APIs you find on Windows:

      http://msdn.microsoft.com/en-us/library/windows/desktop/dd374089%28v=vs.85%29.aspx
      http://msdn.microsoft.com/en-us/library/windows/desktop/dd374061%28v=vs.85%29.aspx

    9. pitrou commented on Oct 8, 2013

      @pitrou
      Member

      MS Notepad and MS Office save Unicode text files in UTF-16-LE,
      unless you explicitly specify UTF-8, just like many other Windows
      applications that support Unicode text files:

      I'd be curious to know if people actually edit *text files* using
      Microsoft Word (rather than Word documents).
      Same for Notepad, which is much too poor to edit something else
      than a 10-line configuration file.

      You are forgetting that wchar_t is UTF-16 on Windows, so UTF-16
      is all around you when working on Windows, not only in the OS APIs,
      but also in most other Unicode APIs you find on Windows:

      Still, unless those APIs get passed rather large strings, the performance
      different should be irrelevant IMHO. We're talking about using those APIs
      from Python, not from a raw optimized C program.

    10. serhiy-storchaka commented on Oct 8, 2013

      @serhiy-storchaka
      Member

      UTF-16 codec still fast enough. Let first make it correct and then will try optimize it. I have an idea how restore 3.3 performance (if it worth, the code already complicated enough).

      The converting to/from wchar_t* uses different code.

    11. malemburg commented on Oct 8, 2013

      @malemburg
      Member

      On 08.10.2013 11:33, Antoine Pitrou wrote:

      Antoine Pitrou added the comment:

      > MS Notepad and MS Office save Unicode text files in UTF-16-LE,
      > unless you explicitly specify UTF-8, just like many other Windows
      > applications that support Unicode text files:

      I'd be curious to know if people actually edit *text files* using
      Microsoft Word (rather than Word documents).
      Same for Notepad, which is much too poor to edit something else
      than a 10-line configuration file.

      The question is not so much which program they use for editing.
      The format "Unicode text file" is defined as UTF-16-LE on
      Windows (see the links I posted).

      > You are forgetting that wchar_t is UTF-16 on Windows, so UTF-16
      > is all around you when working on Windows, not only in the OS APIs,
      > but also in most other Unicode APIs you find on Windows:

      Still, unless those APIs get passed rather large strings, the performance
      different should be irrelevant IMHO. We're talking about using those APIs
      from Python, not from a raw optimized C program.

      Antoine, I'm just pointing out that your statement that UTF-16
      is not widely used may apply to the Unix world, but
      it doesn't apply to Windows. Java also uses UTF-16
      internally and makes this available via JNI as jchar*.

      The APIs on those platforms are used from Python (the interpreter
      and also by extensions) and do use the UTF-16 Python codec to
      convert the data to Python Unicode/string objects, so the fact
      that UTF-16 is used widely on some of the more popular
      platforms does matter.

      UTF-8, UTF-16 and UTF-32 codecs need to be as fast as possible
      in Python to not create performance problems when converting
      between platform Unicode data and the internal formats
      used in Python.

      The real question is: Can the UTF-16/32 codecs be made fast
      while still detecting lone surrogates ? Not whether UTF-16
      is widely used or not.

    12. serhiy-storchaka commented on Oct 8, 2013

      @serhiy-storchaka
      Member

      I repeat myself. Even with the patch, UTF-16 codec is faster than UTF-8 codec (except ASCII-only data). This is fastest Unicode codec in Python (perhaps UTF-32 can be made faster, but this is another issue).

      The real question is: Can the UTF-16/32 codecs be made fast
      while still detecting lone surrogates ? Not whether UTF-16
      is widely used or not.

      Yes, they can. But let defer this to other issues.

    13. pitrou commented on Oct 8, 2013

      @pitrou
      Member

      UTF-8, UTF-16 and UTF-32 codecs need to be as fast as possible
      in Python to not create performance problems when converting
      between platform Unicode data and the internal formats
      used in Python.

      "As fast as possible" is a platonic dream.
      They only need to be fast enough not to be bottlenecks.
      If you know of a *Python* workload where UTF-16 decoding is the
      bottleneck, I'd like to know about it :-)

    14. malemburg commented on Oct 8, 2013

      @malemburg
      Member

      On 08.10.2013 11:42, Serhiy Storchaka wrote:

      UTF-16 codec still fast enough. Let first make it correct and then will try optimize it. I have an idea how restore 3.3 performance (if it worth, the code already complicated enough).

      That's a good plan :-)

    15. malemburg commented on Oct 8, 2013

      @malemburg
      Member

      On 08.10.2013 12:30, Antoine Pitrou wrote:

      > UTF-8, UTF-16 and UTF-32 codecs need to be as fast as possible
      > in Python to not create performance problems when converting
      > between platform Unicode data and the internal formats
      > used in Python.

      "As fast as possible" is a platonic dream.
      They only need to be fast enough not to be bottlenecks.

      No, they need to be as fast as possible, without sacrificing
      correctness.

      This has always been our guideline for codec implementations
      and string methods. As a result, our implementations are some
      of the best out there.

    16. vstinner commented on Oct 8, 2013

      @vstinner
      Member

      I don't think that performances on a microbenchmark is the good question.
      The good question is: does Python conform to Unicode? The answer is simple
      and explicit: no. Encoding lone surrogates may lead to bugs and even
      security vulnerabilities.

      Please open a new performance issue after fixing this one if you have
      another patch improving performances.

      I didn't read the patch yet, but strict, surrogatepass and surrogateescape
      error handlers must be checked.

    17. serhiy-storchaka commented on Oct 8, 2013

      @serhiy-storchaka
      Member

      Here is my idea: http://permalink.gmane.org/gmane.comp.python.ideas/23521.

      I see that a discussion about how fast UTF-16 codec should be already larger than discussion about patches. Could you please review this not so simple patch instead?

      Yet one help which I need is writing a note in "Porting to Python 3.4" section in Doc/whatsnew/3.4.rst.

    18. loewis commented on Oct 8, 2013

      loewismannequin
      Mannequin

      Marc-Andre: please don't confuse "use in major operating systems" with "major use in operating systems". I agree with Antoine that UTF-16 isn't widely used on Windows, despite notepad and Office supporting it. Most users on Windows using notepad continue to use the ANSI code page, most users of Word use Word files (instead of plain text).

      Also, wchar_t on Windows isn't *really* UTF-16. Many APIs support lone surrogates just fine; they really are UCS-2 instead (e.g. the file system APIs). Only starting with Vista, MultiByteToWideChar will complain about lone surrogates.

    19. vstinner commented on Oct 10, 2013

      @vstinner
      Member

      I tested utf_16_32_surrogates_4.patch: surrogateescape with as encoder does not work as expected.

      >>> b'[\x00\x80\xdc]\x00'.decode('utf-16-le', 'ignore')
      '[]'
      >>> b'[\x00\x80\xdc]\x00'.decode('utf-16-le', 'replace')
      '[�]'
      >>> b'[\x00\x80\xdc]\x00'.decode('utf-16-le', 'surrogateescape')
      '[\udc80\udcdc\uffff'

      => I expected '[\udc80\udcdc]'.

      With a decoder, surrogateescape does not work neither:

      >>> '[\uDC80]'.encode('utf-16-le', 'surrogateescape')
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      UnicodeEncodeError: 'utf-16-le' codec can't encode character '\udc80' in position 1: surrogates not allowed

      Using the PEP-383, I expect that data.decode(encoding, 'surrogateescape') does never fail, data.decode(encoding, 'surrogateescape').encode(encoding, 'surrogateescape') should give data.

      --

      With UTF-16, there is a corner case:

      >>> b'[\x00\x00'.decode('utf-16-le', 'surrogateescape')
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
        File "/home/haypo/prog/python/default/Lib/encodings/utf_16_le.py", line 16, in decode
          return codecs.utf_16_le_decode(input, errors, True)
      UnicodeDecodeError: 'utf-16-le' codec can't decode byte 0x00 in position 2: truncated data
      >>> b'[\x00\x80'.decode('utf-16-le', 'surrogateescape')
      '[\udc80'

      The incomplete sequence b'\x00' raises a decoder error, wheras b'\x80' does not. Should we extend the PEP-383 to bytes in range [0; 127]? Or should we keep this behaviour?

      Sorry, this question is unrelated to this issue.

    20. vstinner commented on Oct 10, 2013

      @vstinner
      Member

      Could you please review this not so simple patch instead?

      I did a first review of your code on rietveld.

    21. serhiy-storchaka commented on Oct 11, 2013

      @serhiy-storchaka
      Member

      Updated patch addresses Victor's comments on Rietveld. Thank you Victor. The "surrogatepass" error handler now works with different spellings of encodings ("utf_32le", "UTF-32-LE", etc).

      I tested utf_16_32_surrogates_4.patch: surrogateescape with as encoder does not work as expected.

      Yes, surrogateescape doesn't work with ASCII incompatible encodings and can't. First, it can't represent the result of decoding b'\x00\xd8' from utf-16-le or b'ABCD' from utf-32*. This problem is worth separated issue (or even PEP) and discussion on Python-Dev.

    22. serhiy-storchaka commented on Oct 18, 2013

      @serhiy-storchaka
      Member

      Changed the documentation as was discussed with Ezio on IRC.

      Ezio, do you want commit this patch? Feel free to reword the documentation if you are feeling be better.

    23. python-dev commented on Nov 19, 2013

      python-devmannequin
      Mannequin

      New changeset 0d9624f2ff43 by Serhiy Storchaka in branch 'default':
      Issue bpo-12892: The utf-16* and utf-32* codecs now reject (lone) surrogates.
      http://hg.python.org/cpython/rev/0d9624f2ff43

    24. serhiy-storchaka commented on Nov 19, 2013

      @serhiy-storchaka
      Member

      Ezio have approved the patch and I have committed it.

      Thank you Victor and Kang-Hao for your patches. Thanks all for the reviews.

    25. python-dev commented on Nov 19, 2013

      python-devmannequin
      Mannequin

      New changeset 130597102dac by Serhiy Storchaka in branch 'default':
      Remove dead code committed in issue bpo-12892.
      http://hg.python.org/cpython/rev/130597102dac

    26. vstinner commented on Nov 21, 2013

      @vstinner
      Member

      Thanks Ezio and Serhiy for having fix UTF-16 and UTF-32 codecs!

    27. mjpieters commented on Aug 10, 2015

      mjpietersmannequin
      Mannequin

      I don't understand why encoding with surrogateescape isn't supported still; is it the fact that a surrogate would have to produce single bytes rather than double? E.g. b'\x80' -> '\udc80' -> b'\x80' doesn't work because that would mean the UTF-16 and UTF-32 codec could then end up producing an odd number of bytes?

    28. serhiy-storchaka commented on Aug 11, 2015

      @serhiy-storchaka
      Member

      There are two causes:

      1. UTF-16 and UTF-32 are based on 2- and 4-bytes units. If the surrogateescape error handler will support UTF-16 and UTF-32, encoding could produce the data that can't be decoded back correctly. For example '\udcac \udcac' -> b'\xac\x20\x00\xac' -> '\u20ac\uac20' == '€가'.

      2. ASCII bytes (0x00-0x80) can't be escaped with surrogateescape. UTF-16 and UTF-32 data can contain illegal ASCII bytes (b'\xD8\x00' in UTF-16-BE, b'abcd' in UTF-32). For the same reason surrogateescape is not compatible with UTF-7 and CP037.

    29. tmp12342 commented on Aug 11, 2015

      tmp12342mannequin
      Mannequin

      Serhiy, I understand the first reason, but https://docs.python.org/3/library/codecs.html says

      applicable to text encodings:
      [...]
      This code will then be turned back into the same byte when the 'surrogateescape' error handler is used when encoding the data.
      Shouldn't it be corrected? Text encoding is defined as "A codec which encodes Unicode strings to bytes."

      And about second one, could you explain a bit more? I mean, I don't know how to interpret it.

      You say b'\xD8\x00' are invalid ASCII bytes, but from these two only 0xD8 is invalid. Also, we are talking about encoding here, str -> bytes, so who cares are resulting bytes ASCII compatible or not?

    30. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions