Refactor PDO's quoter handler to return a zend_string

Closes GH-6547

UPGRADING.INTERNALS

@@ -47,8 +47,12 @@ PHP 8.1 INTERNALS UPGRADE NOTES
       getColumnMeta(). The type provided here does not need to match the type
       returned by get_col (in fact no corresponding type might exist, e.g. for
       floats). It should be the closest logical equivalent for the column type.
-    - The transaction, set_attribute, quoter, and preparer handler's return type
+    - The transaction, set_attribute, and preparer handler's return type
       has been formalized to bool instead of int.
     - The check_liveness handler's return type has been formalized to zend_return
       instead of int.
     - The closer, and fetch_error handlers have been voidified.
+    - The quoter handler now returns the quoted string as zend_string* instead
+      of returning a boolean, and the quoted string as a pair of out params.
+      Similarly the unquoted string is now a zend_string* instead of a pair of
+      char* and size_t length.

ext/pdo/pdo_dbh.c

@@ -1119,18 +1119,17 @@ PHP_METHOD(PDO, query)
 }
 /* }}} */
 
-/* {{{ quotes string for use in a query. The optional paramtype acts as a hint for drivers that have alternate quoting styles. The default value is PDO_PARAM_STR */
+/* {{{ quotes string for use in a query.
+ * The optional paramtype acts as a hint for drivers that have alternate quoting styles.
+ * The default value is PDO_PARAM_STR */
 PHP_METHOD(PDO, quote)
 {
 	pdo_dbh_t *dbh = Z_PDO_DBH_P(ZEND_THIS);
-	char *str;
-	size_t str_len;
+	zend_string *str;
 	zend_long paramtype = PDO_PARAM_STR;
-	char *qstr;
-	size_t qlen;
 
 	ZEND_PARSE_PARAMETERS_START(1, 2)
-		Z_PARAM_STRING(str, str_len)
+		Z_PARAM_STR(str)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_LONG(paramtype)
 	ZEND_PARSE_PARAMETERS_END();
@@ -1143,13 +1142,7 @@ PHP_METHOD(PDO, quote)
 		RETURN_FALSE;
 	}
 
-	if (dbh->methods->quoter(dbh, str, str_len, &qstr, &qlen, paramtype)) {
-		RETVAL_STRINGL(qstr, qlen);
-		efree(qstr);
-		return;
-	}
-	PDO_HANDLE_DBH_ERR();
-	RETURN_FALSE;
+	RETURN_STR(dbh->methods->quoter(dbh, str, paramtype));
 }
 /* }}} */
 

ext/pdo/pdo_sql_parser.re

@@ -235,21 +235,18 @@ safe:
 					php_stream_from_zval_no_verify(stm, parameter);
 					if (stm) {
 						zend_string *buf;
+						zend_string *quoted_buf;
 
 						buf = php_stream_copy_to_mem(stm, PHP_STREAM_COPY_ALL, 0);
 						if (!buf) {
 							buf = ZSTR_EMPTY_ALLOC();
 						}
-						if (!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf), ZSTR_LEN(buf), &plc->quoted, &plc->qlen,
-								param->param_type)) {
-							/* bork */
-							ret = -1;
-							strncpy(stmt->error_code, stmt->dbh->error_code, 6);
-							if (buf) {
-								zend_string_release_ex(buf, 0);
-							}
-							goto clean_up;
-						}
+
+						quoted_buf = stmt->dbh->methods->quoter(stmt->dbh, buf, param->param_type);
+						plc->quoted = estrndup(ZSTR_VAL(quoted_buf), ZSTR_LEN(quoted_buf));
+						plc->qlen = ZSTR_LEN(quoted_buf);
+						zend_string_release_ex(quoted_buf, 0);
+
 						if (buf) {
 							zend_string_release_ex(buf, 0);
 						}
@@ -289,21 +286,29 @@ safe:
 							plc->freeq = 0;
 							break;
 
