Skip to content

Commit 24e5e4e

Browse files
nielsdoswazelin
andcommitted
Fix GH-8996: DOMNode serialization on PHP ^8.1
PHP 8.1 introduced a seemingly unintentional BC break in ca94d55 by blocking the (un)serialization of DOM objects. This was done because the serialization never really worked and just resulted in an empty object, which upon unserialization just resulted in an object that you can't use. Users can however implement their own serialization methods, but the commit made that impossible as the ACC flag gets passed down to the child class. An approach was tried in #10307 with a new ACC flag to selectively allow serialization with subclasses if they implement the right methods. However, that was found to be too ad hoc. Instead, let's abuse how the __sleep and __wakeup methods work to throw the exception instead. If the child class implements the __serialize / __unserialize method, then the throwing methods won't be called. Similarly, if the child class implements __sleep and __wakeup, then they're overridden and it doesn't matter that they throw. For the user, this PR has the exact same behaviour for (sub)classes that don't implement the serialization methods: an exception will be thrown. For code that previously implemented subclasses with these methods, this approach will make that code work again. This approach should be both BC preserving and unbreak user's code. Closes GH-12388. For the test: Co-authored-by: wazelin <contact@sergeimikhailov.com>
1 parent fb68387 commit 24e5e4e

File tree

7 files changed

+200
-8
lines changed

7 files changed

+200
-8
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ PHP NEWS
1919

2020
- DOM:
2121
. Restore old namespace reconciliation behaviour. (nielsdos)
22+
. Fixed bug GH-8996 (DOMNode serialization on PHP ^8.1). (nielsdos)
2223

2324
- Fileinfo:
2425
. Fixed bug GH-11891 (fileinfo returns text/xml for some svg files). (usarise)

ext/dom/node.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,4 +1786,25 @@ PHP_METHOD(DOMNode, getLineNo)
17861786
}
17871787
/* }}} */
17881788

1789+
/**
1790+
* We want to block the serialization and unserialization of DOM classes.
1791+
* However, using @not-serializable makes the child classes also not serializable, even if the user implements the methods.
1792+
* So instead, we implement the methods wherein we throw exceptions.
1793+
* The reason we choose these methods is because:
1794+
* - If the user implements __serialize / __unserialize, the respective throwing methods are not called.
1795+
* - If the user implements __sleep / __wakeup, then it's also not a problem because they will not enter the throwing methods.
1796+
*/
1797+
1798+
PHP_METHOD(DOMNode, __sleep)
1799+
{
1800+
zend_throw_exception_ex(NULL, 0, "Serialization of '%s' is not allowed, unless serialization methods are implemented in a subclass", ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
1801+
RETURN_THROWS();
1802+
}
1803+
1804+
PHP_METHOD(DOMNode, __wakeup)
1805+
{
1806+
zend_throw_exception_ex(NULL, 0, "Unserialization of '%s' is not allowed, unless unserialization methods are implemented in a subclass", ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
1807+
RETURN_THROWS();
1808+
}
1809+
17891810
#endif

ext/dom/php_dom.stub.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ public function after(...$nodes): void;
5656
public function replaceWith(...$nodes): void;
5757
}
5858

59-
/** @not-serializable */
6059
class DOMNode
6160
{
6261
/** @readonly */
@@ -104,6 +103,10 @@ class DOMNode
104103

105104
public string $textContent;
106105

106+
public function __sleep(): array {}
107+
108+
public function __wakeup(): void {}
109+
107110
/** @return DOMNode|false */
108111
public function appendChild(DOMNode $node) {}
109112

@@ -156,7 +159,6 @@ public function removeChild(DOMNode $child) {}
156159
public function replaceChild(DOMNode $node, DOMNode $child) {}
157160
}
158161

159-
/** @not-serializable */
160162
class DOMNameSpaceNode
161163
{
162164
/** @readonly */
@@ -182,6 +184,12 @@ class DOMNameSpaceNode
182184

183185
/** @readonly */
184186
public ?DOMNode $parentNode;
187+
188+
/** @implementation-alias DOMNode::__sleep */
189+
public function __sleep(): array {}
190+
191+
/** @implementation-alias DOMNode::__wakeup */
192+
public function __wakeup(): void {}
185193
}
186194

187195
class DOMImplementation

ext/dom/php_dom_arginfo.h

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 20a0ff883af3bbf073d9c8bc8246646ffafe7818 */
2+
* Stub hash: 203760d1cf0e063ffd9abe743a0e24a97985767e */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 0)
55
ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0)
@@ -28,6 +28,11 @@ ZEND_END_ARG_INFO()
2828

2929
#define arginfo_class_DOMChildNode_replaceWith arginfo_class_DOMParentNode_append
3030

