-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Fix bug #79451: Using DOMDocument->replaceChild on doctype causes double free #9201
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.
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
Conversation
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 don't know libxml, but the fix seems reasonable and the test works.
Only nits are coding style related.
ext/dom/node.c
Outdated
@@ -1000,7 +1001,7 @@ PHP_METHOD(DOMNode, replaceChild) | |||
zval *id, *newnode, *oldnode; | |||
xmlNodePtr children, newchild, oldchild, nodep; | |||
dom_object *intern, *newchildobj, *oldchildobj; | |||
int foundoldchild = 0, stricterror; | |||
int foundoldchild = 0, stricterror, replacedoctype = 0; |
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.
int foundoldchild = 0, stricterror, replacedoctype = 0; | |
int foundoldchild = 0, stricterror; | |
bool replacedoctype = false; |
PHP 8.0 uses C99 so might as well use bool :)
But even better would to move into the branch where it is only relevant.
The fix for 8.1 and above is different, and I don't know how to do it. So reoppening |
@Girgias I think the only think that needs to be done is switch from the |
https://bugs.php.net/bug.php?id=79451
We have to reset intSubset if replacing doctype with another doctype node.