From bf4d351bca75e8bb0843517c6abadc8d4a57fe0d Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Mon, 21 Dec 2020 05:20:41 -0800 Subject: [PATCH 1/7] Wrap curl fn wrapper arg parsing in php8 conditionals --- agent/php_internal_instrument.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 8248934eb..fd6be659c 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -2189,9 +2189,14 @@ NR_INNER_WRAPPER(curl_exec) { zval* curlres = NULL; int zcaught = 0; - if (SUCCESS - != zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, - ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres)) { +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ + int parse_status = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); +#else + int parse_status = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); +#endif /* PHP 8.0+ */ + if (SUCCESS != parse_status) { nr_wrapper->oldhandler(INTERNAL_FUNCTION_PARAM_PASSTHRU); return; } @@ -2221,9 +2226,15 @@ NR_INNER_WRAPPER(curl_multi_add_handle) { zval* curlres = NULL; int rv = FAILURE; +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ + rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "oo", &multires, + &curlres); +#else rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "rr", &multires, &curlres); +#endif /* PHP 8.0+ */ if (SUCCESS == rv) { if (nr_php_curl_multi_md_add(multires, curlres TSRMLS_CC) @@ -2255,9 +2266,15 @@ NR_INNER_WRAPPER(curl_multi_remove_handle) { zval* curlres = NULL; int rv = FAILURE; +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ + rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "oo", &multires, + &curlres); +#else rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "rr", &multires, &curlres); +#endif /* PHP 8.0+ */ if (SUCCESS == rv) { if (nr_php_curl_multi_md_remove(multires, curlres TSRMLS_CC)) { From ca522f87ba3d2da6dc51086e141fa634d4da1af6 Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Mon, 21 Dec 2020 06:07:57 -0800 Subject: [PATCH 2/7] Make check_curl_handle macro PHP8 compliant --- agent/php_curl_md.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/agent/php_curl_md.c b/agent/php_curl_md.c index 17b628da2..8c9068e6c 100644 --- a/agent/php_curl_md.c +++ b/agent/php_curl_md.c @@ -14,6 +14,20 @@ static void nr_php_curl_md_destroy(nr_php_curl_md_t* metadata) { nr_free(metadata); } +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ +#define check_curl_handle(ch) \ + ({ \ + bool __ok = true; \ + \ + if (nrunlikely(!nr_php_is_zval_valid_object(ch))) { \ + nrl_verbosedebug(NRL_CAT, "%s: invalid curl handle; not an object", \ + __func__); \ + __ok = false; \ + } \ + \ + __ok; \ + }) +#else #define check_curl_handle(ch) \ ({ \ bool __ok = true; \ @@ -26,6 +40,7 @@ static void nr_php_curl_md_destroy(nr_php_curl_md_t* metadata) { \ __ok; \ }) +#endif #define ensure_curl_metadata_hashmap() \ if (!NRTXNGLOBAL(curl_metadata)) { \ From 16275a76ef8607f32c89fe7ac1b651f23f5d47c6 Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Mon, 21 Dec 2020 06:24:08 -0800 Subject: [PATCH 3/7] Abstract curl_handle validity in function that accounts for pre and post PHP8 --- agent/php_curl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/agent/php_curl.c b/agent/php_curl.c index fb5edffb8..e0b78c1fa 100644 --- a/agent/php_curl.c +++ b/agent/php_curl.c @@ -28,6 +28,14 @@ static int nr_php_curl_do_cross_process(TSRMLS_D) { || NRPRG(txn)->options.distributed_tracing_enabled); } +static int nr_php_is_zval_valid_curl_handle(const zval* ch) { +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ + return NULL != ch && nr_php_is_zval_valid_object(curlres); +#else + return NULL != ch && nr_php_is_zval_valid_resource(curlres); +#endif /* PHP 8.0+ */ +} + static void nr_php_curl_save_response_header_from_zval(const zval* ch, const zval* zstr TSRMLS_DC) { @@ -113,7 +121,7 @@ static void nr_php_curl_set_default_response_header_callback( zval* retval = NULL; zval* curlopt = NULL; - if ((NULL == curlres) || (IS_RESOURCE != Z_TYPE_P(curlres))) { + if ((NULL == curlres)) { return; } @@ -137,7 +145,7 @@ static void nr_php_curl_set_default_request_headers(zval* curlres TSRMLS_DC) { zval* retval = NULL; zval* curlopt = NULL; - if ((NULL == curlres) || (IS_RESOURCE != Z_TYPE_P(curlres))) { + if (!nr_php_is_zval_valid_curl_handle(curlres)) { return; } @@ -358,7 +366,7 @@ static void nr_php_curl_exec_set_httpheaders(zval* curlres, } static void nr_php_curl_setopt_curlopt_writeheader(zval* curlval TSRMLS_DC) { - if ((NULL == curlval) || (IS_RESOURCE != Z_TYPE_P(curlval))) { + if (!nr_php_is_zval_valid_curl_handle(ch)) { return; } @@ -421,8 +429,8 @@ void nr_php_curl_setopt_pre(zval* curlres, return; } - if ((NULL == curlres) || (NULL == curlopt) || (NULL == curlval) - || (IS_RESOURCE != Z_TYPE_P(curlres)) || (IS_LONG != Z_TYPE_P(curlopt))) { + if ( !nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) || (NULL == curlval) + || (IS_LONG != Z_TYPE_P(curlopt))) { return; } @@ -445,8 +453,8 @@ void nr_php_curl_setopt_post(zval* curlres, return; } - if ((NULL == curlres) || (NULL == curlopt) || (NULL == curlval) - || (IS_RESOURCE != Z_TYPE_P(curlres)) || (IS_LONG != Z_TYPE_P(curlopt))) { + if ( !nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) || (NULL == curlval) + || (IS_LONG != Z_TYPE_P(curlopt))) { return; } @@ -526,7 +534,7 @@ void nr_php_curl_setopt_array(zval* curlres, .func = func, }; - if (!nr_php_is_zval_valid_resource(curlres) + if (!nr_php_is_zval_valid_curl_handle(curlres) || !nr_php_is_zval_valid_array(options)) { return; } @@ -541,7 +549,7 @@ static bool nr_php_curl_finished(zval* curlres TSRMLS_DC) { zval* result = NULL; bool finished = false; - if (!nr_php_is_zval_valid_resource(curlres)) { + if (!nr_php_is_zval_valid_curl_handle(curlres)) { return false; } From e2479e6ca77aa574584cce113be58033b3752d21 Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Mon, 21 Dec 2020 13:23:58 -0800 Subject: [PATCH 4/7] Correct curl res vs val copy paste error --- agent/php_curl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agent/php_curl.c b/agent/php_curl.c index e0b78c1fa..0bbe6ccc1 100644 --- a/agent/php_curl.c +++ b/agent/php_curl.c @@ -30,9 +30,9 @@ static int nr_php_curl_do_cross_process(TSRMLS_D) { static int nr_php_is_zval_valid_curl_handle(const zval* ch) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ - return NULL != ch && nr_php_is_zval_valid_object(curlres); + return NULL != ch && nr_php_is_zval_valid_object(ch); #else - return NULL != ch && nr_php_is_zval_valid_resource(curlres); + return NULL != ch && nr_php_is_zval_valid_resource(ch); #endif /* PHP 8.0+ */ } @@ -366,7 +366,7 @@ static void nr_php_curl_exec_set_httpheaders(zval* curlres, } static void nr_php_curl_setopt_curlopt_writeheader(zval* curlval TSRMLS_DC) { - if (!nr_php_is_zval_valid_curl_handle(ch)) { + if ((NULL == curlval) || (IS_RESOURCE != Z_TYPE_P(curlval))) { return; } From 798b8acd7691e4472633bded3d546f0f5669423d Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Mon, 21 Dec 2020 13:26:57 -0800 Subject: [PATCH 5/7] Run clang-format on changed files --- agent/php_curl.c | 8 ++++---- agent/php_curl_md.c | 22 +++++++++++----------- agent/php_internal_instrument.c | 8 ++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/agent/php_curl.c b/agent/php_curl.c index 0bbe6ccc1..3f17e44a9 100644 --- a/agent/php_curl.c +++ b/agent/php_curl.c @@ -429,8 +429,8 @@ void nr_php_curl_setopt_pre(zval* curlres, return; } - if ( !nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) || (NULL == curlval) - || (IS_LONG != Z_TYPE_P(curlopt))) { + if (!nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) + || (NULL == curlval) || (IS_LONG != Z_TYPE_P(curlopt))) { return; } @@ -453,8 +453,8 @@ void nr_php_curl_setopt_post(zval* curlres, return; } - if ( !nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) || (NULL == curlval) - || (IS_LONG != Z_TYPE_P(curlopt))) { + if (!nr_php_is_zval_valid_curl_handle(curlres) || (NULL == curlopt) + || (NULL == curlval) || (IS_LONG != Z_TYPE_P(curlopt))) { return; } diff --git a/agent/php_curl_md.c b/agent/php_curl_md.c index 8c9068e6c..8af04e58a 100644 --- a/agent/php_curl_md.c +++ b/agent/php_curl_md.c @@ -15,17 +15,17 @@ static void nr_php_curl_md_destroy(nr_php_curl_md_t* metadata) { } #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ -#define check_curl_handle(ch) \ - ({ \ - bool __ok = true; \ - \ - if (nrunlikely(!nr_php_is_zval_valid_object(ch))) { \ - nrl_verbosedebug(NRL_CAT, "%s: invalid curl handle; not an object", \ - __func__); \ - __ok = false; \ - } \ - \ - __ok; \ +#define check_curl_handle(ch) \ + ({ \ + bool __ok = true; \ + \ + if (nrunlikely(!nr_php_is_zval_valid_object(ch))) { \ + nrl_verbosedebug(NRL_CAT, "%s: invalid curl handle; not an object", \ + __func__); \ + __ok = false; \ + } \ + \ + __ok; \ }) #else #define check_curl_handle(ch) \ diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index fd6be659c..e3d81f197 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -2190,11 +2190,11 @@ NR_INNER_WRAPPER(curl_exec) { int zcaught = 0; #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ - int parse_status = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, - ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); + int parse_status = zend_parse_parameters_ex( + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); #else - int parse_status = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, - ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); + int parse_status = zend_parse_parameters_ex( + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); #endif /* PHP 8.0+ */ if (SUCCESS != parse_status) { nr_wrapper->oldhandler(INTERNAL_FUNCTION_PARAM_PASSTHRU); From f4674ce999f91adb84318d968c15afeda4b34d66 Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Tue, 22 Dec 2020 10:14:10 -0800 Subject: [PATCH 6/7] Address feedback: formatting, missing check --- agent/php_curl.c | 2 +- agent/php_internal_instrument.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/agent/php_curl.c b/agent/php_curl.c index 3f17e44a9..a2bc945c3 100644 --- a/agent/php_curl.c +++ b/agent/php_curl.c @@ -121,7 +121,7 @@ static void nr_php_curl_set_default_response_header_callback( zval* retval = NULL; zval* curlopt = NULL; - if ((NULL == curlres)) { + if (!nr_php_is_zval_valid_curl_handle(curlres)) { return; } diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index e3d81f197..eae8a9120 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -2188,15 +2188,16 @@ NR_INNER_WRAPPER(curl_init) { NR_INNER_WRAPPER(curl_exec) { zval* curlres = NULL; int zcaught = 0; + int rv = FAILURE; #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ - int parse_status = zend_parse_parameters_ex( + rv = zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); #else - int parse_status = zend_parse_parameters_ex( + rv = zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); #endif /* PHP 8.0+ */ - if (SUCCESS != parse_status) { + if (SUCCESS != rv) { nr_wrapper->oldhandler(INTERNAL_FUNCTION_PARAM_PASSTHRU); return; } From 0dbe92dae3369b3f35e9b6fd6566030f7df4af27 Mon Sep 17 00:00:00 2001 From: Kyle Kneitinger Date: Tue, 22 Dec 2020 10:22:19 -0800 Subject: [PATCH 7/7] clang-format --- agent/php_internal_instrument.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index eae8a9120..71966ea22 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -2191,11 +2191,11 @@ NR_INNER_WRAPPER(curl_exec) { int rv = FAILURE; #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ - rv = zend_parse_parameters_ex( - ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); + rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "o", &curlres); #else - rv = zend_parse_parameters_ex( - ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); + rv = zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, + ZEND_NUM_ARGS() TSRMLS_CC, "r", &curlres); #endif /* PHP 8.0+ */ if (SUCCESS != rv) { nr_wrapper->oldhandler(INTERNAL_FUNCTION_PARAM_PASSTHRU);