PDO refactor quoter hook to return zend_string*

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

@@ -70,15 +70,13 @@ static int scan(Scanner *s)
 struct placeholder {
 	const char *pos;
 	size_t len;
-	size_t qlen;	/* quoted length of value */
-	char *quoted;	/* quoted value */
-	int freeq;
+	zend_string *quoted;	/* quoted value */
 	int bindno;
 	struct placeholder *next;
 };
 
 static void free_param_name(zval *el) {
-	efree(Z_PTR_P(el));
+	zend_string_release(Z_PTR_P(el));
 }
 
 PDO_API int pdo_parse_params(pdo_stmt_t *stmt, zend_string *inquery, zend_string **outquery)
@@ -123,9 +121,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, zend_string *inquery, zend_string
 
 			if (t == PDO_PARSER_ESCAPED_QUESTION) {
 				plc->bindno = PDO_PARSER_BINDNO_ESCAPED_CHAR;
-				plc->quoted = "?";
-				plc->qlen = 1;
-				plc->freeq = 0;
+				plc->quoted = ZSTR_CHAR('?');
 				escapes++;
 			} else {
 				plc->bindno = bindno++;
@@ -240,16 +236,9 @@ safe:
 						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;
-						}
+
+						plc->quoted = stmt->dbh->methods->quoter(stmt->dbh, buf, param->param_type);
+
 						if (buf) {
 							zend_string_release_ex(buf, 0);
 						}
@@ -258,7 +247,6 @@ safe:
 						ret = -1;
 						goto clean_up;
 					}
-					plc->freeq = 1;
 				} else {
 					enum pdo_param_type param_type = param->param_type;
 					zend_string *buf = NULL;
@@ -270,40 +258,29 @@ safe:
 
 					switch (param_type) {
 						case PDO_PARAM_BOOL:
-							plc->quoted = zend_is_true(parameter) ? "1" : "0";
-							plc->qlen = sizeof("1")-1;
-							plc->freeq = 0;
+							plc->quoted = zend_is_true(parameter) ? ZSTR_CHAR('1') : ZSTR_CHAR('0');
 							break;
 
 						case PDO_PARAM_INT:
-							buf = zend_long_to_str(zval_get_long(parameter));
-
-							plc->qlen = ZSTR_LEN(buf);
-							plc->quoted = estrdup(ZSTR_VAL(buf));
-							plc->freeq = 1;
+							plc->quoted = zend_long_to_str(zval_get_long(parameter));
 							break;
 
 						case PDO_PARAM_NULL:
-							plc->quoted = "NULL";
-							plc->qlen = sizeof("NULL")-1;
-							plc->freeq = 0;
+							plc->quoted = ZSTR_KNOWN(ZEND_STR_NULL);
 							break;
 
-						default:
-							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)) {
+						default: {
+							buf = zval_try_get_string(parameter);
+							/* parameter does not have a string representation, buf == NULL */
+							if (EG(exception)) {
 								/* bork */
 								ret = -1;
 								strncpy(stmt->error_code, stmt->dbh->error_code, 6);
-								if (buf) {
-									zend_string_release_ex(buf, 0);
-								}
 								goto clean_up;
 							}
-							plc->freeq = 1;
+
+							plc->quoted = stmt->dbh->methods->quoter(stmt->dbh, buf, param_type);
+						}
 					}
 
 					if (buf) {
@@ -317,10 +294,9 @@ safe:
 				} else {
 					parameter = &param->parameter;
 				}
-				plc->quoted = Z_STRVAL_P(parameter);
-				plc->qlen = Z_STRLEN_P(parameter);
+				plc->quoted = zend_string_copy(Z_STR_P(parameter));
 			}
-			newbuffer_len += plc->qlen;
+			newbuffer_len += ZSTR_LEN(plc->quoted);
 		}
 
 rewrite:
