[prev in list] [next in list] [prev in thread] [next in thread] 

List:       pecl-cvs
Subject:    [PECL-CVS] com =?UTF-8?Q?pecl/database/mysql=5Fxdevapi=3A=20orabug=20=23=33=30=32=32?= =?UTF-8?Q?=36
From:       hery ramilison <mysqlre () php ! net>
Date:       2019-08-26 17:31:08
Message-ID: php-mail-984e041fe177b6a6338955279439f390423642128 () git ! php ! net
[Download RAW message or body]

Commit:    9110609c57833ab43cb40325aa3c17720102b048
Author:    Darek Slusarczyk <dariusz.slusarczyk@oracle.com>         Mon, 26 Aug 2019 \
                19:30:25 +0200
Parents:   cba371016cff36efbe32a0705d4f5bcd0fb2fa15
Branches:  release/8.0.18

Link:       http://git.php.net/?p=pecl/database/mysql_xdevapi.git;a=commitdiff;h=9110609c57833ab43cb40325aa3c17720102b048


Log:
orabug #30226232: incorrect behaviour of chained CRUD ops in webserver mode
- CRUD Collection.Add: fix wrong management of raw zvals which was the main reason of \
                incorrect behaviour (in Collection_add there was kept zval* \
                object_zv;)
- fix related tests/add_with_empty_arg_is_noop.phpt
- add json_api.h header to safely include <ext/json/...> headers
- improvements in util::zvalue

Bugs:
https://bugs.php.net/30226232

Changed paths:
  A  json_api.h
  M  mysqlx_collection.cc
  M  mysqlx_collection__add.cc
  M  mysqlx_collection__add.h
  M  mysqlx_table__insert.cc
  M  php_api.h
  M  tests/add_with_empty_arg_is_noop.phpt
  M  util/json_utils.cc
  M  util/json_utils.h
  M  util/value.cc
  M  util/value.h
  M  util/value.inl


["diff_9110609c57833ab43cb40325aa3c17720102b048.txt" (text/plain)]

diff --git a/json_api.h b/json_api.h
new file mode 100644
index 0000000..ea1e47d
--- /dev/null
+++ b/json_api.h
@@ -0,0 +1,27 @@
+/*
+  +----------------------------------------------------------------------+
+  | PHP Version 7                                                        |
+  +----------------------------------------------------------------------+
+  | Copyright (c) 2006-2019 The PHP Group                                |
+  +----------------------------------------------------------------------+
+  | This source file is subject to version 3.01 of the PHP license,      |
+  | that is bundled with this package in the file LICENSE, and is        |
+  | available through the world-wide-web at the following url:           |
+  | http://www.php.net/license/3_01.txt                                  |
+  | If you did not receive a copy of the PHP license and are unable to   |
+  | obtain it through the world-wide-web, please send a note to          |
+  | license@php.net so we can mail you a copy immediately.               |
+  +----------------------------------------------------------------------+
+  | Authors: Darek Slusarczyk <marines@php.net>                          |
+  +----------------------------------------------------------------------+
+*/
+#ifndef MYSQL_XDEVAPI_JSON_API_H
+#define MYSQL_XDEVAPI_JSON_API_H
+
+extern "C" {
+#include <ext/json/php_json.h>
+#include <ext/json/php_json_parser.h>
+#include <zend_smart_str.h>
+}
+
+#endif // MYSQL_XDEVAPI_JSON_API_H
diff --git a/mysqlx_collection.cc b/mysqlx_collection.cc
index c006bca..c0911b3 100644
--- a/mysqlx_collection.cc
+++ b/mysqlx_collection.cc
@@ -562,7 +562,7 @@ MYSQL_XDEVAPI_PHP_METHOD(mysqlx_collection, addOrReplaceOne)
 	Collection_add coll_add;
 	zval doc_with_id;
 	util::json::ensure_doc_id(doc, id, &doc_with_id);