-						default:
+						default: {
+							zend_string *quoted_buf;
+
+							/* TODO Should this be zval_try_get_string_func() ? */
 							buf = zval_get_string(parameter);
-							if (EG(exception) ||
-								!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf),
-									ZSTR_LEN(buf), &plc->quoted, &plc->qlen,
-									param_type)) {
+							/* TODO Check when this can occur? */
+							if (EG(exception)) {
 								/* bork */
 								ret = -1;
 								strncpy(stmt->error_code, stmt->dbh->error_code, 6);
+								/* TODO Is this dead code now? */
 								if (buf) {
 									zend_string_release_ex(buf, 0);
 								}
 								goto clean_up;
 							}
+
+							quoted_buf = stmt->dbh->methods->quoter(stmt->dbh, buf, param_type);
+							plc->quoted = estrndup(ZSTR_VAL(quoted_buf), ZSTR_LEN(quoted_buf));
+							plc->qlen = ZSTR_LEN(quoted_buf);
 							plc->freeq = 1;
+							zend_string_release_ex(quoted_buf, 0);
+						}
 					}
 
 					if (buf) {

ext/pdo/php_pdo_driver.h

@@ -236,7 +236,7 @@ typedef bool (*pdo_dbh_prepare_func)(pdo_dbh_t *dbh, zend_string *sql, pdo_stmt_
 typedef zend_long (*pdo_dbh_do_func)(pdo_dbh_t *dbh, const char *sql, size_t sql_len);
 
 /* quote a string */
-typedef bool (*pdo_dbh_quote_func)(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype);
+typedef zend_string* (*pdo_dbh_quote_func)(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype);
 
 /* transaction related (beingTransaction(), commit, rollBack, inTransaction)
  * Return true if currently inside a transaction, false otherwise. */

ext/pdo_dblib/dblib_driver.c

@@ -142,14 +142,14 @@ static zend_long dblib_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_l
 	return DBCOUNT(H->link);
 }
 
-static bool dblib_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype)
+static zend_string* dblib_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
 {
 	pdo_dblib_db_handle *H = (pdo_dblib_db_handle *)dbh->driver_data;
 	zend_bool use_national_character_set = 0;
-
 	size_t i;
-	char * q;
-	*quotedlen = 0;
+	char *q;
+	size_t quotedlen = 0;
+	zend_string *quoted_str;
 
 	if (H->assume_national_character_set_strings) {
 		use_national_character_set = 1;
@@ -162,34 +162,34 @@ static bool dblib_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unq
 	}
 
 	/* Detect quoted length, adding extra char for doubled single quotes */
-	for (i = 0; i < unquotedlen; i++) {
-		if (unquoted[i] == '\'') ++*quotedlen;
-		++*quotedlen;
+	for (i = 0; i < ZSTR_LEN(unquoted); i++) {
+		if (ZSTR_VAL(unquoted)[i] == '\'') ++quotedlen;
+		++quotedlen;
 	}
 
-	*quotedlen += 2; /* +2 for opening, closing quotes */
+	quotedlen += 2; /* +2 for opening, closing quotes */
 	if (use_national_character_set) {
-		++*quotedlen; /* N prefix */
+		++quotedlen; /* N prefix */
 	}
-	q = *quoted = emalloc(*quotedlen + 1); /* Add byte for terminal null */
+	quoted_str = zend_string_alloc(quotedlen, 0);
+	q = ZSTR_VAL(quoted_str);
 	if (use_national_character_set) {
 		*q++ = 'N';
 	}
 	*q++ = '\'';
 
-	for (i = 0; i < unquotedlen; i++) {
-		if (unquoted[i] == '\'') {
+	for (i = 0; i < ZSTR_LEN(unquoted); i++) {
+		if (ZSTR_VAL(unquoted)[i] == '\'') {
 			*q++ = '\'';
 			*q++ = '\'';
 		} else {
-			*q++ = unquoted[i];
+			*q++ = ZSTR_VAL(unquoted)[i];
 		}
 	}
 	*q++ = '\'';
+	*q = '\0';
 
-	*q = 0;
-
-	return true;
+	return quoted_str;
 }
 
 static bool pdo_dblib_transaction_cmd(const char *cmd, pdo_dbh_t *dbh)

ext/pdo_firebird/firebird_driver.c

@@ -649,30 +649,29 @@ free_statement:
 /* }}} */
 
 /* called by the PDO SQL parser to add quotes to values that are copied into SQL */
-static bool firebird_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, /* {{{ */
-	char **quoted, size_t *quotedlen, enum pdo_param_type paramtype)
+static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
 {
 	int qcount = 0;
 	char const *co, *l, *r;
 	char *c;
+	size_t quotedlen;
+	zend_string *quoted_str;
 
-	if (!unquotedlen) {
-		*quotedlen = 2;
-		*quoted = emalloc(*quotedlen+1);
-		strcpy(*quoted, "''");
-		return true;
+	if (ZSTR_LEN(unquoted) == 0) {
+		return zend_string_init("''", 2, 0);
 	}
 
 	/* Firebird only requires single quotes to be doubled if string lengths are used */
 	/* count the number of ' characters */
-	for (co = unquoted; (co = strchr(co,'\'')); qcount++, co++);
+	for (co = ZSTR_VAL(unquoted); (co = strchr(co,'\'')); qcount++, co++);
 
-	*quotedlen = unquotedlen + qcount + 2;
-	*quoted = c = emalloc(*quotedlen+1);
+	quotedlen = ZSTR_LEN(unquoted) + qcount + 2;
+	quoted_str = zend_string_alloc(quotedlen, 0);
+	c = ZSTR_VAL(quoted_str);
 	*c++ = '\'';
 
 	/* foreach (chunk that ends in a quote) */
-	for (l = unquoted; (r = strchr(l,'\'')); l = r+1) {
+	for (l = ZSTR_VAL(unquoted); (r = strchr(l,'\'')); l = r+1) {
 		strncpy(c, l, r-l+1);
 		c += (r-l+1);
 		/* add the second quote */
@@ -680,11 +679,11 @@ static bool firebird_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t
 	}
 
 	/* copy the remainder */
-	strncpy(c, l, *quotedlen-(c-*quoted)-1);
-	(*quoted)[*quotedlen-1] = '\'';
-	(*quoted)[*quotedlen]   = '\0';
+	strncpy(c, l, quotedlen-(c-ZSTR_VAL(quoted_str))-1);
+	ZSTR_VAL(quoted_str)[quotedlen-1] = '\'';
+	ZSTR_VAL(quoted_str)[quotedlen]   = '\0';
 
-	return true;
+	return quoted_str;
 }
 /* }}} */
 

ext/pdo_mysql/mysql_driver.c

@@ -301,10 +301,13 @@ static char *pdo_mysql_last_insert_id(pdo_dbh_t *dbh, const char *name, size_t *
 #endif
 
 /* {{{ mysql_handle_quoter */
-static bool mysql_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype )
+static zend_string* mysql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype )
 {
 	pdo_mysql_db_handle *H = (pdo_mysql_db_handle *)dbh->driver_data;
 	zend_bool use_national_character_set = 0;
+	char *quoted;
+	size_t quotedlen;
+	zend_string *quoted_str;
 
 	if (H->assume_national_character_set_strings) {
 		use_national_character_set = 1;
@@ -318,24 +321,27 @@ static bool mysql_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unq
 
 	PDO_DBG_ENTER("mysql_handle_quoter");
 	PDO_DBG_INF_FMT("dbh=%p", dbh);
-	PDO_DBG_INF_FMT("unquoted=%.*s", (int)unquotedlen, unquoted);
-	*quoted = safe_emalloc(2, unquotedlen, 3 + (use_national_character_set ? 1 : 0));
+	PDO_DBG_INF_FMT("unquoted=%.*s", (int)ZSTR_LEN(unquoted), ZSTR_VAL(unquoted));
+	quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3 + (use_national_character_set ? 1 : 0));
 
 	if (use_national_character_set) {
-		*quotedlen = mysql_real_escape_string_quote(H->server, *quoted + 2, unquoted, unquotedlen, '\'');
-		(*quoted)[0] = 'N';
-		(*quoted)[1] = '\'';
+		quotedlen = mysql_real_escape_string_quote(H->server, quoted + 2, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), '\'');
+		quoted[0] = 'N';
+		quoted[1] = '\'';
 
-		++*quotedlen; /* N prefix */
+		++quotedlen; /* N prefix */
 	} else {
-		*quotedlen = mysql_real_escape_string_quote(H->server, *quoted + 1, unquoted, unquotedlen, '\'');
-		(*quoted)[0] = '\'';
+		quotedlen = mysql_real_escape_string_quote(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), '\'');
+		quoted[0] = '\'';
 	}
 
-	(*quoted)[++*quotedlen] = '\'';
-	(*quoted)[++*quotedlen] = '\0';
-	PDO_DBG_INF_FMT("quoted=%.*s", (int)*quotedlen, *quoted);
-	PDO_DBG_RETURN(true);
+	quoted[++quotedlen] = '\'';
+	quoted[++quotedlen] = '\0';
+	PDO_DBG_INF_FMT("quoted=%.*s", (int)quotedlen, quoted);
+
+	quoted_str = zend_string_init(quoted, quotedlen, 0);
+	efree(quoted);
+	PDO_DBG_RETURN(quoted_str);
 }
 /* }}} */
 

ext/pdo_oci/oci_driver.c

@@ -350,40 +350,40 @@ static zend_long oci_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_len
 }
 /* }}} */
 
