ext/curl: Convert php_curl_read to just use FCC without a function name zval

Girgias · Girgias · commit 11bf0427477e · 2024-02-01T00:00:48.000Z
diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h
@@ -53,19 +53,13 @@ typedef struct {
 } php_curl_write;
 
 typedef struct {
-	zval                  func_name;
-	zend_fcall_info_cache fci_cache;
+	zend_fcall_info_cache fcc;
 	FILE                 *fp;
 	zend_resource        *res;
 	int                   method;
 	zval                  stream;
 } php_curl_read;
 
-typedef struct {
-	zval                  func_name;
-	zend_fcall_info_cache fci_cache;
-} php_curl_callback;
-
 typedef struct {
 	php_curl_write    *write;
 	php_curl_write    *write_header;
diff --git a/ext/curl/interface.c b/ext/curl/interface.c
@@ -473,7 +473,9 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n)
 
 	zend_get_gc_buffer_add_zval(gc_buffer, &curl->postfields);
 	if (curl->handlers.read) {
-		zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.read->func_name);
+		if (ZEND_FCC_INITIALIZED(curl->handlers.read->fcc)) {
+			zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.read->fcc);
+		}
 		zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.read->stream);
 	}
 
@@ -765,53 +767,41 @@ static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key,
 static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx)
 {
 	php_curl *ch = (php_curl *)ctx;
-	php_curl_read *t = ch->handlers.read;
+	php_curl_read *read_handler = ch->handlers.read;
 	int length = 0;
 
-	switch (t->method) {
+	switch (read_handler->method) {
 		case PHP_CURL_DIRECT:
-			if (t->fp) {
-				length = fread(data, size, nmemb, t->fp);
+			if (read_handler->fp) {
+				length = fread(data, size, nmemb, read_handler->fp);
 			}
 			break;
 		case PHP_CURL_USER: {
 			zval argv[3];
 			zval retval;
-			zend_result error;
-			zend_fcall_info fci;
 
 			GC_ADDREF(&ch->std);
 			ZVAL_OBJ(&argv[0], &ch->std);
-			if (t->res) {
-				GC_ADDREF(t->res);
-				ZVAL_RES(&argv[1], t->res);
+			if (read_handler->res) {
+				GC_ADDREF(read_handler->res);
+				ZVAL_RES(&argv[1], read_handler->res);
 			} else {
 				ZVAL_NULL(&argv[1]);
 			}
 			ZVAL_LONG(&argv[2], (int)size * nmemb);
 
-			fci.size = sizeof(fci);
-			ZVAL_COPY_VALUE(&fci.function_name, &t->func_name);
-			fci.object = NULL;
-			fci.retval = &retval;
-			fci.param_count = 3;
-			fci.params = argv;
-			fci.named_params = NULL;
-
-			ch->in_callback = 1;
-			error = zend_call_function(&fci, &t->fci_cache);
-			ch->in_callback = 0;
-			if (error == FAILURE) {
-				php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_READFUNCTION");
-				length = CURL_READFUNC_ABORT;
-			} else if (!Z_ISUNDEF(retval)) {
+			ch->in_callback = true;
+			zend_call_known_fcc(&read_handler->fcc, &retval, /* param_count */ 3, argv, /* named_params */ NULL);
+			ch->in_callback = false;
+			if (!Z_ISUNDEF(retval)) {
 				_php_curl_verify_handlers(ch, /* reporterror */ true);
 				if (Z_TYPE(retval) == IS_STRING) {
 					length = MIN((int) (size * nmemb), Z_STRLEN(retval));
 					memcpy(data, Z_STRVAL(retval), length);
 				} else if (Z_TYPE(retval) == IS_LONG) {
 					length = Z_LVAL_P(&retval);
 				}
+				// TODO Do type error if invalid type?
 				zval_ptr_dtor(&retval);
 			}
 
@@ -1149,8 +1139,8 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source)
 	ch->handlers.read->fp = source->handlers.read->fp;
 	ch->handlers.read->res = source->handlers.read->res;
 
-	if (!Z_ISUNDEF(source->handlers.read->func_name)) {
-		ZVAL_COPY(&ch->handlers.read->func_name, &source->handlers.read->func_name);
+	if (ZEND_FCC_INITIALIZED(source->handlers.read->fcc)) {
+		zend_fcc_dup(&source->handlers.read->fcc, &source->handlers.read->fcc);
 	}
 	if (ZEND_FCC_INITIALIZED(source->handlers.write->fcc)) {
 		zend_fcc_dup(&source->handlers.write->fcc, &source->handlers.write->fcc);
@@ -2048,6 +2038,16 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
 			break;
 		}
 
+		case CURLOPT_READFUNCTION:
+			/* Check value is actually a callable and set it */
+			const char option_name[] = "CURLOPT_READFUNCTION";
+			bool result = php_curl_set_callable_handler(&ch->handlers.read->fcc, zvalue, is_array_config, option_name);
+			if (!result) {
+				return FAILURE;
+			}
+			ch->handlers.read->method = PHP_CURL_USER;
+			break;
+
 		case CURLOPT_PROGRESSFUNCTION: {
 			/* Check value is actually a callable and set it */
 			const char option_name[] = "CURLOPT_PROGRESSFUNCTION";
@@ -2098,15 +2098,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
 		}
 #endif
 
-		case CURLOPT_READFUNCTION:
-			if (!Z_ISUNDEF(ch->handlers.read->func_name)) {
-				zval_ptr_dtor(&ch->handlers.read->func_name);
-				ch->handlers.read->fci_cache = empty_fcall_info_cache;
-			}
-			ZVAL_COPY(&ch->handlers.read->func_name, zvalue);
-			ch->handlers.read->method = PHP_CURL_USER;
-			break;
-
 		case CURLOPT_RETURNTRANSFER:
 			if (zend_is_true(zvalue)) {
 				ch->handlers.write->method = PHP_CURL_RETURN;
@@ -2716,13 +2707,15 @@ static void curl_free_obj(zend_object *object)
 	}
 
 	smart_str_free(&ch->handlers.write->buf);
-	zval_ptr_dtor(&ch->handlers.read->func_name);
 	if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) {
 		zend_fcc_dtor(&ch->handlers.write->fcc);
 	}
 	if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) {
 		zend_fcc_dtor(&ch->handlers.write_header->fcc);
 	}
+	if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) {
+		zend_fcc_dtor(&ch->handlers.read->fcc);
+	}
 	zval_ptr_dtor(&ch->handlers.std_err);
 	if (ch->header.str) {
 		zend_string_release_ex(ch->header.str, 0);
diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt
@@ -29,6 +29,7 @@ testOption($ch, CURLOPT_XFERINFOFUNCTION);
 testOption($ch, CURLOPT_FNMATCH_FUNCTION);
 testOption($ch, CURLOPT_WRITEFUNCTION);
 testOption($ch, CURLOPT_HEADERFUNCTION);
+testOption($ch, CURLOPT_READFUNCTION);
 
 ?>
 --EXPECT--
@@ -44,3 +45,5 @@ TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for opti
 TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_WRITEFUNCTION, function "undefined" not found or invalid function name
 TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name
 TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name
+TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_READFUNCTION, function "undefined" not found or invalid function name
+TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_READFUNCTION, function "undefined" not found or invalid function name
