Skip to content

Commit

Permalink
PHP 8 cURL Object vs. Resource Fixes (newrelic#70)
Browse files Browse the repository at this point in the history 
* Wrap curl fn wrapper arg parsing in PHP8-aware conditionals

* Make check_curl_handle macro PHP8 compliant

* Abstract curl_handle validity in function that accounts for pre and post PHP8 format
  • Loading branch information
@kneitinger
kneitinger authored and Joshua Benuck committed Feb 16, 2021
1 parent 93c8c0b commit 8bcaf9d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 11 deletions.
24 changes: 16 additions & 8 deletions agent/php_curl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(ch);
#else
return NULL != ch && nr_php_is_zval_valid_resource(ch);
#endif /* PHP 8.0+ */
}

static void nr_php_curl_save_response_header_from_zval(const zval* ch,
const zval* zstr
TSRMLS_DC) {
Expand Down Expand Up @@ -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 (!nr_php_is_zval_valid_curl_handle(curlres)) {
return;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand Down
15 changes: 15 additions & 0 deletions agent/php_curl_md.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand All @@ -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)) { \
Expand Down
24 changes: 21 additions & 3 deletions agent/php_internal_instrument.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2250,10 +2250,16 @@ NR_INNER_WRAPPER(curl_init) {
NR_INNER_WRAPPER(curl_exec) {
zval* curlres = NULL;
int zcaught = 0;
int rv = FAILURE;

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+ */
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);
#endif /* PHP 8.0+ */
if (SUCCESS != rv) {
nr_wrapper->oldhandler(INTERNAL_FUNCTION_PARAM_PASSTHRU);
return;
}
Expand Down Expand Up @@ -2283,9 +2289,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)
Expand Down Expand Up @@ -2317,9 +2329,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)) {
Expand Down

0 comments on commit 8bcaf9d

Please sign in to comment.