-static bool oci_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype ) /* {{{ */
+static zend_string* oci_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype ) /* {{{ */
 {
 	int qcount = 0;
 	char const *cu, *l, *r;
-	char *c;
+	char *c, *quoted;
+	zend_string *quoted_str;
 
-	if (!unquotedlen) {
-		*quotedlen = 2;
-		*quoted = emalloc(*quotedlen+1);
-		strcpy(*quoted, "''");
-		return true;
+	if (ZSTR_LEN(unquoted) == 0) {
+		return zend_string_init("''", 2, 0);
 	}
 
 	/* count single quotes */
-	for (cu = unquoted; (cu = strchr(cu,'\'')); qcount++, cu++)
+	for (cu = ZSTR_VAL(unquoted); (cu = strchr(cu,'\'')); qcount++, cu++)
 		; /* empty loop */
 
-	*quotedlen = unquotedlen + qcount + 2;
-	*quoted = c = emalloc(*quotedlen+1);
+	quotedlen = ZSTR_LEN(unquoted) + qcount + 2;
+	quoted = c = emalloc(quotedlen+1);
 	*c++ = '\'';
 
 	/* foreach (chunk that ends in a quote) */
-	for (l = unquoted; (r = strchr(l,'\'')); l = r+1) {
+	for (l = ZSTR_VAL(unquoted); (r = strchr(l,'\'')); l = r+1) {
 		strncpy(c, l, r-l+1);
 		c += (r-l+1);
 		*c++ = '\'';			/* add second quote */
 	}
 
     /* Copy remainder and add enclosing quote */
-	strncpy(c, l, *quotedlen-(c-*quoted)-1);
-	(*quoted)[*quotedlen-1] = '\'';
-	(*quoted)[*quotedlen]   = '\0';
+	strncpy(c, l, quotedlen-(c-quoted)-1);
+	quoted[quotedlen-1] = '\'';
+	quoted[quotedlen]   = '\0';
 
-	return true;
+	quoted_str = zend_string_init(quoted, quotedlen, 0);
+	efree(quoted);
+	return quoted_str;
 }
 /* }}} */
 

ext/pdo_pgsql/pgsql_driver.c

@@ -317,33 +317,39 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_l
 	return ret;
 }
 
-static bool pgsql_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype)
+static zend_string* pgsql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
 {
 	unsigned char *escaped;
+	char *quoted;
+	size_t quotedlen;
+	zend_string *quoted_str;
 	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
 	size_t tmp_len;
 
 	switch (paramtype) {
 		case PDO_PARAM_LOB:
 			/* escapedlen returned by PQescapeBytea() accounts for trailing 0 */
-			escaped = PQescapeByteaConn(H->server, (unsigned char *)unquoted, unquotedlen, &tmp_len);
-			*quotedlen = tmp_len + 1;
-			*quoted = emalloc(*quotedlen + 1);
-			memcpy((*quoted)+1, escaped, *quotedlen-2);
-			(*quoted)[0] = '\'';
-			(*quoted)[*quotedlen-1] = '\'';
-			(*quoted)[*quotedlen] = '\0';
+			escaped = PQescapeByteaConn(H->server, (unsigned char *)ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), &tmp_len);
+			quotedlen = tmp_len + 1;
+			quoted = emalloc(quotedlen + 1);
+			memcpy(quoted+1, escaped, quotedlen-2);
+			quoted[0] = '\'';
+			quoted[quotedlen-1] = '\'';
+			quoted[quotedlen] = '\0';
 			PQfreemem(escaped);
 			break;
 		default:
-			*quoted = safe_emalloc(2, unquotedlen, 3);
-			(*quoted)[0] = '\'';
-			*quotedlen = PQescapeStringConn(H->server, *quoted + 1, unquoted, unquotedlen, NULL);
-			(*quoted)[*quotedlen + 1] = '\'';
-			(*quoted)[*quotedlen + 2] = '\0';
-			*quotedlen += 2;
+			quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
+			quoted[0] = '\'';
+			quotedlen = PQescapeStringConn(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), NULL);
+			quoted[quotedlen + 1] = '\'';
+			quoted[quotedlen + 2] = '\0';
+			quotedlen += 2;
 	}
