GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-149489: Fix ElementTree serialization to HTML by serhiy-storchaka · Pull Request #149490 · python/cpython

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 7, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • The content of comments, processing instructions and elements "xmp", "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
  • The "plaintext" element no longer have the closing tag.
  • Add support of empty attributes (with value None).
  • Issue: Fix ElementTree serialization to HTML #149489

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot mentioned this pull request May 7, 2026
Fix ElementTree serialization to HTML #149489
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label May 7, 2026
@serhiy-storchaka
serhiy-storchaka requested review from ezio-melotti and scoder May 7, 2026 11:21
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels May 7, 2026
@serhiy-storchaka
pythongh-149489: Fix ElementTree serialization to HTML
e41d61c 
* The content of comments, processing instructions and elements "xmp",
  "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.
* Add support of empty attributes (with value None).
@serhiy-storchaka
serhiy-storchaka force-pushed the xml-etree-serialize-html branch from 9f169a8 to e41d61c Compare May 7, 2026 17:22
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 7, 2026
gh-149468: Add option to validate ElementTree during serialization #149469
Open
@StanFromIreland

StanFromIreland commented May 7, 2026

Copy link
Copy Markdown
Member

Updating to fix some errors we introduced on the main branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@StanFromIreland
Merge branch 'main' into xml-etree-serialize-html
a9795d1
@StanFromIreland StanFromIreland added the needs backport to 3.15 bugs and security fixes label May 7, 2026
@scoder

scoder commented May 8, 2026

Copy link
Copy Markdown
Contributor

I'm not done with the review yet, but I find it risky to silently change output in a point release like 3.1[34].x. If we change this, it's probably still fine for 3.15, but I'd rather see the maintenance releases excluded.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented May 8, 2026

Copy link
Copy Markdown
Member Author

Well, the fix for XML was applied to 2.7 and 3.2 and was not backported to 2.6 and 3.1. It introduces a risk of XML/HTML injection if the comment content was not previously sanitized. See #149468.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@scoder

scoder commented May 8, 2026

Copy link
Copy Markdown
Contributor

The content of comments, processing instructions and elements "xmp", "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
[...] It introduces a risk of XML/HTML injection if the comment content was not previously sanitized.

Well, yes. If we remove the current escaping, then we leave user code unprotected. Definitely not something that users should expect from a point release.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented May 10, 2026

Copy link
Copy Markdown
Member Author

Test failures are related to #149571.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka added 2 commits May 28, 2026 18:09
@serhiy-storchaka
Merge branch 'main' into xml-etree-serialize-html
0f35342
@serhiy-storchaka
Update tests after fixing itertext().
e111ef9
ezio-melotti
ezio-melotti reviewed May 28, 2026
View reviewed changes
Comment thread Lib/test/test_xml_etree.py

