From c472246d81f2e9e7842c0d6e663ea01429e2efa9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 26 Nov 2015 23:49:42 +0200 Subject: [PATCH] Issue #10131: Fixed deep copying of minidom documents. Based on patch by Marian Ganisin. --- Lib/test/test_minidom.py | 94 ++++++++++++++++------------- Lib/test/test_xml_dom_minicompat.py | 36 +++++++++++ Lib/xml/dom/minicompat.py | 6 +- Misc/NEWS | 3 + 4 files changed, 94 insertions(+), 45 deletions(-) diff --git a/Lib/test/test_minidom.py b/Lib/test/test_minidom.py index 05df6e93962..f64f43803e7 100644 --- a/Lib/test/test_minidom.py +++ b/Lib/test/test_minidom.py @@ -1,5 +1,6 @@ # test for xml.dom.minidom +import copy import pickle from test.support import run_unittest, findfile import unittest @@ -11,6 +12,13 @@ from xml.dom.minidom import getDOMImplementation tstfile = findfile("test.xml", subdir="xmltestdata") +sample = ("\n" + "\n" + " \n" + "]> text\n" + " ") # The tests of DocumentType importing use these helpers to construct # the documents to work with, since not all DOM builders actually @@ -1466,52 +1474,54 @@ class MinidomTest(unittest.TestCase): self.confirm(e.isSameNode(doc.getElementById("w")) and a2.isId) + def assert_recursive_equal(self, doc, doc2): + stack = [(doc, doc2)] + while stack: + n1, n2 = stack.pop() + self.assertEqual(n1.nodeType, n2.nodeType) + self.assertEqual(len(n1.childNodes), len(n2.childNodes)) + self.assertEqual(n1.nodeName, n2.nodeName) + self.assertFalse(n1.isSameNode(n2)) + self.assertFalse(n2.isSameNode(n1)) + if n1.nodeType == Node.DOCUMENT_TYPE_NODE: + len(n1.entities) + len(n2.entities) + len(n1.notations) + len(n2.notations) + self.assertEqual(len(n1.entities), len(n2.entities)) + self.assertEqual(len(n1.notations), len(n2.notations)) + for i in range(len(n1.notations)): + # XXX this loop body doesn't seem to be executed? + no1 = n1.notations.item(i) + no2 = n1.notations.item(i) + self.assertEqual(no1.name, no2.name) + self.assertEqual(no1.publicId, no2.publicId) + self.assertEqual(no1.systemId, no2.systemId) + stack.append((no1, no2)) + for i in range(len(n1.entities)): + e1 = n1.entities.item(i) + e2 = n2.entities.item(i) + self.assertEqual(e1.notationName, e2.notationName) + self.assertEqual(e1.publicId, e2.publicId) + self.assertEqual(e1.systemId, e2.systemId) + stack.append((e1, e2)) + if n1.nodeType != Node.DOCUMENT_NODE: + self.assertTrue(n1.ownerDocument.isSameNode(doc)) + self.assertTrue(n2.ownerDocument.isSameNode(doc2)) + for i in range(len(n1.childNodes)): + stack.append((n1.childNodes[i], n2.childNodes[i])) + def testPickledDocument(self): - doc = parseString("\n" - "\n" - " \n" - "]> text\n" - " ") + doc = parseString(sample) for proto in range(2, pickle.HIGHEST_PROTOCOL + 1): s = pickle.dumps(doc, proto) doc2 = pickle.loads(s) - stack = [(doc, doc2)] - while stack: - n1, n2 = stack.pop() - self.confirm(n1.nodeType == n2.nodeType - and len(n1.childNodes) == len(n2.childNodes) - and n1.nodeName == n2.nodeName - and not n1.isSameNode(n2) - and not n2.isSameNode(n1)) - if n1.nodeType == Node.DOCUMENT_TYPE_NODE: - len(n1.entities) - len(n2.entities) - len(n1.notations) - len(n2.notations) - self.confirm(len(n1.entities) == len(n2.entities) - and len(n1.notations) == len(n2.notations)) - for i in range(len(n1.notations)): - # XXX this loop body doesn't seem to be executed? - no1 = n1.notations.item(i) - no2 = n1.notations.item(i) - self.confirm(no1.name == no2.name - and no1.publicId == no2.publicId - and no1.systemId == no2.systemId) - stack.append((no1, no2)) - for i in range(len(n1.entities)): - e1 = n1.entities.item(i) - e2 = n2.entities.item(i) - self.confirm(e1.notationName == e2.notationName - and e1.publicId == e2.publicId - and e1.systemId == e2.systemId) - stack.append((e1, e2)) - if n1.nodeType != Node.DOCUMENT_NODE: - self.confirm(n1.ownerDocument.isSameNode(doc) - and n2.ownerDocument.isSameNode(doc2)) - for i in range(len(n1.childNodes)): - stack.append((n1.childNodes[i], n2.childNodes[i])) + self.assert_recursive_equal(doc, doc2) + + def testDeepcopiedDocument(self): + doc = parseString(sample) + doc2 = copy.deepcopy(doc) + self.assert_recursive_equal(doc, doc2) def testSerializeCommentNodeWithDoubleHyphen(self): doc = create_doc_without_doctype() diff --git a/Lib/test/test_xml_dom_minicompat.py b/Lib/test/test_xml_dom_minicompat.py index 47c4de62e05..3b03dfc5399 100644 --- a/Lib/test/test_xml_dom_minicompat.py +++ b/Lib/test/test_xml_dom_minicompat.py @@ -1,5 +1,6 @@ # Tests for xml.dom.minicompat +import copy import pickle import unittest @@ -89,6 +90,7 @@ class NodeListTestCase(unittest.TestCase): node_list = NodeList() pickled = pickle.dumps(node_list, proto) unpickled = pickle.loads(pickled) + self.assertIsNot(unpickled, node_list) self.assertEqual(unpickled, node_list) # Non-empty NodeList. @@ -96,7 +98,41 @@ class NodeListTestCase(unittest.TestCase): node_list.append(2) pickled = pickle.dumps(node_list, proto) unpickled = pickle.loads(pickled) + self.assertIsNot(unpickled, node_list) self.assertEqual(unpickled, node_list) + def test_nodelist_copy(self): + # Empty NodeList. + node_list = NodeList() + copied = copy.copy(node_list) + self.assertIsNot(copied, node_list) + self.assertEqual(copied, node_list) + + # Non-empty NodeList. + node_list.append([1]) + node_list.append([2]) + copied = copy.copy(node_list) + self.assertIsNot(copied, node_list) + self.assertEqual(copied, node_list) + for x, y in zip(copied, node_list): + self.assertIs(x, y) + + def test_nodelist_deepcopy(self): + # Empty NodeList. + node_list = NodeList() + copied = copy.deepcopy(node_list) + self.assertIsNot(copied, node_list) + self.assertEqual(copied, node_list) + + # Non-empty NodeList. + node_list.append([1]) + node_list.append([2]) + copied = copy.deepcopy(node_list) + self.assertIsNot(copied, node_list) + self.assertEqual(copied, node_list) + for x, y in zip(copied, node_list): + self.assertIsNot(x, y) + self.assertEqual(x, y) + if __name__ == '__main__': unittest.main() diff --git a/Lib/xml/dom/minicompat.py b/Lib/xml/dom/minicompat.py index 1244500259d..5d6fae9a257 100644 --- a/Lib/xml/dom/minicompat.py +++ b/Lib/xml/dom/minicompat.py @@ -64,10 +64,10 @@ class NodeList(list): length = property(_get_length, _set_length, doc="The number of nodes in the NodeList.") - def __getstate__(self): - return list(self) - + # For backward compatibility def __setstate__(self, state): + if state is None: + state = [] self[:] = state diff --git a/Misc/NEWS b/Misc/NEWS index 9dc0aae2f5f..95f2626f26e 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -113,6 +113,9 @@ Core and Builtins Library ------- +- Issue #10131: Fixed deep copying of minidom documents. Based on patch + by Marian Ganisin. + - Issue #25725: Fixed a reference leak in pickle.loads() when unpickling invalid data including tuple instructions.