31+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DOMNode___sleep, 0, 0, IS_ARRAY, 0)
32+
ZEND_END_ARG_INFO()
33+
34+
#define arginfo_class_DOMNode___wakeup arginfo_class_DOMChildNode_remove
35+
3136
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_DOMNode_appendChild, 0, 0, 1)
3237
ZEND_ARG_OBJ_INFO(0, node, DOMNode, 0)
3338
ZEND_END_ARG_INFO()
@@ -100,6 +105,10 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_DOMNode_replaceChild, 0, 0, 2)
100105
ZEND_ARG_OBJ_INFO(0, child, DOMNode, 0)
101106
ZEND_END_ARG_INFO()
102107

108+
#define arginfo_class_DOMNameSpaceNode___sleep arginfo_class_DOMNode___sleep
109+
110+
#define arginfo_class_DOMNameSpaceNode___wakeup arginfo_class_DOMChildNode_remove
111+
103112
ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_DOMImplementation_getFeature, 0, 2, IS_NEVER, 0)
104113
ZEND_ARG_TYPE_INFO(0, feature, IS_STRING, 0)
105114
ZEND_ARG_TYPE_INFO(0, version, IS_STRING, 0)
@@ -491,6 +500,8 @@ ZEND_END_ARG_INFO()
491500
ZEND_FUNCTION(dom_import_simplexml);
492501
ZEND_METHOD(DOMCdataSection, __construct);
493502
ZEND_METHOD(DOMComment, __construct);
503+
ZEND_METHOD(DOMNode, __sleep);
504+
ZEND_METHOD(DOMNode, __wakeup);
494505
ZEND_METHOD(DOMNode, appendChild);
495506
ZEND_METHOD(DOMNode, C14N);
496507
ZEND_METHOD(DOMNode, C14NFile);
@@ -672,6 +683,8 @@ static const zend_function_entry class_DOMChildNode_methods[] = {
672683

673684

674685
static const zend_function_entry class_DOMNode_methods[] = {
686+
ZEND_ME(DOMNode, __sleep, arginfo_class_DOMNode___sleep, ZEND_ACC_PUBLIC)
687+
ZEND_ME(DOMNode, __wakeup, arginfo_class_DOMNode___wakeup, ZEND_ACC_PUBLIC)
675688
ZEND_ME(DOMNode, appendChild, arginfo_class_DOMNode_appendChild, ZEND_ACC_PUBLIC)
676689
ZEND_ME(DOMNode, C14N, arginfo_class_DOMNode_C14N, ZEND_ACC_PUBLIC)
677690
ZEND_ME(DOMNode, C14NFile, arginfo_class_DOMNode_C14NFile, ZEND_ACC_PUBLIC)
@@ -694,6 +707,8 @@ static const zend_function_entry class_DOMNode_methods[] = {
694707

695708

696709
static const zend_function_entry class_DOMNameSpaceNode_methods[] = {
710+
ZEND_MALIAS(DOMNode, __sleep, __sleep, arginfo_class_DOMNameSpaceNode___sleep, ZEND_ACC_PUBLIC)
711+
ZEND_MALIAS(DOMNode, __wakeup, __wakeup, arginfo_class_DOMNameSpaceNode___wakeup, ZEND_ACC_PUBLIC)
697712
ZEND_FE_END
698713
};
699714

@@ -989,7 +1004,6 @@ static zend_class_entry *register_class_DOMNode(void)
9891004

9901005
INIT_CLASS_ENTRY(ce, "DOMNode", class_DOMNode_methods);
9911006
class_entry = zend_register_internal_class_ex(&ce, NULL);
992-
class_entry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
9931007

9941008
zval property_nodeName_default_value;
9951009
ZVAL_UNDEF(&property_nodeName_default_value);
@@ -1104,7 +1118,6 @@ static zend_class_entry *register_class_DOMNameSpaceNode(void)
11041118

11051119
INIT_CLASS_ENTRY(ce, "DOMNameSpaceNode", class_DOMNameSpaceNode_methods);
11061120
class_entry = zend_register_internal_class_ex(&ce, NULL);
1107-
class_entry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
11081121

11091122
zval property_nodeName_default_value;
11101123
ZVAL_UNDEF(&property_nodeName_default_value);

ext/dom/tests/gh8996.phpt

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
--TEST--
2+
GH-8996: DOMNode serialization on PHP ^8.1
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
echo "=== __sleep and __wakeup ===\n";
9+
10+
class SerializableDomDocumentSleepWakeup extends DOMDocument
11+
{
12+
private $xmlData;
13+
14+
public function __sleep(): array
15+
{
16+
$this->xmlData = $this->saveXML();
17+
return ['xmlData'];
18+
}
19+
20+
public function __wakeup(): void
21+
{
22+
$this->loadXML($this->xmlData);
23+
}
24+
}
25+
26+
$dom = new SerializableDomDocumentSleepWakeup('1.0', 'UTF-8');
27+
$dom->loadXML('<tag>value</tag>');
28+
29+
$serialized = serialize($dom);
30+
var_dump($serialized);
31+
$unserialized = unserialize($serialized);
32+
33+
echo "Serialized:\n-----------\n$serialized\n-----------\nRestored:\n-----------\n{$unserialized->saveXml()}";
34+
35+
echo "=== __serialize and __unserialize ===\n";
36+
37+
class SerializableDomDocument__Serialize__Unserialize extends DOMDocument
38+
{
39+
public function __serialize(): array
40+
{
41+
return ['xmlData' => $this->saveXML()];
42+
}
43+
44+
public function __unserialize(array $data): void
45+
{
46+
$this->loadXML($data['xmlData']);
47+
}
48+
}
49+
50+
$dom = new SerializableDomDocument__Serialize__Unserialize('1.0', 'UTF-8');
51+
$dom->loadXML('<tag>value</tag>');
52+
53+
$serialized = serialize($dom);
54+
$unserialized = unserialize($serialized);
55+
56+
echo "Serialized:\n-----------\n$serialized\n-----------\nRestored:\n-----------\n{$unserialized->saveXml()}";
57+
58+
echo "=== serialize and unserialize ===\n";
59+
60+
class SerializableDomDocumentSerializeUnserialize extends DOMDocument implements Serializable
61+
{
62+
public function serialize(): ?string
63+
{
64+
return $this->saveXML();
65+
}
66+
67+
public function unserialize(string $data): void
68+
{
69+
$this->loadXML($data);
70+
}
71+
}
72+
73+
$dom = new SerializableDomDocumentSerializeUnserialize('1.0', 'UTF-8');
74+
$dom->loadXML('<tag>value</tag>');
75+
76+
$serialized = serialize($dom);
77+
$unserialized = unserialize($serialized);
78+
79+
echo "Serialized:\n-----------\n$serialized\n-----------\nRestored:\n-----------\n{$unserialized->saveXml()}";
80+
81+
?>
82+
--EXPECTF--
83+
=== __sleep and __wakeup ===
84+
string(144) "O:34:"SerializableDomDocumentSleepWakeup":1:{s:43:"%0SerializableDomDocumentSleepWakeup%0xmlData";s:39:"<?xml version="1.0"?>
85+
<tag>value</tag>
86+
";}"
87+
Serialized:
88+
-----------
89+
O:34:"SerializableDomDocumentSleepWakeup":1:{s:43:"%0SerializableDomDocumentSleepWakeup%0xmlData";s:39:"<?xml version="1.0"?>
90+
<tag>value</tag>
91+
";}
92+
-----------
93+
Restored:
94+
-----------
95+
<?xml version="1.0"?>
96+
<tag>value</tag>
97+
=== __serialize and __unserialize ===
98+
Serialized:
99+
-----------
100+
O:47:"SerializableDomDocument__Serialize__Unserialize":1:{s:7:"xmlData";s:39:"<?xml version="1.0"?>
101+
<tag>value</tag>
102+
";}
103+
-----------
104+
Restored:
105+
-----------
106+
<?xml version="1.0"?>
107+
<tag>value</tag>
108+
=== serialize and unserialize ===
109+
110+
Deprecated: SerializableDomDocumentSerializeUnserialize implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s on line %d
111+
Serialized:
112+
-----------
113+
C:43:"SerializableDomDocumentSerializeUnserialize":39:{<?xml version="1.0"?>
114+
<tag>value</tag>
115+
}
116+
-----------
117+
Restored:
118+
-----------
119+
<?xml version="1.0"?>
120+
<tag>value</tag>

ext/dom/tests/not_serializable.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ try {
3636

3737
?>
3838
--EXPECT--
39-
Serialization of 'DOMDocument' is not allowed
40-
Serialization of 'DOMElement' is not allowed
39+
Serialization of 'DOMDocument' is not allowed, unless serialization methods are implemented in a subclass
40+
Serialization of 'DOMElement' is not allowed, unless serialization methods are implemented in a subclass
4141
Serialization of 'DOMXPath' is not allowed
42-
Serialization of 'DOMNameSpaceNode' is not allowed
42+
Serialization of 'DOMNameSpaceNode' is not allowed, unless serialization methods are implemented in a subclass

ext/dom/tests/not_unserializable.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
DOM classes are not unserializable
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$classes = [
9+
"DOMXPath",
10+
"DOMDocument",
11+
"DOMNode",
12+
"DOMNameSpaceNode",
13+
];
14+
15+
foreach ($classes as $class)
16+
{
17+
try {
18+
unserialize('O:' . strlen($class) . ':"' . $class . '":0:{}');
19+
} catch (Exception $e) {
20+
echo $e->getMessage(), "\n";
21+
}
22+
}
23+
24+
?>
25+
--EXPECT--
26+
Unserialization of 'DOMXPath' is not allowed
27+
Unserialization of 'DOMDocument' is not allowed, unless unserialization methods are implemented in a subclass
28+
Unserialization of 'DOMNode' is not allowed, unless unserialization methods are implemented in a subclass
29+
Unserialization of 'DOMNameSpaceNode' is not allowed, unless unserialization methods are implemented in a subclass

0 commit comments

Comments
 (0)