ElementTree Comment text isn't escaped

[johnburnett]

BPO	33303
Nosy	@scoder, @johnburnett, @websurfer5
PRs	bpo-33303: ElementTree Comment text isn't escaped #13229

^{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 2019-05-12.07:38:02.602>
created_at = <Date 2018-04-18.00:33:12.408>
labels = ['expert-XML', 'invalid', 'type-bug', '3.8']
title = "ElementTree Comment text isn't escaped"
updated_at = <Date 2019-05-12.07:38:02.600>
user = 'https://github.com/johnburnett'

bugs.python.org fields:

activity = <Date 2019-05-12.07:38:02.600>
actor = 'scoder'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-12.07:38:02.602>
closer = 'scoder'
components = ['XML']
creation = <Date 2018-04-18.00:33:12.408>
creator = 'johnburnett'
dependencies = []
files = []
hgrepos = []
issue_num = 33303
keywords = ['patch']
message_count = 6.0
messages = ['315428', '340990', '342145', '342168', '342243', '342244']
nosy_count = 4.0
nosy_names = ['scoder', 'eli.bendersky', 'johnburnett', 'Jeffrey.Kintscher']
pr_nums = ['13229']
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue33303'
versions = ['Python 3.8']

[johnburnett]

The _serialize_xml function in ElementTree.py doesn't escape Comment.text values when writing output. This means the following code:

    import sys
    import xml.etree.ElementTree
    elem = xml.etree.ElementTree.Comment()
    elem.text = 'hi --> bye'
    tree = xml.etree.ElementTree.ElementTree(elem)
    tree.write(sys.stdout)

...will output the following invalid xml:

<!--hi --> bye-->

In Python 3.7, changing the _serialize_xml function on line 903/904 from this:

    if tag is Comment:
        write("<!--%s-->" % text)

...to this:

    if tag is Comment:
        write("<!--%s-->" % _escape_cdata(text))

...writes something more expected:

<!--hi --&gt; bye-->

[scoder]

Yes, comment text should be escaped internally like all other text, not by the user. The same applies to processing instructions.

This suggests that it's probably also untested currently. Could you provide a PR for that changes both and adds tests?

[websurfer5]

Which characters would you escape in a processing instruction? The XML spec says not to escape '<' and '&', but doesn't say what should be escaped. It seems to me that escaping '?' when followed by '>' should be sufficient since the character sequence "?>" is the closing delimiter.

[scoder]

Hmm, sorry, I was wrong here. I looked it up and also checked the behaviour of other libraries: the data content of PIs is application specific and must not be escaped, at all. It's not XML character data.

Sorry for the confusion and the extra work on your side.

[websurfer5]

I backed-out the Processing Instruction changes.

[scoder]

I'm really sorry again, but I only consulted the XML spec on this now (and also the way libxml2 does it), and I found that XML comment text actually does not get escaped. It's not character data, and, in fact, "--" is not even allowed at all inside of comments. (Funny enough, the HTML serialiser does escaping for both comments and PIs, but, well, that's HTML, I guess…)

https://www.w3.org/TR/REC-xml/#sec-comments

Sorry, Jeffrey, I should have looked that up in the spec much earlier, before you invested so much time into this.

There are two disallowed cases: "--" in the text content, and "-" at the end of the text (which would lead to an "--->"). Now, the thing is, such validation is currently unprecedented in ElementTree, so I don't know if we should start raising exceptions from the serialiser for this case, and if yes, which. Since comments are rare, it won't hurt performance to do that, but once we get started on this, users would probably also want their text and attribute content and their tag and attribute names to be validated, and that would hurt then.

So, I will have to reject the PR and this ticket.