Skip to content

ElementTree (py3k) doesn't properly encode characters that can't be represented in the specified encoding #50482

New issue
New issue
Closed
Closed
ElementTree (py3k) doesn't properly encode characters that can't be represented in the specified encoding#50482
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Description

@NeilMuller
NeilMuller
mannequin
opened on Jun 7, 2009
BPO 6233
Nosy @loewis, @rhettinger, @pitrou, @benjaminp
Files
  • issue6233_py3k.diff: Simple patch
  • issue6233_py3k_with_test.diff: Combined patch with test case
  • issue6233-escape_entities.diff: Excp handling in _encode + prev submitted test
  • issue6233-encode_cdata.diff: _encode & effbot's _escape_cdata, w/ test
    		•

    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 2010-02-09.16:56:25.962>
created_at = <Date 2009-06-07.21:30:58.882>
labels = ['type-bug', 'library']
title = "ElementTree (py3k) doesn't properly encode characters that can't be represented in the specified encoding"
updated_at = <Date 2010-02-09.16:56:25.961>
user = 'https://bugs.python.org/NeilMuller'

    bugs.python.org fields:

    activity = <Date 2010-02-09.16:56:25.961>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2010-02-09.16:56:25.962>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2009-06-07.21:30:58.882>
creator = 'Neil Muller'
dependencies = []
files = ['14223', '14316', '14340', '14361']
hgrepos = []
issue_num = 6233
keywords = ['patch']
message_count = 16.0
messages = ['89058', '89059', '89276', '89504', '89505', '89583', '89585', '89623', '89684', '89690', '89715', '89718', '89722', '89728', '95045', '99125']
nosy_count = 8.0
nosy_names = ['loewis', 'effbot', 'rhettinger', 'pitrou', 'benjamin.peterson', 'hodgestar', 'Neil Muller', 'jcsalterego']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue6233'
versions = ['Python 3.1', 'Python 3.2']

    Activity

    1. NeilMuller commented on Jun 7, 2009

      @NeilMuller
      NeilMullermannequin
      on Jun 7, 2009
      MannequinAuthor

      In py3k, ElementTree no longer correctly converts characters to entities
      when they can't be represented in the requested output encoding.

      Python 2:

      >>> import xml.etree.ElementTree as ET