-	return true;
+
+	quoted_str = zend_string_init(quoted, quotedlen, 0);
+	efree(quoted);
+	return quoted_str;
 }
 
 static char *pdo_pgsql_last_insert_id(pdo_dbh_t *dbh, const char *name, size_t *len)

ext/pdo_sqlite/sqlite_driver.c

@@ -227,12 +227,14 @@ static char *pdo_sqlite_last_insert_id(pdo_dbh_t *dbh, const char *name, size_t
 }
 
 /* NB: doesn't handle binary strings... use prepared stmts for that */
-static bool sqlite_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type paramtype )
+static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
 {
-	*quoted = safe_emalloc(2, unquotedlen, 3);
-	sqlite3_snprintf(2*unquotedlen + 3, *quoted, "'%q'", unquoted);
-	*quotedlen = strlen(*quoted);
-	return true;
+	char *quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
+	/* TODO use %Q format? */
+	sqlite3_snprintf(2*ZSTR_LEN(unquoted) + 3, quoted, "'%q'", ZSTR_VAL(unquoted));
+	zend_string *quoted_str = zend_string_init(quoted, strlen(quoted), 0);
+	efree(quoted);
+	return quoted_str;
 }
 
 static bool sqlite_handle_begin(pdo_dbh_t *dbh)