Fix $x .= $x

Also add a nice comment pointing out that concat runs on magic.
php · Sep 6, 2014 · 29397f8 · 29397f8
1 parent 17cb244
commit 29397f8
Showing 1 changed file with 10 additions and 5 deletions.
diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c
@@ -1531,24 +1531,29 @@ ZEND_API int concat_function(zval *result, zval *op1, zval *op2 TSRMLS_DC) /* {{
 
 	{
 		size_t op1_len = Z_STRLEN_P(op1);
-		size_t result_len = op1_len + Z_STRLEN_P(op2);
+		size_t op2_len = Z_STRLEN_P(op2);
+		size_t result_len = op1_len + op2_len;
 		zend_string *result_str;
 
-		if (op1_len > SIZE_MAX - Z_STRLEN_P(op2)) {
+		if (op1_len > SIZE_MAX - op2_len) {
 			zend_error_noreturn(E_ERROR, "String size overflow");
 		}
 
-		if (result == op1 && !IS_INTERNED(Z_STR_P(op1))) {
+		if (result == op1 && !IS_INTERNED(Z_STR_P(result))) {
 			/* special case, perform operations on result */
 			result_str = zend_string_realloc(Z_STR_P(result), result_len, 0);
 		} else {
 			result_str = zend_string_alloc(result_len, 0);
 			memcpy(result_str->val, Z_STRVAL_P(op1), op1_len);
 		}
 
-		memcpy(result_str->val + op1_len, Z_STRVAL_P(op2), Z_STRLEN_P(op2));
-		result_str->val[result_len] = '\0';
+		/* This has to happen first to account for the cases where result == op1 == op2 and
+		 * the realloc is done. In this case this line will also update Z_STRVAL_P(op2) to
+		 * point to the new string. The first op2_len bytes of result will still be the same. */
 		ZVAL_NEW_STR(result, result_str);
+
+		memcpy(result_str->val + op1_len, Z_STRVAL_P(op2), op2_len);
+		result_str->val[result_len] = '\0';
 	}
 
 	if (UNEXPECTED(use_copy1)) {