Fixed wrong fix for bug #41582

4 files changed, 100 insertions, 48 deletions

diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index 4c0fdf11a2..9000ac5892 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -249,18 +249,16 @@ static zval * sxe_prop_dim_read(zval *object, zval *member, zend_bool elements,
 	int             nodendx = 0;
 	int             test = 0;
 
-	if (!member) {
-		return_value = &EG(uninitialized_zval);
-		return_value->is_ref = 1;
-		return return_value;
-	}
-
 	sxe = php_sxe_fetch_object(object TSRMLS_CC);
 
-	if (Z_TYPE_P(member) == IS_LONG) {
+	if (!member || Z_TYPE_P(member) == IS_LONG) {
 		if (sxe->iter.type != SXE_ITER_ATTRLIST) {
 			attribs = 0;
 			elements = 1;
+		} else if (!member) {
+			/* This happens when the user did: $sxe[]->foo = $value */
+			php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create unnamed attribute");
+			return NULL;
 		}
 		name = NULL;
 	} else {
@@ -288,6 +286,12 @@ static zval * sxe_prop_dim_read(zval *object, zval *member, zend_bool elements,
 		node = php_sxe_get_first_node(sxe, node TSRMLS_CC);
 		attr = node ? node->properties : NULL;
 		test = 0;
+		if (!member && node && node->parent &&
+		    node->parent->type == XML_DOCUMENT_NODE) {
+			/* This happens when the user did: $sxe[]->foo = $value */
+			php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create unnamed attribute");
+			return NULL;
+		}
 	}
 
 	if (node) {
@@ -320,13 +324,30 @@ static zval * sxe_prop_dim_read(zval *object, zval *member, zend_bool elements,
 			if (!sxe->node) {
 				php_libxml_increment_node_ptr((php_libxml_node_object *)sxe, node, NULL TSRMLS_CC);
 			}
-			if (Z_TYPE_P(member) == IS_LONG) {
+			if (!member || Z_TYPE_P(member) == IS_LONG) {
+				long cnt = 0;
+				xmlNodePtr mynode = node;
+
 				if (sxe->iter.type == SXE_ITER_CHILD) {
 					node = php_sxe_get_first_node(sxe, node TSRMLS_CC);
 				}
-				node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL);
+				if (sxe->iter.type == SXE_ITER_NONE) {
+					if (member && Z_LVAL_P(member) > 0) {
+						php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot add element %s number %ld when only 0 such elements exist", mynode->name, Z_LVAL_P(member));
+					}
+				} else if (member) {
+					node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
+				} else {					
+					node = NULL;
+				}
 				if (node) {
 					_node_as_zval(sxe, node, return_value, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix TSRMLS_CC);
+				} else if (type == BP_VAR_W || type == BP_VAR_RW) {
+					if (member && cnt < Z_LVAL_P(member)) {
+						php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot add element %s number %ld when only %ld such elements exist", mynode->name, Z_LVAL_P(member), cnt);
+					}
+					node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL);
+					_node_as_zval(sxe, node, return_value, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix TSRMLS_CC);
 				}
 			} else {
 #if SXE_ELEMENT_BY_NAME
@@ -431,7 +452,6 @@ static void change_node_zval(xmlNodePtr node, zval *value TSRMLS_DC)
 static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_bool elements, zend_bool attribs, xmlNodePtr *pnewnode TSRMLS_DC)
 {
 	php_sxe_object *sxe;
-	char           *name;
 	xmlNodePtr      node;
 	xmlNodePtr      newnode = NULL;
 	xmlNodePtr      mynode;
@@ -445,22 +465,19 @@ static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_boo
 	long            cnt = 0;
 	zval            tmp_zv, trim_zv, value_copy;
 
-	if (!member) {
-		/* This happens when the user did: $sxe[] = $value
-		 * and could also be E_PARSE, but we use this only during parsing
-		 * and this is during runtime.
-		 */
-		php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create unnamed attribute");
-		return;
-	}
-
-
 	sxe = php_sxe_fetch_object(object TSRMLS_CC);
 
-	if (Z_TYPE_P(member) == IS_LONG) {
+	if (!member || Z_TYPE_P(member) == IS_LONG) {
 		if (sxe->iter.type != SXE_ITER_ATTRLIST) {
 			attribs = 0;
 			elements = 1;
+		} else if (!member) {
+			/* This happens when the user did: $sxe[] = $value
+			 * and could also be E_PARSE, but we use this only during parsing
+			 * and this is during runtime.
+			 */
+			php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create unnamed attribute");
+			return;
 		}
 	} else {
 		if (Z_TYPE_P(member) != IS_STRING) {
@@ -481,8 +498,6 @@ static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_boo
 		}
 	}
 
-	name = Z_STRVAL_P(member);
-
 	GET_NODE(sxe, node);
 
 	if (sxe->iter.type == SXE_ITER_ATTRLIST) {
@@ -496,6 +511,15 @@ static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_boo
 		node = php_sxe_get_first_node(sxe, node TSRMLS_CC);
 		attr = node ? node->properties : NULL;
 		test = 0;
+		if (!member && node && node->parent &&
+		    node->parent->type == XML_DOCUMENT_NODE) {
+			/* This happens when the user did: $sxe[] = $value
+			 * and could also be E_PARSE, but we use this only during parsing
+			 * and this is during runtime.
+			 */
+			php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create unnamed attribute");
+			return;
+		}
 		if (attribs && !node && sxe->iter.type == SXE_ITER_ELEMENT) {
 			node = xmlNewChild(mynode, mynode->ns, sxe->iter.name, NULL);
 			attr = node->properties;
@@ -552,7 +576,7 @@ static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_boo
 				}
 			} else {
 				while (attr) {
-					if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)name) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
+					if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
 						is_attr = 1;
 						++counter;
 						break;
@@ -564,17 +588,30 @@ static void sxe_prop_dim_write(zval *object, zval *member, zval *value, zend_boo
 		}
 
 		if (elements) {
-			if (Z_TYPE_P(member) == IS_LONG) {
-				newnode = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
-				if (newnode) {
+			if (!member || Z_TYPE_P(member) == IS_LONG) {
+				if (node->type == XML_ATTRIBUTE_NODE) {
+					php_error_docref(NULL TSRMLS_CC, E_ERROR, "Cannot create duplicate attribute");
+					return;
+				}
+
+				if (sxe->iter.type == SXE_ITER_NONE) {
+					newnode = node;
 					++counter;
+					if (member && Z_LVAL_P(member) > 0) {
+						php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot add element %s number %ld when only 0 such elements exist", mynode->name, Z_LVAL_P(member));
+					}
+				} else if (member) {
+					newnode = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
+					if (newnode) {
+						++counter;
+					}
 				}
 			} else {
 				node = node->children;
 				while (node) {
 					SKIP_TEXT(node);
 
-					if (!xmlStrcmp(node->name, (xmlChar *)name)) {
+					if (!xmlStrcmp(node->name, (xmlChar *)Z_STRVAL_P(member))) {
 						newnode = node;
 						++counter;
 					}
@@ -600,15 +637,23 @@ next_iter:
 			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot assign to an array of nodes (duplicate subnodes or attr detected)");
 		} else if (elements) {
 			if (!node) {
-				newnode = xmlNewTextChild(mynode, mynode->ns, (xmlChar *)name, value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
-			} else if (Z_TYPE_P(member) == IS_LONG) {
-				if (cnt < Z_LVAL_P(member)) {
+				if (!member || Z_TYPE_P(member) == IS_LONG) {
+					newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
+				} else {				
+					newnode = xmlNewTextChild(mynode, mynode->ns, (xmlChar *)Z_STRVAL_P(member), value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
+				}
+			} else if (!member || Z_TYPE_P(member) == IS_LONG) {
+				if (member && cnt < Z_LVAL_P(member)) {
 					php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot add element %s number %ld when only %ld such elements exist", mynode->name, Z_LVAL_P(member), cnt);
 				}
 				newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
 			}
 		} else if (attribs) {
-			newnode = (xmlNodePtr)xmlNewProp(node, (xmlChar *)name, value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
+			if (Z_TYPE_P(member) == IS_LONG) {
+				php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot change attribute number %ld when only %d attributes exist", Z_LVAL_P(member), nodendx);
+			} else {
+				newnode = (xmlNodePtr)xmlNewProp(node, (xmlChar *)Z_STRVAL_P(member), value ? (xmlChar *)Z_STRVAL_P(value) : NULL);
+			}
 		}
 	}
 
diff --git a/ext/simplexml/tests/027.phpt b/ext/simplexml/tests/027.phpt
index 105f990668..a531ccad46 100755
--- a/ext/simplexml/tests/027.phpt
+++ b/ext/simplexml/tests/027.phpt
@@ -40,7 +40,7 @@ $people->person[3] = 'Minni-me';
 $people->person[2]['gender'] = 'male';
 traverse_xml($people);
 $people->person[3]['gender'] = 'error';
-
+traverse_xml($people);
 ?>
 ===DONE===
 --EXPECTF--
@@ -70,6 +70,14 @@ Warning: main(): Cannot add element person number 3 when only 2 such elements ex
   <person gender="male">Minni-me
   </person>
 </people>
-
-Notice: Indirect modification of overloaded element of SimpleXMLElement has no effect in %s027.php on line %d
+<people>
+  <person gender="female">Jane
+  </person>
+  <person gender="male">Joe
+  </person>
+  <person gender="male">Minni-me
+  </person>
+  <person gender="error">
+  </person>
+</people>
 ===DONE===
diff --git a/ext/simplexml/tests/bug35785.phpt b/ext/simplexml/tests/bug35785.phpt
index 4156a7c9f3..0e03f07c55 100755
--- a/ext/simplexml/tests/bug35785.phpt
+++ b/ext/simplexml/tests/bug35785.phpt
@@ -8,13 +8,13 @@ Bug #35785 (SimpleXML memory read error)
 $xml = simplexml_load_string("<root></root>");
 $xml->bla->posts->name = "FooBar";
 echo $xml->asXML();
-
-echo "===FAIL===\n";
-
 $xml = simplexml_load_string("<root></root>");
 $count = count($xml->bla->posts);
 var_dump($count);
-$xml->bla->posts[++$count]->name = "FooBar";
+$xml->bla->posts[$count]->name = "FooBar";
+echo $xml->asXML();
+$xml = simplexml_load_string("<root></root>");
+$xml->bla->posts[]->name = "FooBar";
 echo $xml->asXML();
 ?>
 ===DONE===
@@ -22,12 +22,9 @@ echo $xml->asXML();
 --EXPECTF--
 <?xml version="1.0"?>
 <root><bla><posts><name>FooBar</name></posts></bla></root>
-===FAIL===
 int(0)
-
-Notice: Indirect modification of overloaded element of SimpleXMLElement has no effect in %sbug35785.php on line %d
-
-Strict Standards: Creating default object from empty value in %sbug35785.php on line %d
 <?xml version="1.0"?>
-<root><bla><posts/></bla></root>
+<root><bla><posts><name>FooBar</name></posts></bla></root>
+<?xml version="1.0"?>
+<root><bla><posts><name>FooBar</name></posts></bla></root>
 ===DONE===
diff --git a/ext/simplexml/tests/bug41582.phpt b/ext/simplexml/tests/bug41582.phpt
index 5f0aa78006..b689607d0c 100644
--- a/ext/simplexml/tests/bug41582.phpt
+++ b/ext/simplexml/tests/bug41582.phpt
@@ -8,9 +8,11 @@ $xml = new SimpleXMLElement('<?xml version="1.0" standalone="yes"?>
 
 $xml->movie[]->characters->character[0]->name = 'Miss Coder';
 
-var_dump($xml->asXml());
+echo($xml->asXml());
 
 echo "Done\n";
 ?>
---EXPECTF--	
-Fatal error: Cannot use object of type stdClass as array in %s on line %d
+--EXPECT--
+<?xml version="1.0" standalone="yes"?>
+<collection><movie><characters><character><name>Miss Coder</name></character></characters></movie></collection>
+Done