@ezio-melotti ezio-melotti May 28, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It would be good to add some comments to clearly state the intent of the tests (e.g. comments shouldn't be excaped, comments are omitted in text serialization, empty attrs only work in html, etc.).
  2. Is there any reason why the there are 3 similar but different test strings ('<spam> & ham', 'ham & eggs < spam', '<spam>&ham')?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/xml/etree/ElementTree.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka and others added 2 commits May 29, 2026 09:14
@serhiy-storchaka @ezio-melotti
Apply suggestion from @ezio-melotti
752f4e7 
Co-authored-by: Ezio Melotti <[email protected]>
@serhiy-storchaka
Add comments to tests.
57b5739
ezio-melotti
ezio-melotti approved these changes May 29, 2026
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 29, 2026
@serhiy-storchaka
serhiy-storchaka merged commit bcd29e4 into python:main May 29, 2026
59 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label May 29, 2026
@miss-islington-app

miss-islington-app Bot commented May 29, 2026

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
serhiy-storchaka deleted the xml-etree-serialize-html branch May 29, 2026 21:04
@serhiy-storchaka serhiy-storchaka removed needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels May 29, 2026
@serhiy-storchaka serhiy-storchaka removed the needs backport to 3.15 bugs and security fixes label May 29, 2026
@bedevere-app

bedevere-app Bot commented May 29, 2026

Copy link
Copy Markdown

GH-150592 is a backport of this pull request to the 3.15 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented May 29, 2026

Copy link
Copy Markdown

GH-150593 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented May 29, 2026

Copy link
Copy Markdown

GH-150594 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.15 bugs and security fixes label May 29, 2026
@miss-islington-app

miss-islington-app Bot commented May 29, 2026

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2026
@serhiy-storchaka @miss-islington
pythongh-149489: Fix ElementTree serialization to HTML (pythonGH-149490)
26f9c5f 
* The content of comments, processing instructions and elements "xmp",
  "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.
* Add support of empty attributes (with value None).
(cherry picked from commit bcd29e4)

Co-authored-by: Serhiy Storchaka <[email protected]>
@miss-islington-app

miss-islington-app Bot commented May 29, 2026

Copy link
Copy Markdown

Sorry @serhiy-storchaka, I had trouble completing the backport.
Please retry by removing and re-adding the "needs backport to 3.15" label.
Please backport backport using cherry_picker on the command line.

cherry_picker bcd29e466f55d8b4e3849ed6ada8ce86a46f5072 3.15
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned serhiy-storchaka May 29, 2026
@serhiy-storchaka serhiy-storchaka added needs backport to 3.15 bugs and security fixes and removed needs backport to 3.15 bugs and security fixes labels May 29, 2026
@miss-islington-app

miss-islington-app Bot commented May 29, 2026

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2026
@serhiy-storchaka @miss-islington
pythongh-149489: Fix ElementTree serialization to HTML (pythonGH-149490)
6efc3f3 
* The content of comments, processing instructions and elements "xmp",
  "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.
* Add support of empty attributes (with value None).
(cherry picked from commit bcd29e4)

Co-authored-by: Serhiy Storchaka <[email protected]>
@miss-islington-app

miss-islington-app Bot commented May 29, 2026

Copy link
Copy Markdown

Sorry @serhiy-storchaka, I had trouble completing the backport.
Please retry by removing and re-adding the "needs backport to 3.15" label.
Please backport backport using cherry_picker on the command line.

cherry_picker bcd29e466f55d8b4e3849ed6ada8ce86a46f5072 3.15
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented May 29, 2026

Copy link
Copy Markdown

GH-150595 is a backport of this pull request to the 3.15 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label May 29, 2026
serhiy-storchaka added a commit that referenced this pull request May 29, 2026
@serhiy-storchaka
[3.15] gh-149489: Fix ElementTree serialization to HTML (GH-149490) (G…
2f91315 
…H-150595)

* The content of comments, processing instructions and elements "xmp",
  "iframe", "noembed", "noframes", and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.
* Add support of empty attributes (with value None).
(cherry picked from commit bcd29e4)
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 29, 2026
@serhiy-storchaka
[3.14] pythongh-149489: Fix ElementTree serialization to HTML (python…
de5e730 
…GH-149490) (pythonGH-150595)

* The content of elements "xmp", "iframe", "noembed", "noframes",
  and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.

(cherry picked from commit bcd29e4)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 29, 2026
@serhiy-storchaka
[3.14] pythongh-149489: Fix ElementTree serialization to HTML (python…
f596830 
…GH-149490)

* The content of elements "xmp", "iframe", "noembed", "noframes",
  and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.

(cherry picked from commit bcd29e4)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app

bedevere-app Bot commented May 29, 2026

Copy link
Copy Markdown

GH-150596 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka added a commit that referenced this pull request May 30, 2026
@serhiy-storchaka
[3.14] gh-149489: Fix ElementTree serialization to HTML (GH-149490) (G…
c42e6d3 
…H-150596)

* The content of elements "xmp", "iframe", "noembed", "noframes",
  and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.

(cherry picked from commit bcd29e4)
serhiy-storchaka added a commit that referenced this pull request May 30, 2026
@miss-islington @serhiy-storchaka
[3.13] gh-149489: Fix ElementTree serialization to HTML (GH-149490) (G…
b46df7f 
…H-150596) (GH-150609)

* The content of elements "xmp", "iframe", "noembed", "noframes",
  and "plaintext" is no longer escaped.
* The "plaintext" element no longer have the closing tag.
(cherry picked from commit c42e6d3)


(cherry picked from commit bcd29e4)

Co-authored-by: Serhiy Storchaka <[email protected]>
KowalskiThomas
KowalskiThomas reviewed Jun 1, 2026
View reviewed changes
Comment thread Lib/xml/etree/ElementTree.py
Comment on lines 930 to +931
if tag is Comment:
write("<!--%s-->" % _escape_cdata(text))
write("<!--%s-->" % text)

@KowalskiThomas KowalskiThomas Jun 1, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit late to the party, but what happens if text contains -->? Shouldn't this be escaped (if there's a way to escape it, I don't think so 🤔) or probably instead be rejected altogether?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/xml/etree/ElementTree.py
write("<!--%s-->" % text)
elif tag is ProcessingInstruction:
write("<?%s?>" % _escape_cdata(text))
write("<?%s?>" % text)

@KowalskiThomas KowalskiThomas Jun 1, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess same question here -- what if text contains ?>?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@scoder

scoder commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

what happens if text contains --> ?

Yes, that was my concern in #149490 (comment) as well. Note that this part of the changes was not backported and will only appear in 3.15+.

The HTML5 spec is explicit about what is not allowed in comments:
https://html.spec.whatwg.org/#comments

It's not clear about what is allowed in processing instructions, but that probably just means that ?> should be rejected and nothing else. PIs are rather relaxed by design.

I think it's worth rejecting non well-formed text. lxml does it in the API when creating a Comment / ProcessingInstruction element, but for xml.etree, it might be more reasonable to reject it in the serialiser.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka serhiy-storchaka removed their assignment Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezio-melotti ezio-melotti ezio-melotti approved these changes
@scoder scoder Awaiting requested review from scoder
+1 more reviewer
@KowalskiThomas KowalskiThomas KowalskiThomas left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@serhiy-storchaka @StanFromIreland @scoder @KowalskiThomas @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.