-	if (!coll_add.add_docs(object_zv, data_object.collection, id, &doc_with_id)) {
+	if (!coll_add.add_docs(data_object.collection, id, &doc_with_id)) {
 		DBG_VOID_RETURN;
 	}
 
diff --git a/mysqlx_collection__add.cc b/mysqlx_collection__add.cc
index 4161c7a..5a9a6ee 100644
--- a/mysqlx_collection__add.cc
+++ b/mysqlx_collection__add.cc
@@ -17,10 +17,7 @@
 */
 #include "php_api.h"
 #include "mysqlnd_api.h"
-extern "C" {
-#include <ext/json/php_json.h>
-#include <ext/json/php_json_parser.h>
-}
+#include "json_api.h"
 #include "xmysqlnd/xmysqlnd.h"
 #include "xmysqlnd/xmysqlnd_session.h"
 #include "xmysqlnd/xmysqlnd_schema.h"
@@ -61,40 +58,20 @@ ZEND_END_ARG_INFO()
 
 //------------------------------------------------------------------------------
 
-
 /* {{{ execute_statement */
 enum_func_status
-execute_statement(xmysqlnd_stmt* stmt,zval* return_value)
+execute_statement(xmysqlnd_stmt& stmt, zval* resultset)
 {
-	enum_func_status ret{FAIL};
-	if (stmt) {
-		zval stmt_zv;
-		ZVAL_UNDEF(&stmt_zv);
-		mysqlx_new_stmt(&stmt_zv, stmt);
-		if (Z_TYPE(stmt_zv) == IS_NULL) {
-			xmysqlnd_stmt_free(stmt, nullptr, nullptr);
-		}
-		if (Z_TYPE(stmt_zv) == IS_OBJECT) {
-			zval zv;
-			ZVAL_UNDEF(&zv);
-			zend_long flags{0};
-			mysqlx_statement_execute_read_response(Z_MYSQLX_P(&stmt_zv),
-								flags, MYSQLX_RESULT, &zv);
-			ZVAL_COPY(return_value, &zv);
-			zval_dtor(&zv);
-			ret = PASS;
-		}
-		zval_ptr_dtor(&stmt_zv);
-	}
-	return ret;
+	util::zvalue stmt_zv;
+	mysqlx_new_stmt(stmt_zv.ptr(), &stmt);
+	if (!stmt_zv.is_object()) return FAIL;
+	zend_long flags{0};
+	mysqlx_statement_execute_read_response(Z_MYSQLX_P(stmt_zv.ptr()),
+						flags, MYSQLX_RESULT, resultset);
+	return PASS;
 }
 /* }}} */
 
-
-#define ID_COLUMN_NAME		"_id"
-#define ID_TEMPLATE_PREFIX	"\"" ID_COLUMN_NAME "\":\""
-#define ID_TEMPLATE_SUFFIX	"\"}"
-
 enum class Add_op_status
 {
 	success,
@@ -112,10 +89,9 @@ struct doc_add_op_return_status
 Add_op_status
 collection_add_string(
 	st_xmysqlnd_crud_collection_op__add* add_op,
-	zval* doc,
-	zval* /*return_value*/)
+	util::zvalue& doc)
 {
-	if( PASS == xmysqlnd_crud_collection_add__add_doc(add_op,doc) ) {
+	if( PASS == xmysqlnd_crud_collection_add__add_doc(add_op, doc.ptr()) ) {
 		return Add_op_status::success;
 	}
 	return Add_op_status::fail;
@@ -125,49 +101,32 @@ collection_add_string(
 
 /* {{{ collection_add_object*/
 Add_op_status
-collection_add_object_impl(
+collection_add_object(
 	st_xmysqlnd_crud_collection_op__add* add_op,
-	zval* doc,
-	zval* /*return_value*/)
+	util::zvalue& doc)
 {
-	zval new_doc;
+	util::zvalue new_doc;
 	Add_op_status ret = Add_op_status::fail;
-	util::json::to_zv_string(doc, &new_doc);
-	if( PASS == xmysqlnd_crud_collection_add__add_doc(add_op, &new_doc) ) {
+	util::json::to_zv_string(doc, new_doc);
+	if( PASS == xmysqlnd_crud_collection_add__add_doc(add_op, new_doc.ptr()) ) {
 		ret = Add_op_status::success;
 	}
-	zval_dtor(&new_doc);
 	return ret;
 }
 /* }}} */
 
 
-/* {{{ collection_add_object*/
-Add_op_status
-collection_add_object(
-	st_xmysqlnd_crud_collection_op__add* add_op,
-	zval* doc,
-	zval* return_value)
-{
-	return collection_add_object_impl(
-				add_op, doc, return_value);
-}
-/* }}} */
-
-
 /* {{{ collection_add_array*/
 Add_op_status
 collection_add_array(
 	st_xmysqlnd_crud_collection_op__add* add_op,
-	zval* doc,
-	zval* return_value)
+	util::zvalue& doc)
 {
 	Add_op_status ret = Add_op_status::fail;
-	if( zend_hash_num_elements(Z_ARRVAL_P(doc)) == 0 ) {
+	if( doc.empty() ) {
 		ret = Add_op_status::noop;
 	} else {
-		ret = collection_add_object_impl(
-			add_op, doc, return_value);
+		ret = collection_add_object(add_op, doc);
 	}
 	return ret;
 }
@@ -180,12 +139,11 @@ collection_add_array(
 
 /* {{{ Collection_add::init() */
 bool Collection_add::add_docs(
-	zval* obj_zv,
 	xmysqlnd_collection* coll,
 	zval* documents,
 	int num_of_documents)
 {
-	if (!obj_zv || !documents || !num_of_documents) return false;
+	if (!documents || !num_of_documents) return false;
 
 	for (int i{0}; i < num_of_documents; ++i) {
 		if (Z_TYPE(documents[i]) != IS_STRING &&
@@ -198,20 +156,15 @@ bool Collection_add::add_docs(
 
 	if( !add_op ) {
 		if( !coll ) return false;
-		object_zv = obj_zv;
 		collection = coll->get_reference();
 		add_op = xmysqlnd_crud_collection_add__create(
 			mnd_str2c(collection->get_schema()->get_name()),
 			mnd_str2c(collection->get_name()));
 		if (!add_op) return false;
-	} else {
-		ZVAL_COPY(obj_zv, object_zv);
 	}
 
-	zval doc;
 	for (int i{0}; i < num_of_documents; ++i) {
-		ZVAL_DUP(&doc, &documents[i]);
-		docs.push_back( doc );
+		docs.push_back(util::zvalue::clone_from(&documents[i]));
 	}
 
 	return true;
@@ -221,13 +174,12 @@ bool Collection_add::add_docs(
 
 /* {{{ Collection_add::init() */
 bool Collection_add::add_docs(
-	zval* obj_zv,
 	xmysqlnd_collection* coll,
 	const util::string_view& /*doc_id*/,
 	zval* doc)
 {
 	const int num_of_documents = 1;
-	if (!add_docs(obj_zv, coll, doc, num_of_documents)) return false;
+	if (!add_docs(coll, doc, num_of_documents)) return false;
 	return xmysqlnd_crud_collection_add__set_upsert(add_op) == PASS;
 }
 /* }}} */
@@ -236,11 +188,6 @@ bool Collection_add::add_docs(
 /* {{{ Collection_add::~Collection_add() */
 Collection_add::~Collection_add()
 {
-	for (size_t i{0}; i < docs.size(); ++i) {
-		zval_ptr_dtor(&docs[i]);
-		ZVAL_UNDEF(&docs[i]);
-	}
-
 	if (add_op) {
 		xmysqlnd_crud_collection_add__destroy(add_op);
 	}
@@ -253,30 +200,24 @@ Collection_add::~Collection_add()
 
 
 /* {{{ Collection_add::execute() */
-void Collection_add::execute(zval* return_value)
+void Collection_add::execute(zval* resultset)
 {
-	enum_func_status execute_ret_status{PASS};
-	size_t noop_cnt{0};
-
 	DBG_ENTER("Collection_add::execute");
 
-	RETVAL_FALSE;
-
+	size_t noop_cnt{0};
 	Add_op_status ret = Add_op_status::success;
-	for (size_t i{0}; i < docs.size() && ret != Add_op_status::fail ; ++i) {
+	for (auto it{docs.begin()}; it != docs.end() && ret != Add_op_status::fail ; ++it) \
{  ret = Add_op_status::fail;
-		switch(Z_TYPE(docs[i])) {
-		case IS_STRING:
-			ret = collection_add_string(
-				add_op, &docs[i], return_value);
+		auto& doc{ *it };
+		switch(doc.type()) {
+		case util::zvalue::Type::String:
+			ret = collection_add_string(add_op, doc);
 			break;
-		case IS_ARRAY:
-			ret = collection_add_array(
-				add_op, &docs[i], return_value);
+		case util::zvalue::Type::Array:
+			ret = collection_add_array(add_op, doc);
 			break;
-		case IS_OBJECT:
-			ret = collection_add_object(
-				add_op, &docs[i], return_value);
+		case util::zvalue::Type::Object:
+			ret = collection_add_object(add_op, doc);
 			break;
 		}
 		if( ret == Add_op_status::noop ) {
@@ -284,10 +225,11 @@ void Collection_add::execute(zval* return_value)
 		}
 	}
 
-	if ( execute_ret_status != FAIL && docs.size() > noop_cnt ) {
+	enum_func_status execute_ret_status{PASS};
+	if ( docs.size() > noop_cnt ) {
 		xmysqlnd_stmt* stmt = collection->add(add_op);
 		if( nullptr != stmt ) {
-			execute_ret_status =  execute_statement(stmt,return_value);
+			execute_ret_status = execute_statement(*stmt, resultset);
 		} else {
 			execute_ret_status = FAIL;
 		}
@@ -314,10 +256,9 @@ MYSQL_XDEVAPI_PHP_METHOD(mysqlx_collection__add, __construct)
 /* {{{ proto mixed mysqlx_collection__add::execute() */
 MYSQL_XDEVAPI_PHP_METHOD(mysqlx_collection__add, execute)
 {
-	zval* object_zv{nullptr};
-
 	DBG_ENTER("mysqlx_collection__add::execute");
 
+	zval* object_zv{nullptr};
 	if (FAILURE == util::zend::parse_method_parameters(execute_data, getThis(), "O",
 												&object_zv,
 												collection_add_class_entry))
@@ -355,7 +296,9 @@ MYSQL_XDEVAPI_PHP_METHOD(mysqlx_collection__add, add)
 	 * For subsequent calls to add_docs, the xmysqlnd_collection is set to NULL
 	 */
 	Collection_add& coll_add = util::fetch_data_object<Collection_add>(object_zv);
-	coll_add.add_docs(return_value, nullptr, docs, num_of_docs);
+	if (coll_add.add_docs(nullptr, docs, num_of_docs)) {
+		util::zvalue::copy_to(object_zv, return_value);
+	}
 
 	DBG_VOID_RETURN;
 }
@@ -477,7 +420,7 @@ mysqlx_new_collection__add(
 	if (SUCCESS == object_init_ex(return_value, collection_add_class_entry) && \
IS_OBJECT == Z_TYPE_P(return_value)) {  const st_mysqlx_object* const mysqlx_object = \
Z_MYSQLX_P(return_value);  Collection_add* const coll_add = \
                static_cast<Collection_add*>(mysqlx_object->ptr);
-		if (!coll_add || !coll_add->add_docs(return_value, collection, docs, num_of_docs)) \
{ +		if (!coll_add || !coll_add->add_docs(collection, docs, num_of_docs)) {
 			php_error_docref(nullptr, E_WARNING, "invalid object of class %s", \
ZSTR_VAL(mysqlx_object->zo.ce->name));  zval_ptr_dtor(return_value);
 			ZVAL_NULL(return_value);
diff --git a/mysqlx_collection__add.h b/mysqlx_collection__add.h
index eb9d0f3..86be291 100644
--- a/mysqlx_collection__add.h
+++ b/mysqlx_collection__add.h
@@ -18,6 +18,8 @@
 #ifndef MYSQLX_COLLECTION__ADD_H
 #define MYSQLX_COLLECTION__ADD_H
 
+#include "util/value.h"
+
 namespace mysqlx {
 
 namespace drv {
@@ -39,24 +41,21 @@ public:
 	~Collection_add();
 
 	bool add_docs(
-		zval* object_zv,
 		drv::xmysqlnd_collection* collection,
 		zval* docs,
 		int num_of_docs);
 	bool add_docs(
-		zval* object_zv,
 		drv::xmysqlnd_collection* collection,
 		const util::string_view& single_doc_id,
 		zval* doc);
 
 public:
-	void execute(zval* return_value);
+	void execute(zval* resultset);
 
 private:
-	zval* object_zv{nullptr};
 	drv::xmysqlnd_collection* collection{nullptr};
 	drv::st_xmysqlnd_crud_collection_op__add* add_op{nullptr};
-	std::vector<zval> docs;
+	std::vector<util::zvalue> docs;
 };
 /* }}} */
 
diff --git a/mysqlx_table__insert.cc b/mysqlx_table__insert.cc
index 632fcf0..dc99dcf 100644
--- a/mysqlx_table__insert.cc
+++ b/mysqlx_table__insert.cc
@@ -17,10 +17,7 @@
 */
 #include "php_api.h"
 #include "mysqlnd_api.h"
-extern "C" {
-#include <ext/json/php_json.h>
-#include <zend_smart_str.h>
-}
+#include "json_api.h"
 #include "xmysqlnd/xmysqlnd.h"
 #include "xmysqlnd/xmysqlnd_session.h"
 #include "xmysqlnd/xmysqlnd_schema.h"
diff --git a/php_api.h b/php_api.h
index 716a9c8..1d5590c 100644
--- a/php_api.h
+++ b/php_api.h
@@ -62,7 +62,7 @@ extern "C" {
 	ZEND_HASH_FOREACH_STR_KEY_VAL(ht, _key, _val) \
 	MYSQLX_RESTORE_WARNINGS()
 
-#else
+#else // UNIX OSes
 
 #define MYSQLX_HASH_FOREACH_VAL ZEND_HASH_FOREACH_VAL
 #define MYSQLX_HASH_FOREACH_PTR	ZEND_HASH_FOREACH_PTR
@@ -70,5 +70,4 @@ extern "C" {
 
 #endif
 
-
 #endif // MYSQL_XDEVAPI_PHP_API_H
diff --git a/tests/add_with_empty_arg_is_noop.phpt \
b/tests/add_with_empty_arg_is_noop.phpt index ed95fc0..5c04761 100644
--- a/tests/add_with_empty_arg_is_noop.phpt
+++ b/tests/add_with_empty_arg_is_noop.phpt
@@ -16,7 +16,7 @@ error_reporting=0
 
 		$result = $coll->add([])->execute();//Single noop
 
-		expect_false($result);
+		expect_null($result);
 
 		$res = $coll->find()->execute()->fetchAll();
 		expect_empty_array($res);
@@ -30,13 +30,13 @@ error_reporting=0
 		expect_eq($result->getAffectedItemsCount(), 2);
 
 		$result = $coll->add([])->execute();//Again noop
-		expect_false($result);
+		expect_null($result);
 
 		$res = $coll->find()->execute()->fetchAll();
 		expect_eq(count($res),2);
 
 		$result = $coll->add([],[],[])->execute(); //Perverted case...
-		expect_false($result);
+		expect_null($result);
 		$res = $coll->find()->execute()->fetchAll();
 		expect_eq(count($res),2);
 
diff --git a/util/json_utils.cc b/util/json_utils.cc
index 051a8e1..647527a 100644
--- a/util/json_utils.cc
+++ b/util/json_utils.cc
@@ -59,6 +59,11 @@ void to_zv_string(zval* src, zval* dest)
 }
 /* }}} */
 
+void to_zv_string(util::zvalue& src, util::zvalue& dest)
+{
+	to_zv_string(src.ptr(), dest.ptr());
+}
+
 namespace {
 
 class Ensure_doc_id
diff --git a/util/json_utils.h b/util/json_utils.h
index 9f85722..7a1b049 100644
--- a/util/json_utils.h
+++ b/util/json_utils.h
@@ -27,9 +27,12 @@ namespace mysqlx {
 
 namespace util {
 
+class zvalue;
+
 namespace json {
 
 void to_zv_string(zval* src, zval* dest);
+void to_zv_string(util::zvalue& src, util::zvalue& dest);
 
 void ensure_doc_id(
 	zval* raw_doc,
diff --git a/util/value.cc b/util/value.cc
index 159ec95..f764cb2 100644
--- a/util/value.cc
+++ b/util/value.cc
@@ -512,27 +512,47 @@ zvalue zvalue::clone() const
 	return result;
 }
 
-void zvalue::acquire(zval* other)
+zvalue zvalue::clone_from(zval* src)
 {
-	assert(other);
+	zvalue result;
+	ZVAL_DUP(&result.zv, src);
+	return result;
+}
+
+void zvalue::acquire(zval* src)
+{
+	assert(src);
 	zval_ptr_dtor(&zv);
-	ZVAL_ZVAL(&zv, other, 1, 1);
-	ZVAL_UNDEF(other);
+	ZVAL_ZVAL(&zv, src, 1, 1);
+	ZVAL_UNDEF(src);
 }
 
-void zvalue::copy_to(zval* other)
+void zvalue::copy_to(zval* dst)
 {
-	assert(other);
-	ZVAL_ZVAL(other, &zv, 1, 0);
+	assert(dst);
+	ZVAL_ZVAL(dst, &zv, 1, 0);
 }
 
-void zvalue::move_to(zval* other)
+void zvalue::copy_to(zval* src, zval* dst)
 {
-	assert(other);
-	ZVAL_ZVAL(other, &zv, 1, 1);
+	assert(src && dst);
+	ZVAL_ZVAL(dst, src, 1, 0);
+}
+
+void zvalue::move_to(zval* dst)
+{
+	assert(dst);
+	ZVAL_ZVAL(dst, &zv, 1, 1);
 	ZVAL_UNDEF(&zv);
 }
 
+void zvalue::move_to(zval* src, zval* dst)
+{
+	assert(src && dst);
+	ZVAL_ZVAL(dst, src, 1, 1);
+	ZVAL_UNDEF(src);
+}
+
 zval zvalue::release()
 {
 	zval other;
diff --git a/util/value.h b/util/value.h
index 48e6cf7..7167406 100644
--- a/util/value.h
+++ b/util/value.h
@@ -187,6 +187,8 @@ class zvalue
 		// new fully separated item, not just next reference incremented
 		zvalue clone() const;
 
+		static zvalue clone_from(zval* src);
+
 		// take over ownership of passed zval
 		void acquire(zval* zv);
 		void acquire(zval& zv);
@@ -195,10 +197,14 @@ class zvalue
 		void copy_to(zval* zv);
 		void copy_to(zval& zv);
 
+		static void copy_to(zval* src, zval* dst);
+
 		// moves value to zv, and sets type to undefined
 		void move_to(zval* zv);
 		void move_to(zval& zv);
 
+		static void move_to(zval* src, zval* dst);
+
 		// increment / decrement reference counter if type is refcounted, else does \
nothing  void inc_ref() const;
 		void dec_ref() const;
diff --git a/util/value.inl b/util/value.inl
index 5d8666a..0f0f980 100644
--- a/util/value.inl
+++ b/util/value.inl
@@ -260,19 +260,19 @@ inline bool zvalue::empty() const
 	return size() == 0;
 }
 
-inline void zvalue::acquire(zval& zv)
+inline void zvalue::acquire(zval& src)
 {
-	acquire(&zv);
+	acquire(&src);
 }
 
-inline void zvalue::copy_to(zval& zv)
+inline void zvalue::copy_to(zval& dst)
 {
-	copy_to(&zv);
+	copy_to(&dst);
 }
 
-inline void zvalue::move_to(zval& zv)
+inline void zvalue::move_to(zval& dst)
 {
-	move_to(&zv);
+	move_to(&dst);
 }
 
 inline void zvalue::inc_ref() const



-- 
PECL CVS Mailing List 
To unsubscribe, visit: http://www.php.net/unsub.php

[prev in list] [next in list] [prev in thread] [next in thread]