gh-128302: fix apparent bug in xml.dom.xmlbuilder.DOMBuilder.parse()
#128284
Conversation
|
Considering the fact that we use |
|
I'll put together a more thorough analysis. |
tungol
changed the title
xml.dom.xmlbuilder.DOMBuilder.parse()xml.dom.xmlbuilder.DOMBuilder.parse()
|
Please see issue #128302 for a full write-up and analysis of what's going on here. |
|
I'll look at adding some tests. There's no existing tests for this module, but you have to start somewhere I suppose. Would it be better if this was two MRs? I'm happy to split it if so. |
|
Yes better to do it in another PR for the tests |
There was a problem hiding this comment.
I see now that it is difficult to add new tests, because there are no any tests for xmlbuilder at all. And adding meaningful tests is a larger issue, much larger than this bug fix. But we need any test, just to check that the new code works and the old code does not work. Even if they are only two test in a new file.
Co-authored-by: Serhiy Storchaka <[email protected]>
|
Here are some tests for the changed behavior. I'm a little reluctant to split this MR in two now that I've added |
|
If it's too hard, let's do it in one PR if Serhiy agrees. I don't think there will be conflicts though but the file looks smalle enough. |
serhiy-storchaka
added
needs backport to 3.12
|
Thanks @tungol for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @tungol for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
* Allow DOMParser.parse() to correctly handle DOMInputSource instances that only have a systemId attribute set. * Fix DOMEntityResolver.resolveEntity(), which was broken by the Python 3.0 transition. * Add Lib/test/test_xml_dom_xmlbuilder.py with few tests. (cherry picked from commit 6ea04da) Co-authored-by: Stephen Morton <[email protected]>
* Allow DOMParser.parse() to correctly handle DOMInputSource instances that only have a systemId attribute set. * Fix DOMEntityResolver.resolveEntity(), which was broken by the Python 3.0 transition. * Add Lib/test/test_xml_dom_xmlbuilder.py with few tests. (cherry picked from commit 6ea04da) Co-authored-by: Stephen Morton <[email protected]>
|
GH-128582 is a backport of this pull request to the 3.13 branch. |
|
GH-128583 is a backport of this pull request to the 3.12 branch. |
* Allow DOMParser.parse() to correctly handle DOMInputSource instances that only have a systemId attribute set. * Fix DOMEntityResolver.resolveEntity(), which was broken by the Python 3.0 transition. * Add Lib/test/test_xml_dom_xmlbuilder.py with few tests.
gh-128302: Fix bugs in xml.dom.xmlbuilder (GH-128284) * Allow DOMParser.parse() to correctly handle DOMInputSource instances that only have a systemId attribute set. * Fix DOMEntityResolver.resolveEntity(), which was broken by the Python 3.0 transition. * Add Lib/test/test_xml_dom_xmlbuilder.py with few tests. (cherry picked from commit 6ea04da) Co-authored-by: Stephen Morton <[email protected]>
gh-128302: Fix bugs in xml.dom.xmlbuilder (GH-128284) * Allow DOMParser.parse() to correctly handle DOMInputSource instances that only have a systemId attribute set. * Fix DOMEntityResolver.resolveEntity(), which was broken by the Python 3.0 transition. * Add Lib/test/test_xml_dom_xmlbuilder.py with few tests. (cherry picked from commit 6ea04da) Co-authored-by: Stephen Morton <[email protected]>
|
This looks like a bug. The
Optionsclass has nosystemIdattribute, but the input parameter, which seems to be an instance ofDOMInputSource, does.I don't really know how this is meant to be used, but given this code:
As-is this raises an error:
With this change:
I'd consider adding a test, but there's no tests right now for any of this.
xml.dom.xmlbuilder.DOMBuilder.parse()#128302