>>> e = ET.XML("<?xml version='1.0'
encoding='iso-8859-1'?><body>t\xe3t</body>")
>>> ET.tostring(e, 'ascii')
"<?xml version='1.0' encoding='ascii'?>\n<body>t&#227;t</body>"

      Python 3:

      >>> import xml.etree.ElementTree as ET
>>> e = ET.XML("<?xml version='1.0'
encoding='iso-8859-1'?><body>t\xe3t</body>")
>>> ET.tostring(e, 'ascii')
.....
UnicodeEncodeError: 'ascii' codec can't encode characters in position
1-2: ordinal not in range(128)

      It looks like _encode_entity isn't ever called inside ElementTree
      anymore - it probably should be called as part of _encode for characters
      that can't be represented.

    2. NeilMuller
      added
      stdlibStandard Library Python modules in the Lib/ directory
      type-bugAn unexpected behavior, bug, or error
      on Jun 7, 2009

    3. NeilMuller commented on Jun 7, 2009

      @NeilMuller
      NeilMullermannequin
      on Jun 7, 2009
      MannequinAuthor

      Simple possible patch uploaded

      This doesn't give the expected answer for the test above, but does work
      when starting from an XML file in utf-8 encoding. I still need to
      determine why this happens.

    4. NeilMuller commented on Jun 12, 2009

      @NeilMuller
      NeilMullermannequin
      on Jun 12, 2009
      MannequinAuthor

      This doesn't give the expected answer for the test above

      Which is obviously due to not comparing apples with apples, as I should
      be using a byte-string in the py3k example.

      >>> import xml.etree.ElementTree as ET
>>> e = ET.XML(b"<?xml version='1.0'
encoding='iso-8859-1'?><body>t\xe3t</body>")
>>> ET.tostring(e, 'ascii')

      Fails without the patch, behaves as expected with the patch.

    5. NeilMuller commented on Jun 18, 2009

      @NeilMuller
      NeilMullermannequin
      on Jun 18, 2009
      MannequinAuthor

      Updated patch - adds a test for this.

    6. pitrou commented on Jun 18, 2009

      @pitrou
      pitrou
      on Jun 18, 2009
      Member

      This regression is probably annoying enough to make it a blocker.

    7. pitrou
      added
      release-blocker
      on Jun 18, 2009

    8. effbot commented on Jun 21, 2009

      @effbot
      effbotmannequin
      on Jun 21, 2009
      Mannequin

      Umm. Isn't _encode used to encode tags and attribute names? The charref
      syntax is only valid in CDATA sections and attribute values, which are
      encoded by the corresponding _escape functions. I suspect this patch will
      make things blow up on a non-ASCII tag/attribute name.

    9. effbot commented on Jun 21, 2009

      @effbot
      effbotmannequin
      on Jun 21, 2009
      Mannequin

      Did you look at the 1.3 alpha code base when you came up with this idea?
      Unfortunately, 1.3's _encode is used for a different purpose...

      I don't have time to test it tonight, but I suspect that 1.3's
      escape_data/escape_attrib functions might work better under 3.X; they do
      the text.replace dance first, and then an explicit text.encode(encoding,
      "xmlcharrefreplace") at the end. E.g.

      def _escape_cdata(text, encoding):
    # escape character data
    try:
        # it's worth avoiding do-nothing calls for strings that are
        # shorter than 500 character, or so.  assume that's, by far,
        # the most common case in most applications.
        if "&" in text:
            text = text.replace("&", "&amp;")
        if "<" in text:
            text = text.replace("<", "&lt;")
        if ">" in text:
            text = text.replace(">", "&gt;")
        return text.encode(encoding, "xmlcharrefreplace")
    except (TypeError, AttributeError):
        _raise_serialization_error(text)

    10. jcsalterego commented on Jun 23, 2009

      @jcsalterego
      jcsalteregomannequin
      on Jun 23, 2009
      Mannequin

      The attached patch includes Neil's original additions to test_xml_etree.py.

      I also noticed that _encode_entity wasn't being called in ElementTree in
      py3k, with the important bit being the nested function
      escape_entities(), in conjunction with _escape and _escape_map.

      In 2.x, _encode_entity() is used after _encode() throws Unicode
      exceptions [1], so I figured it would make sense to take the core
      functionality of _escape_entities() and integrate it into _encode in the
      same fashion -- when an exception is thrown.

      Basically, I:

      • changed _escape regexp from using "[\x0080-\uffff]" to "[\x80-xff]"
      • extracted _encode_entity.escape_entities() and made it
        _escape_entities of module scope
      • removed _encode_entity()
      • added UnicodeEncodeError exception in _encode()

      I'm not sure what the expected outcome is supposed to be when the text
      is not type bytes but str. With this patch, the output has
      b"tãt" rather than b"tãt".

      Hope this is a step in the right direction.

      [1] ElementTree.py:814, ElementTree.py:829, python 2.7 HEAD r50941

    11. effbot commented on Jun 24, 2009

      @effbot
      effbotmannequin
      on Jun 24, 2009
      Mannequin

      That's backwards, unless I'm missing something here: charrefs represent
      Unicode characters, not UTF-8 byte values. The character "LATIN SMALL
      LETTER A WITH TILDE" with the character value 227 should be represented as
      "ã" if serialized to an encoding that doesn't support non-ASCII
      characters.

      And there's no need to use RE:s to filter things under 3.X; those parts of
      ET 1.2 are there for pre-2.0 compatibility.

      Did you try running the tests with the escape function I posted?

    12. jcsalterego commented on Jun 24, 2009

      @jcsalterego
      jcsalteregomannequin
      on Jun 24, 2009
      Mannequin

      Thanks for the explanation -- looks like I was way off base on that one.

      I took a look at the code you provided but it doesn't work as a drop-in
      replacement for _escape_cdata, since that function returns a string
      rather than bytes.

      However taking your code, calling it _encode_cdata and then refactoring
      all calls _encode(_escape_cdata(x), encoding) to _encode_cdata(x,
      encoding) seems to do the trick and passes the tests.

      Specific example:

      • file.write(_encode(_escape_cdata(node.text), encoding))
        + file.write(_encode_cdata(node.text, encoding))

      One minor modification is to return the string as is if encoding=None,
      just like _encode:

      def _encode_cdata(text, encoding):
    # escape character data
    try:
        text = text.replace("&", "&amp;")
        text = text.replace("<", "&lt;")
        text = text.replace(">", "&gt;")
        if encoding:
            return text.encode(encoding, "xmlcharrefreplace")
        else:
            return text
    except (TypeError, AttributeError):
        _raise_serialization_error(text)

    13. benjaminp commented on Jun 25, 2009

      @benjaminp
      benjaminp
      on Jun 25, 2009
      Contributor

      effbot, do you have an opinion about the latest patch? It'd be nice to
      not have to delay the release for this.

    14. loewis commented on Jun 26, 2009

      @loewis
      loewismannequin
      on Jun 26, 2009
      Mannequin

      I disagree with this report being classified as release-critical - it is
      *not* a regression over 3.0 (i.e. 3.0 already behaved in the same way).
      That it is a regression relative to 2.x should not make it
      release-critical - we can still fix such regressions in 3.2.

      In addition, there is an easy work-around for applications that run into
      the problem - just use utf-8 as the output encoding always:

      py> e = ET.XML(b"<?xml version='1.0'
      encoding='iso-8859-1'?><body>t\xe3t</body>")
      py> ET.tostring(e,encoding='utf-8')
      b'<body>t\xc3\xa3t</body>'

    15. rhettinger commented on Jun 26, 2009

      @rhettinger
      rhettinger
      on Jun 26, 2009
      Contributor

      +1 for Py3.1.1

    16. pitrou
      removed
      release-blocker
      on Jun 26, 2009

    17. jcsalterego commented on Jun 26, 2009

      @jcsalterego
      jcsalteregomannequin
      on Jun 26, 2009
      Mannequin

      Either way, it would be nice to get feedback so we can iterate on the
      patch or close out this issue already :-)

    18. pitrou commented on Nov 8, 2009

      @pitrou
      pitrou
      on Nov 8, 2009
      Member

      The patch looks ok to me.

    19. pitrou commented on Feb 9, 2010

      @pitrou
      pitrou
      on Feb 9, 2010
      Member

      Committed in r78123 (py3k) and r78124 (3.1). I've also removed _escape_cdata() since it wasn't used anymore. Thanks Jerry for the patch.

    20. pitrou
      closed this as completedon Feb 9, 2010
    21. ezio-melotti
      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

    Assignees

    No one assigned

      Labels

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

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions