fix(php_set_attribute_id): fix incorrect ID removal, when is_id == 1

[bingo-soft]

This is how we can reproduce the bug:

$dom = new DOMDocument('1.0', 'utf-8');

$element = $dom->createElement('test', 'root');

$dom->appendChild($element);

$element->setAttribute("id", 123);
$element->setIdAttribute("id", true);

$node = $element->getAttributeNode("id");
echo $node->isId(), "\n";

$element->setIdAttribute("id", true);
echo $node->isId(), "\n";

And we get printed just

1

So, the second call to setIdAttribute, even with is_id set to true, revokes the identity from the attribute. The problem is with else branch of the source code:

	if (is_id == 1 && attrp->atype != XML_ATTRIBUTE_ID) {
		xmlChar *id_val;

		id_val = xmlNodeListGetString(attrp->doc, attrp->children, 1);
		if (id_val != NULL) {
			xmlAddID(NULL, attrp->doc, id_val, attrp);
			xmlFree(id_val);
		}
	} else {
		if (attrp->atype == XML_ATTRIBUTE_ID) {
			xmlRemoveID(attrp->doc, attrp);
			attrp->atype = 0;
		}
	}

It removes identity, but does not check if is_id is 0.

[Girgias]

Please open up a bug report at https://bugs.php.net and add a regression test as a file.

[bingo-soft]

Please open up a bug report at https://bugs.php.net and add a regression test as a file.

I made a bug report and I will prepare a regression test. What I'm not sure about, is whether this fix for PHP-7.4 will be enough. In fact this bug is reproduced in versions 5.x-8.x

[nikic]

This --CLEAN-- section is unnecessary.

[cmb69]

This fix looks good, thank you!

Note that versions prior to PHP-7.4 are no longer actively supported, so targeting PHP-7.4 is correct.

[cmb69]

Thank you! I'll merge this soon.