@@ -339,8 +315,8 @@ rewrite:
 				newbuffer += t;
 			}
 			if (plc->quoted) {
-				memcpy(newbuffer, plc->quoted, plc->qlen);
-				newbuffer += plc->qlen;
+				memcpy(newbuffer, ZSTR_VAL(plc->quoted), ZSTR_LEN(plc->quoted));
+				newbuffer += ZSTR_LEN(plc->quoted);
 			} else {
 				memcpy(newbuffer, plc->pos, plc->len);
 				newbuffer += plc->len;
@@ -363,7 +339,6 @@ rewrite:
 
 	} else if (query_type == PDO_PLACEHOLDER_POSITIONAL) {
 		/* rewrite ? to :pdoX */
-		char *name, *idxbuf;
 		const char *tmpl = stmt->named_rewrite_template ? stmt->named_rewrite_template : ":pdo%d";
 		int bind_no = 1;
 
@@ -376,36 +351,35 @@ rewrite:
 
 		for (plc = placeholders; plc; plc = plc->next) {
 			int skip_map = 0;
-			char *p;
+			zend_string *p;
+			zend_string *idxbuf;
 
 			if (plc->bindno == PDO_PARSER_BINDNO_ESCAPED_CHAR) {
 				continue;
 			}
 
-			name = estrndup(plc->pos, plc->len);
+			zend_string *name = zend_string_init(plc->pos, plc->len, 0);
 
 			/* check if bound parameter is already available */
-			if (!strcmp(name, "?") || (p = zend_hash_str_find_ptr(stmt->bound_param_map, name, plc->len)) == NULL) {
-				spprintf(&idxbuf, 0, tmpl, bind_no++);
+			if (zend_string_equals_literal(name, "?") || (p = zend_hash_find_ptr(stmt->bound_param_map, name)) == NULL) {
+				idxbuf = zend_strpprintf(0, tmpl, bind_no++);
 			} else {
-				idxbuf = estrdup(p);
+				idxbuf = zend_string_copy(p);
 				skip_map = 1;
 			}
 
 			plc->quoted = idxbuf;
-			plc->qlen = strlen(plc->quoted);
-			plc->freeq = 1;
-			newbuffer_len += plc->qlen;
+			newbuffer_len += ZSTR_LEN(plc->quoted);
 
 			if (!skip_map && stmt->named_rewrite_template) {
 				/* create a mapping */
-				zend_hash_str_update_mem(stmt->bound_param_map, name, plc->len, idxbuf, plc->qlen + 1);
+				zend_hash_update_ptr(stmt->bound_param_map, name, zend_string_copy(plc->quoted));
 			}
 
 			/* map number to name */
-			zend_hash_index_update_mem(stmt->bound_param_map, plc->bindno, idxbuf, plc->qlen + 1);
+			zend_hash_index_update_ptr(stmt->bound_param_map, plc->bindno, zend_string_copy(plc->quoted));
 
-			efree(name);
+			zend_string_release(name);
 		}
 
 		goto rewrite;
@@ -421,12 +395,9 @@ rewrite:
 		}
 
 		for (plc = placeholders; plc; plc = plc->next) {
-			char *name;
-			name = estrndup(plc->pos, plc->len);
-			zend_hash_index_update_mem(stmt->bound_param_map, plc->bindno, name, plc->len + 1);
-			efree(name);
-			plc->quoted = "?";
-			plc->qlen = 1;
+			zend_string *name = zend_string_init(plc->pos, plc->len, 0);
+			zend_hash_index_update_ptr(stmt->bound_param_map, plc->bindno, name);
+			plc->quoted = ZSTR_CHAR('?');
 			newbuffer_len -= plc->len - 1;
 		}
 
@@ -438,11 +409,9 @@ clean_up:
 	while (placeholders) {
 		plc = placeholders;
 		placeholders = plc->next;
-
-		if (plc->freeq) {
-			efree(plc->quoted);
+		if (plc->quoted) {
+			zend_string_release_ex(plc->quoted, 0);
 		}
-
 		efree(plc);
 	}
 

ext/pdo/pdo_stmt.c

@@ -50,7 +50,7 @@ static inline bool rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_p
 		 * we will raise an error, as we can't be sure that it is safe
 		 * to bind multiple parameters onto the same zval in the underlying
 		 * driver */
-		char *name;
+		zend_string *name;
 		int position = 0;
 
 		if (stmt->named_rewrite_template) {
@@ -60,7 +60,7 @@ static inline bool rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_p
 		if (!param->name) {
 			/* do the reverse; map the parameter number to the name */
 			if ((name = zend_hash_index_find_ptr(stmt->bound_param_map, param->paramno)) != NULL) {
-				param->name = zend_string_init(name, strlen(name), 0);
+				param->name = zend_string_copy(name);
 				return 1;
 			}
 			/* TODO Error? */
@@ -69,7 +69,7 @@ static inline bool rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_p
 		}
 
 		ZEND_HASH_FOREACH_PTR(stmt->bound_param_map, name) {
-			if (strncmp(name, ZSTR_VAL(param->name), ZSTR_LEN(param->name) + 1)) {
+			if (!zend_string_equals(name, param->name)) {
 				position++;
 				continue;
 			}

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)