Skip to content

bpo-35052: Fix handler on xml.dom.minidom.cloneNode()#11061

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:minidom_entity_handler
Dec 10, 2018
Merged

bpo-35052: Fix handler on xml.dom.minidom.cloneNode()#11061
vstinner merged 2 commits into
python:masterfrom
vstinner:minidom_entity_handler

Conversation

@vstinner

@vstinner vstinner commented Dec 10, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Fix xml.dom.minidom cloneNode() on a document with an entity: pass
the correct arguments to the user data handler of an entity (fix an
old copy/paste mistake).

Bug spotted and fix proposed by Charalampos Stratakis, initial
reproducer written by Petr Viktorin.

Co-Authored-By: Charalampos Stratakis cstratak@redhat.com
Co-Authored-By: Petr Viktorin encukou@gmail.com

https://bugs.python.org/issue35052

@vstinner @stratakis @encukou
bpo-35052: Fix handler on xml.dom.minidom.cloneNode()
633c325 
Fix xml.dom.minidom cloneNode() on a document with an entity: pass
the correct arguments to the user data handler of an entity (fix an
old copy/paste mistake).

Bug spotted and fix proposed by Charalampos Stratakis, initial
reproducer written by Petr Viktorin.

Co-Authored-By: Charalampos Stratakis <cstratak@redhat.com>
Co-Authored-By: Petr Viktorin <encukou@gmail.com>
@vstinner

vstinner commented Dec 10, 2018

Copy link
Copy Markdown
Member Author

cc @stratakis @encukou

@taleinat

taleinat commented Dec 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

@vstinner, did you see that this same copy/paste error appears twice in that file? ISTM we should fix both instances.

See the other instance here.

@vstinner

vstinner commented Dec 10, 2018

Copy link
Copy Markdown
Member Author

@vstinner, did you see that this same copy/paste error appears twice in that file? ISTM we should fix both instances. See the other instance here.

No, I didn't. I should now be fixed as well.

taleinat
taleinat approved these changes Dec 10, 2018
View reviewed changes

@taleinat taleinat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, though I'm far from an expert on XML.

@vstinner

vstinner commented Dec 10, 2018

Copy link
Copy Markdown
Member Author

LGTM, though I'm far from an expert on XML.

I'm not an expert neither. There is no more xml.dom.minidom expert: https://devguide.python.org/experts/

Since I wrote a complete test, I'm now confident in my fix.

@vstinner vstinner merged commit 8e04186 into python:master Dec 10, 2018
@miss-islington

miss-islington commented Dec 10, 2018

Copy link
Copy Markdown
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

@vstinner vstinner deleted the minidom_entity_handler branch December 10, 2018 10:12
@miss-islington

miss-islington commented Dec 10, 2018

Copy link
Copy Markdown
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8e0418688906206fe59bd26344320c0fc026849e 3.7

@miss-islington

miss-islington commented Dec 10, 2018

Copy link
Copy Markdown
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8e0418688906206fe59bd26344320c0fc026849e 3.6

@miss-islington

miss-islington commented Dec 10, 2018

Copy link
Copy Markdown
Contributor

Sorry, @vstinner, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8e0418688906206fe59bd26344320c0fc026849e 2.7

@bedevere-bot

bedevere-bot commented Dec 10, 2018

Copy link
Copy Markdown

GH-11066 is a backport of this pull request to the 3.7 branch.

@bedevere-bot

bedevere-bot commented Dec 10, 2018

Copy link
Copy Markdown

GH-11067 is a backport of this pull request to the 3.6 branch.

@bedevere-bot

bedevere-bot commented Dec 10, 2018

Copy link
Copy Markdown

GH-11068 is a backport of this pull request to the 2.7 branch.

vstinner added a commit that referenced this pull request Dec 10, 2018
@vstinner @stratakis @encukou
bpo-35052: Fix handler on xml.dom.minidom.cloneNode() (GH-11061) (GH-…
3fd9755 
…11067)

Fix xml.dom.minidom cloneNode() on a document with an entity: pass
the correct arguments to the user data handler of an entity (fix an
old copy/paste mistake).

Bug spotted and fix proposed by Charalampos Stratakis, initial
reproducer written by Petr Viktorin.

Co-Authored-By: Charalampos Stratakis <cstratak@redhat.com>
Co-Authored-By: Petr Viktorin <encukou@gmail.com>
(cherry picked from commit 8e04186)
vstinner added a commit that referenced this pull request Dec 10, 2018
@vstinner @stratakis @encukou
bpo-35052: Fix handler on xml.dom.minidom.cloneNode() (GH-11061) (GH-…
c3cc751 
…11066)

Fix xml.dom.minidom cloneNode() on a document with an entity: pass
the correct arguments to the user data handler of an entity (fix an
old copy/paste mistake).

Bug spotted and fix proposed by Charalampos Stratakis, initial
reproducer written by Petr Viktorin.

Co-Authored-By: Charalampos Stratakis <cstratak@redhat.com>
Co-Authored-By: Petr Viktorin <encukou@gmail.com>
(cherry picked from commit 8e04186)
vstinner added a commit that referenced this pull request Dec 10, 2018
@vstinner @stratakis @encukou
bpo-35052: Fix handler on xml.dom.minidom.cloneNode() (GH-11061) (GH-…
cecf313 
…11068)

Fix xml.dom.minidom cloneNode() on a document with an entity: pass
the correct arguments to the user data handler of an entity (fix an
old copy/paste mistake).

Bug spotted and fix proposed by Charalampos Stratakis, initial
reproducer written by Petr Viktorin.

Co-Authored-By: Charalampos Stratakis <cstratak@redhat.com>
Co-Authored-By: Petr Viktorin <encukou@gmail.com>
(cherry picked from commit 8e04186)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@taleinat taleinat taleinat approved these changes

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner @taleinat @miss-islington @bedevere-bot @the-knights-who-say-ni