Emit a warning in case of unallowed characters. Fix another place this

code is used - reuse

5 files changed, 157 insertions, 114 deletions

diff --git a/ext/mysqli/mysqli_api.c b/ext/mysqli/mysqli_api.c
index 3911202066..e3042b18f8 100644
--- a/ext/mysqli/mysqli_api.c
+++ b/ext/mysqli/mysqli_api.c
@@ -67,9 +67,50 @@ static void mysqli_tx_cor_options_to_string(const MYSQL * const conn, smart_str
 /* }}} */
 
 
+/* {{{ mysqlnd_escape_string_for_tx_name_in_comment */
+char *
+mysqli_escape_string_for_tx_name_in_comment(const char * const name TSRMLS_DC)
+{
+	char * ret = NULL;
+	if (name) {
+		zend_bool warned = FALSE;
+		const char * p_orig = name;
+		char * p_copy;
+		p_copy = ret = emalloc(strlen(name) + 1 + 2 + 2 + 1); /* space, open, close, NullS */
+		*p_copy++ = ' ';
+		*p_copy++ = '/';
+		*p_copy++ = '*';
+		while (1) {
+			register char v = *p_orig;
+			if (v == 0) {
+				break;
+			}
+			if ((v >= '0' && v <= '9') ||
+				(v >= 'a' && v <= 'z') ||
+				(v >= 'A' && v <= 'Z') ||
+				v == '-' ||
+				v == '_' ||
+				v == ' ' ||
+				v == '=')
+			{
+				*p_copy++ = v;
+			} else if (warned == FALSE) {
+				php_error_docref(NULL TSRMLS_CC, E_WARNING, "Transaction name truncated. Must be only [0-9A-Za-z\\-_=]+");
+				warned = TRUE;
+			}
+			++p_orig;
+		}
+		*p_copy++ = '*';
+		*p_copy++ = '/';
+		*p_copy++ = 0;
+	}
+	return ret;
+}
+/* }}} */
+
 
 /* {{{ proto bool mysqli_commit_or_rollback_libmysql */
-static int mysqli_commit_or_rollback_libmysql(MYSQL * conn, zend_bool commit, const unsigned int mode, const char * const name)
+static int mysqli_commit_or_rollback_libmysql(MYSQL * conn, zend_bool commit, const unsigned int mode, const char * const name TSRMLS_DC)
 {
 	int ret;
 	smart_str tmp_str = {0, 0, 0};
@@ -78,37 +119,8 @@ static int mysqli_commit_or_rollback_libmysql(MYSQL * conn, zend_bool commit, co
 
 	{
 		char * query;
-		char * name_esc = NULL;
+		char * name_esc = mysqli_escape_string_for_tx_name_in_comment(name TSRMLS_CC);
 		size_t query_len;
-		
-		if (name) {	
-			const char * p_orig = name;
-			char * p_copy;
-			p_copy = name_esc = emalloc(strlen(name) + 1 + 2 + 2 + 1); /* space, open, close, NullS */
-			*p_copy++ = ' ';
-			*p_copy++ = '/';
-			*p_copy++ = '*';
-			while (1) {
-				register char v = *p_orig;
-				if (v == 0) {
-					break;
-				}
-				if ((v >= '0' && v <= '9') ||
-					(v >= 'a' && v <= 'z') ||
-					(v >= 'A' && v <= 'Z') ||
-					v == '-' ||
-					v == '_' ||
-					v == ' ' ||
-					v == '=')
-				{
-					*p_copy++ = v;
-				}
-				++p_orig;
-			}
-			*p_copy++ = '*';
-			*p_copy++ = '/';
-			*p_copy++ = 0;
-		}
 
 		query_len = spprintf(&query, 0, (commit? "COMMIT%s %s":"ROLLBACK%s %s"),
 										  name_esc? name_esc:"", tmp_str.c? tmp_str.c:"");
@@ -748,7 +760,7 @@ PHP_FUNCTION(mysqli_commit)
 	MYSQLI_FETCH_RESOURCE_CONN(mysql, &mysql_link, MYSQLI_STATUS_VALID);
 
 #if !defined(MYSQLI_USE_MYSQLND)
-	if (mysqli_commit_or_rollback_libmysql(mysql->mysql, TRUE, flags, name)) {
+	if (mysqli_commit_or_rollback_libmysql(mysql->mysql, TRUE, flags, name TSRMLS_CC)) {
 #else
 	if (FAIL == mysqlnd_commit(mysql->mysql, flags, name)) {
 #endif
@@ -1998,7 +2010,7 @@ PHP_FUNCTION(mysqli_rollback)
 	MYSQLI_FETCH_RESOURCE_CONN(mysql, &mysql_link, MYSQLI_STATUS_VALID);
 
 #if !defined(MYSQLI_USE_MYSQLND)
-	if (mysqli_commit_or_rollback_libmysql(mysql->mysql, FALSE, flags, name)) {
+	if (mysqli_commit_or_rollback_libmysql(mysql->mysql, FALSE, flags, name TSRMLS_CC)) {
 #else
 	if (FAIL == mysqlnd_rollback(mysql->mysql, flags, name)) {
 #endif
diff --git a/ext/mysqli/mysqli_nonapi.c b/ext/mysqli/mysqli_nonapi.c
index db0352baeb..17570c7a1a 100644
--- a/ext/mysqli/mysqli_nonapi.c
+++ b/ext/mysqli/mysqli_nonapi.c
@@ -1046,12 +1046,14 @@ PHP_FUNCTION(mysqli_get_charset)
 /* }}} */
 #endif
 
-
 #if !defined(MYSQLI_USE_MYSQLND)
+extern char * mysqli_escape_string_for_tx_name_in_comment(const char * const name TSRMLS_DC);
+
 /* {{{ proto bool mysqli_begin_transaction_libmysql */
-static int mysqli_begin_transaction_libmysql(MYSQL * conn, const unsigned int mode, const char * const name)
+static int mysqli_begin_transaction_libmysql(MYSQL * conn, const unsigned int mode, const char * const name TSRMLS_DC)
 {
 	int ret;
+	zend_bool err = FALSE;
 	smart_str tmp_str = {0, 0, 0};
 	if (mode & TRANS_START_WITH_CONSISTENT_SNAPSHOT) {
 		if (tmp_str.len) {
@@ -1059,33 +1061,37 @@ static int mysqli_begin_transaction_libmysql(MYSQL * conn, const unsigned int mo
 		}
 		smart_str_appendl(&tmp_str, "WITH CONSISTENT SNAPSHOT", sizeof("WITH CONSISTENT SNAPSHOT") - 1);
 	}
-	if (mode & TRANS_START_READ_WRITE) {
-		if (tmp_str.len) {
-			smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
-		}
-		smart_str_appendl(&tmp_str, "READ WRITE", sizeof("READ WRITE") - 1);
-	}
-	if (mode & TRANS_START_READ_ONLY) {
-		if (tmp_str.len) {
-			smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+	if (mode & (TRANS_START_READ_WRITE | TRANS_START_READ_ONLY)) {
+		if (mysql_get_server_version(conn) < 50605L) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "This server version doesn't support 'READ WRITE' and 'READ ONLY'. Minimum 5.6.5 is required");
+			err = TRUE;
+		} else if (mode & TRANS_START_READ_WRITE) {
+			if (tmp_str.len) {
+				smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+			}
+			smart_str_appendl(&tmp_str, "READ WRITE", sizeof("READ WRITE") - 1);
+		} else if (mode & TRANS_START_READ_ONLY) {
+			if (tmp_str.len) {
+				smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+			}
+			smart_str_appendl(&tmp_str, "READ ONLY", sizeof("READ ONLY") - 1);
 		}
-		smart_str_appendl(&tmp_str, "READ ONLY", sizeof("READ ONLY") - 1);
 	}
 	smart_str_0(&tmp_str);
 
-	{
-		char * commented_name = NULL;
-		unsigned int commented_name_len = name? spprintf(&commented_name, 0, " /*%s*/", name):0;
+	if (err == FALSE){
+		char * name_esc = mysqli_escape_string_for_tx_name_in_comment(name TSRMLS_CC);
 		char * query;
 		unsigned int query_len = spprintf(&query, 0, "START TRANSACTION%s %s",
-										  commented_name? commented_name:"", tmp_str.c? tmp_str.c:"");
+										  name_esc? name_esc:"", tmp_str.c? tmp_str.c:"");
+
 		smart_str_free(&tmp_str);
+		if (name_esc) {
+			efree(name_esc);
+		}
 
 		ret = mysql_real_query(conn, query, query_len);
 		efree(query);
-		if (commented_name) {
-			efree(commented_name);
-		}
 	}
 	return ret;
 }
@@ -1120,7 +1126,7 @@ PHP_FUNCTION(mysqli_begin_transaction)
 	}
 	
 #if !defined(MYSQLI_USE_MYSQLND)
-	if (mysqli_begin_transaction_libmysql(mysql->mysql, flags, name)) {
+	if (mysqli_begin_transaction_libmysql(mysql->mysql, flags, name TSRMLS_CC)) {
 		RETURN_FALSE;
 	}
 #else
diff --git a/ext/mysqli/tests/mysqli_begin_transaction.phpt b/ext/mysqli/tests/mysqli_begin_transaction.phpt
index 24e8d3c54b..99f87c1852 100644
--- a/ext/mysqli/tests/mysqli_begin_transaction.phpt
+++ b/ext/mysqli/tests/mysqli_begin_transaction.phpt
@@ -134,7 +134,8 @@ NULL
 [017] [1792] %s
 [018] invalid_flag = %d [%d]%A
 
-Warning: mysqli_begin_transaction(): Invalid value for parameter flags (-1) in %s on line %d
+Warning: mysqli_begin_transaction(): Invalid value for parameter flags (-1) in /work/dev/php/php-5.5/ext/mysqli/tests/mysqli_begin_transaction.php on line 98
 [019] [%d]%A
-[020] [%d]%A
+
+Warning: mysqli::begin_transaction(): Transaction name truncated. Must be only [0-9A-Za-z\-_=]+ in /work/dev/php/php-5.5/ext/mysqli/tests/mysqli_begin_transaction.php on line 103
 done!
\ No newline at end of file
diff --git a/ext/mysqli/tests/mysqli_commit_oo.phpt b/ext/mysqli/tests/mysqli_commit_oo.phpt
index da9432facd..77bcf412e7 100644
--- a/ext/mysqli/tests/mysqli_commit_oo.phpt
+++ b/ext/mysqli/tests/mysqli_commit_oo.phpt
@@ -76,22 +76,22 @@ if (!have_innodb($link))
 		printf("[012] [%d] %s\n", $mysqli->errno, $mysqli->error);
 	}
 	if (!$mysqli->commit(0 , "*/ nonsense")) {
-		printf("[012] [%d] %s\n", $mysqli->errno, $mysqli->error);
+		printf("[013] [%d] %s\n", $mysqli->errno, $mysqli->error);
 	}
-	if (!$mysqli->commit(0 , "tx_name")) {
-		printf("[012] [%d] %s\n", $mysqli->errno, $mysqli->error);
+	if (!$mysqli->commit(0 , "tx_name ulf вендел")) {
+		printf("[014] [%d] %s\n", $mysqli->errno, $mysqli->error);
 	}
-	if (!$mysqli->commit(0 , "tx_name")) {
-		printf("[012] [%d] %s\n", $mysqli->errno, $mysqli->error);
+	if (!$mysqli->commit(0 , "tx_name \t\n\r\b")) {
+		printf("[015] [%d] %s\n", $mysqli->errno, $mysqli->error);
 	}
 	if (!$mysqli->commit(MYSQLI_TRANS_COR_AND_CHAIN | MYSQLI_TRANS_COR_NO_RELEASE , "tx_name")) {
-		printf("[012] [%d] %s\n", $mysqli->errno, $mysqli->error);
+		printf("[016] [%d] %s\n", $mysqli->errno, $mysqli->error);
 	}
 
 	$mysqli->close();
 
 	if (NULL !== ($tmp = @$mysqli->commit())) {
-		printf("[013] Expecting NULL, got %s/%s\n", gettype($tmp), $tmp);
+		printf("[017] Expecting NULL, got %s/%s\n", gettype($tmp), $tmp);
 	}
 
 	print "done!";
@@ -101,4 +101,9 @@ if (!have_innodb($link))
 	require_once("clean_table.inc");
 ?>
 --EXPECTF--
+Warning: mysqli::commit(): Transaction name truncated. Must be only [0-9A-Za-z\-_=]+ in %s on line %d
+
+Warning: mysqli::commit(): Transaction name truncated. Must be only [0-9A-Za-z\-_=]+ in %s on line %d
+
+Warning: mysqli::commit(): Transaction name truncated. Must be only [0-9A-Za-z\-_=]+ in %s on line %d
 done!
\ No newline at end of file
diff --git a/ext/mysqlnd/mysqlnd.c b/ext/mysqlnd/mysqlnd.c
index c138c50ed9..1222a7f302 100644
--- a/ext/mysqlnd/mysqlnd.c
+++ b/ext/mysqlnd/mysqlnd.c
@@ -2678,6 +2678,49 @@ MYSQLND_METHOD(mysqlnd_conn_data, tx_cor_options_to_string)(const MYSQLND_CONN_D
 /* }}} */
 
 
+/* {{{ mysqlnd_escape_string_for_tx_name_in_comment */
+static char *
+mysqlnd_escape_string_for_tx_name_in_comment(const char * const name TSRMLS_DC)
+{
+	char * ret = NULL;
+	DBG_ENTER("mysqlnd_escape_string_for_tx_name_in_comment");
+	if (name) {
+		zend_bool warned = FALSE;
+		const char * p_orig = name;
+		char * p_copy;
+		p_copy = ret = mnd_emalloc(strlen(name) + 1 + 2 + 2 + 1); /* space, open, close, NullS */
+		*p_copy++ = ' ';
+		*p_copy++ = '/';
+		*p_copy++ = '*';
+		while (1) {
+			register char v = *p_orig;
+			if (v == 0) {
+				break;
+			}
+			if ((v >= '0' && v <= '9') ||
+				(v >= 'a' && v <= 'z') ||
+				(v >= 'A' && v <= 'Z') ||
+				v == '-' ||
+				v == '_' ||
+				v == ' ' ||
+				v == '=')
+			{
+				*p_copy++ = v;
+			} else if (warned == FALSE) {
+				php_error_docref(NULL TSRMLS_CC, E_WARNING, "Transaction name truncated. Must be only [0-9A-Za-z\\-_=]+");
+				warned = TRUE;
+			}
+			++p_orig;
+		}
+		*p_copy++ = '*';
+		*p_copy++ = '/';
+		*p_copy++ = 0;
+	}
+	DBG_RETURN(ret);
+}
+/* }}} */
+
+
 /* {{{ mysqlnd_conn_data::tx_commit_ex */
 static enum_func_status
 MYSQLND_METHOD(mysqlnd_conn_data, tx_commit_or_rollback)(MYSQLND_CONN_DATA * conn, const zend_bool commit, const unsigned int flags, const char * const name TSRMLS_DC)
@@ -2695,49 +2738,20 @@ MYSQLND_METHOD(mysqlnd_conn_data, tx_commit_or_rollback)(MYSQLND_CONN_DATA * con
 
 			{
 				char * query;
-				char * name_esc = NULL;
 				size_t query_len;
-		
-				if (name) {	
-					const char * p_orig = name;
-					char * p_copy;
-					p_copy = name_esc = mnd_emalloc(strlen(name) + 1 + 2 + 2 + 1); /* space, open, close, NullS */
-					*p_copy++ = ' ';
-					*p_copy++ = '/';
-					*p_copy++ = '*';
-					while (1) {
-						register char v = *p_orig;
-						if (v == 0) {
-							break;
-						}
-						if ((v >= '0' && v <= '9') ||
-							(v >= 'a' && v <= 'z') ||
-							(v >= 'A' && v <= 'Z') ||
-							v == '-' ||
-							v == '_' ||
-							v == ' ' ||
-							v == '=')
-						{
-							*p_copy++ = v;
-						}
-						++p_orig;
-					}
-					*p_copy++ = '*';
-					*p_copy++ = '/';
-					*p_copy++ = 0;
-				}
-
+				char * name_esc = mysqlnd_escape_string_for_tx_name_in_comment(name TSRMLS_CC);
+				
 				query_len = mnd_sprintf(&query, 0, (commit? "COMMIT%s %s":"ROLLBACK%s %s"),
 										name_esc? name_esc:"", tmp_str.c? tmp_str.c:"");
 				smart_str_free(&tmp_str);
-				if (!query) {
-					SET_OOM_ERROR(*conn->error_info);
-					break;
-				}
 				if (name_esc) {
 					mnd_efree(name_esc);
 					name_esc = NULL;
 				}
+				if (!query) {
+					SET_OOM_ERROR(*conn->error_info);
+					break;
+				}
 
 				ret = conn->m->query(conn, query, query_len TSRMLS_CC);
 				mnd_sprintf_free(query);
@@ -2768,36 +2782,41 @@ MYSQLND_METHOD(mysqlnd_conn_data, tx_begin)(MYSQLND_CONN_DATA * conn, const unsi
 				}
 				smart_str_appendl(&tmp_str, "WITH CONSISTENT SNAPSHOT", sizeof("WITH CONSISTENT SNAPSHOT") - 1);
 			}
-			if (mode & TRANS_START_READ_WRITE) {
-				if (tmp_str.len) {
-					smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
-				}
-				smart_str_appendl(&tmp_str, "READ WRITE", sizeof("READ WRITE") - 1);
-			}
-			if (mode & TRANS_START_READ_ONLY) {
-				if (tmp_str.len) {
-					smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+			if (mode & (TRANS_START_READ_WRITE | TRANS_START_READ_ONLY)) {
+				unsigned long server_version = conn->m->get_server_version(conn TSRMLS_CC);
+				if (server_version < 50605L) {
+					php_error_docref(NULL TSRMLS_CC, E_WARNING, "This server version doesn't support 'READ WRITE' and 'READ ONLY'. Minimum 5.6.5 is required");
+					smart_str_free(&tmp_str);
+					break;
+				} else if (mode & TRANS_START_READ_WRITE) {
+					if (tmp_str.len) {
+						smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+					}
+					smart_str_appendl(&tmp_str, "READ WRITE", sizeof("READ WRITE") - 1);
+				} else if (mode & TRANS_START_READ_ONLY) {
+					if (tmp_str.len) {
+						smart_str_appendl(&tmp_str, ", ", sizeof(", ") - 1);
+					}
+					smart_str_appendl(&tmp_str, "READ ONLY", sizeof("READ ONLY") - 1);
 				}
-				smart_str_appendl(&tmp_str, "READ ONLY", sizeof("READ ONLY") - 1);
 			}
 			smart_str_0(&tmp_str);
 
 			{
-				char * commented_name = NULL;
-				unsigned int commented_name_len = name? mnd_sprintf(&commented_name, 0, " /*%s*/", name):0;
+				char * name_esc = mysqlnd_escape_string_for_tx_name_in_comment(name TSRMLS_CC);
 				char * query;
-				unsigned int query_len = mnd_sprintf(&query, 0, "START TRANSACTION%s %s", commented_name? commented_name:"", tmp_str.c? tmp_str.c:"");
+				unsigned int query_len = mnd_sprintf(&query, 0, "START TRANSACTION%s %s", name_esc? name_esc:"", tmp_str.c? tmp_str.c:"");
 				smart_str_free(&tmp_str);
-
+				if (name_esc) {
+					mnd_efree(name_esc);
+					name_esc = NULL;
+				}
 				if (!query) {
 					SET_OOM_ERROR(*conn->error_info);
 					break;
 				}
 				ret = conn->m->query(conn, query, query_len TSRMLS_CC);
 				mnd_sprintf_free(query);
-				if (commented_name) {
-					mnd_sprintf_free(commented_name);
-				}
 			}
 		} while (0);
 		conn->m->local_tx_end(conn, this_func, ret TSRMLS_CC);