Skip to content

gh-135640: Adds type checking to ElementTree.ElementTree constructor #135643

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+44 −2
Open

gh-135640: Adds type checking to ElementTree.ElementTree constructor #135643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b0a8a64
Tenuous first fix
abstractedfox Jun 17, 2025
f0356c8
Remove commented assertions
abstractedfox Jun 17, 2025
06eaff5
Add tests for ElementTree constructor
abstractedfox Jun 17, 2025
6d4d65f
Add appropriate text to an exception
abstractedfox Jun 17, 2025
84b22d4
Make iselement stricter + incorporate feedback
abstractedfox Jun 22, 2025
1d2f0d9
Revert iselement to previous state
abstractedfox Jun 26, 2025
d5bbe23
Add iselement check to ElementTree.write
abstractedfox Jun 26, 2025
4e2f822
Make errors more descriptive
abstractedfox Jun 26, 2025
8b588f3
Fix whitespace
abstractedfox Jun 26, 2025
e1e489e
Rephrase news entry
abstractedfox Jun 26, 2025
28fcc69
Test: remove unnecessary fields for element-like
abstractedfox Jun 26, 2025
78118ea
Change error messages
abstractedfox Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Lib/test/test_xml_etree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,34 @@ class ElementTreeTest(unittest.TestCase):
def serialize_check(self, elem, expected):
self.assertEqual(serialize(elem), expected)

def test_constructor(self):
# Test constructor behavior.

with self.assertRaises(TypeError):
tree = ET.ElementTree("")
with self.assertRaises(TypeError):
tree = ET.ElementTree(ET.ElementTree())

# Test _setroot as well, since it also sets the _root object.

tree = ET.ElementTree()
with self.assertRaises(TypeError):
tree._setroot("")
with self.assertRaises(TypeError):
tree._setroot(ET.ElementTree())
Comment on lines +232 to +235
Copy link
Member

Choose a reason for hiding this comment

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

Test also _setroot(None).

_setroot() needs a separate test. It should also be tested with valid argument. getroot() should be used to check its effect.


# Make sure it accepts an Element-like object.

class ElementLike:
def __init__(self):
self.tag = "tag"

element_like = ElementLike()
try:
tree = ET.ElementTree(element_like)
except Exception as err:
self.fail(err)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe test also _setroot() here?

Comment on lines +246 to +247
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed.


def test_interface(self):
# Test element tree interface.

Expand Down
14 changes: 12 additions & 2 deletions Lib/xml/etree/ElementTree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,10 @@ class ElementTree:

"""
def __init__(self, element=None, file=None):
# assert element is None or iselement(element)
if element is not None and not iselement(element):
raise TypeError(f"expected an xml.etree.ElementTree.Element or "
f"Element-like object, not "
f"{type(element).__name__}")
Comment on lines +531 to +533
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raise TypeError(f"expected an xml.etree.ElementTree.Element or "
f"Element-like object, not "
f"{type(element).__name__}")
raise TypeError('expected an Element, not %s' % type(e).__name__)

self._root = element # first node
if file:
self.parse(file)
Expand All @@ -543,7 +546,10 @@ def _setroot(self, element):
with the given element. Use with care!

"""
# assert iselement(element)
if not iselement(element):
raise TypeError(f"expected an xml.etree.ElementTree.Element or "
f"Element-like object, not "
f"{type(element).__name__}")
Comment on lines +550 to +552
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raise TypeError(f"expected an xml.etree.ElementTree.Element or "
f"Element-like object, not "
f"{type(element).__name__}")
raise TypeError('expected an Element, not %s' % type(e).__name__)

self._root = element

def parse(self, source, parser=None):
Expand Down Expand Up @@ -709,6 +715,10 @@ def write(self, file_or_filename,
of start/end tags

"""
if not iselement(self._root):
raise TypeError(f"Root element must be "
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need for f-strings.

f"xml.etree.ElementTree.Element "
f"or Element-like object")
Comment on lines +718 to +721
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if not iselement(self._root):
raise TypeError(f"Root element must be "
f"xml.etree.ElementTree.Element "
f"or Element-like object")
if self._root is None:
raise TypeError('ElementTree not initialized')

Copy link
Member

Choose a reason for hiding this comment

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

Please add also a test for writing uninitialized ElementTree:

    def test_write_uninitialized(self):
        tree = ET.ElementTree()
        self.assertRaises(TypeError, tree.write, TESTFN)
        self.assertFalse(os.path.exists(TESTFN))

if not method:
method = "xml"
elif method not in _serialize:
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2025-06-22-02-16-17.gh-issue-135640.FXyFL6.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Address bug where it was possible to call
:func:`xml.etree.ElementTree.ElementTree.write` on an ElementTree object with
an invalid root element. This behavior blanked the file passed to ``write``
if it already existed.
Loading