Skip to content

Move a few custom type checks to ZPP #6034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/zend_operators.c
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ ZEND_API zend_long ZEND_FASTCALL zval_get_long_func(zval *op) /* {{{ */
} else {
/* Previously we used strtol here, not is_numeric_string,
* and strtol gives you LONG_MAX/_MIN on overflow.
* We use use saturating conversion to emulate strtol()'s
* We use saturating conversion to emulate strtol()'s
* behaviour.
*/
return zend_dval_to_lval_cap(dval);
Expand Down
30 changes: 12 additions & 18 deletions ext/mbstring/mbstring.c
Original file line number Diff line number Diff line change
Expand Up @@ -3513,8 +3513,8 @@ PHP_FUNCTION(mb_send_mail)
size_t message_len;
char *subject;
size_t subject_len;
zval *headers = NULL;
zend_string *extra_cmd = NULL;
HashTable *headers_ht = NULL;
zend_string *str_headers = NULL, *tmp_headers;
size_t n, i;
char *to_r = NULL;
Expand Down Expand Up @@ -3560,30 +3560,24 @@ PHP_FUNCTION(mb_send_mail)
Z_PARAM_STRING(subject, subject_len)
Z_PARAM_STRING(message, message_len)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(headers)
Z_PARAM_STR(extra_cmd)
Z_PARAM_STR_OR_ARRAY_HT_OR_NULL(str_headers, headers_ht)
Z_PARAM_STR_OR_NULL(extra_cmd)
ZEND_PARSE_PARAMETERS_END();

/* ASCIIZ check */
MAIL_ASCIIZ_CHECK_MBSTRING(to, to_len);
MAIL_ASCIIZ_CHECK_MBSTRING(subject, subject_len);
MAIL_ASCIIZ_CHECK_MBSTRING(message, message_len);
if (headers) {
switch(Z_TYPE_P(headers)) {
case IS_STRING:
tmp_headers = zend_string_init(Z_STRVAL_P(headers), Z_STRLEN_P(headers), 0);
MAIL_ASCIIZ_CHECK_MBSTRING(ZSTR_VAL(tmp_headers), ZSTR_LEN(tmp_headers));
str_headers = php_trim(tmp_headers, NULL, 0, 2);
zend_string_release_ex(tmp_headers, 0);
break;
case IS_ARRAY:
str_headers = php_mail_build_headers(Z_ARRVAL_P(headers));
break;
default:
zend_argument_value_error(4, "must be of type string|array|null, %s given", zend_zval_type_name(headers));
RETURN_THROWS();
}

if (str_headers) {
tmp_headers = zend_string_init(ZSTR_VAL(str_headers), ZSTR_LEN(str_headers), 0);
MAIL_ASCIIZ_CHECK_MBSTRING(ZSTR_VAL(tmp_headers), ZSTR_LEN(tmp_headers));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by note: I think the MAIL_ASCIIZ_CHECK_MBSTRING calls above are corrupting the argument strings :(

str_headers = php_trim(tmp_headers, NULL, 0, 2);
zend_string_release_ex(tmp_headers, 0);
} else if (headers_ht) {
str_headers = php_mail_build_headers(headers_ht);
}

if (extra_cmd) {
MAIL_ASCIIZ_CHECK_MBSTRING(ZSTR_VAL(extra_cmd), ZSTR_LEN(extra_cmd));
}
Expand Down
3 changes: 1 addition & 2 deletions ext/mbstring/mbstring.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ function mb_encode_numericentity(string $string, array $convmap, ?string $encodi

function mb_decode_numericentity(string $string, array $convmap, ?string $encoding = null): string {}

/** @param string|array|null $additional_headers */
function mb_send_mail(string $to, string $subject, string $message, $additional_headers = null, ?string $additional_parameters = null): bool {}
function mb_send_mail(string $to, string $subject, string $message, array|string|null $additional_headers = null, ?string $additional_parameters = null): bool {}

function mb_get_info(string $type = "all"): array|string|int|false {}

Expand Down
4 changes: 2 additions & 2 deletions ext/mbstring/mbstring_arginfo.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 5ad8a8cf20eeae59713d19135ecccbee243754eb */
* Stub hash: 84096daa0fd395f57401f11e9e79f7c8420e8a93 */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_language, 0, 0, MAY_BE_STRING|MAY_BE_BOOL)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, language, IS_STRING, 1, "null")
Expand Down Expand Up @@ -178,7 +178,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_send_mail, 0, 3, _IS_BOOL, 0)
ZEND_ARG_TYPE_INFO(0, to, IS_STRING, 0)
ZEND_ARG_TYPE_INFO(0, subject, IS_STRING, 0)
ZEND_ARG_TYPE_INFO(0, message, IS_STRING, 0)
ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, additional_headers, "null")
ZEND_ARG_TYPE_MASK(0, additional_headers, MAY_BE_ARRAY|MAY_BE_STRING|MAY_BE_NULL, "null")
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, additional_parameters, IS_STRING, 1, "null")
ZEND_END_ARG_INFO()

Expand Down
2 changes: 1 addition & 1 deletion ext/oci8/oci8.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ function oci_fetch_all($statement_resource, &$output, int $skip = 0, int $maximu

/**
* @param resource $statement_resource
* @param mixed $output
* @param array $output
* @alias oci_fetch_all
* @deprecated
*/
Expand Down
2 changes: 1 addition & 1 deletion ext/oci8/oci8_arginfo.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: a5a245b354b48a56c274c8f74c974d92ec430853 */
* Stub hash: 447880a4bc4add36beab835cc07c09a254dc0c2b */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_oci_define_by_name, 0, 3, _IS_BOOL, 0)
ZEND_ARG_INFO(0, statement_resource)
Expand Down
7 changes: 2 additions & 5 deletions ext/odbc/odbc.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,8 @@ function odbc_fetch_into($result_id, &$result_array, int $rownumber = 0): int|fa
/** @param resource $result_id */
function odbc_fetch_row($result_id, int $row_number = UNKNOWN): bool {}

/**
* @param resource $result_id
* @param string|int $field
*/
function odbc_result($result_id, $field): string|bool|null {}
/** @param resource $result_id */
function odbc_result($result_id, string|int $field): string|bool|null {}

/** @param resource $result_id */
function odbc_result_all($result_id, string $format = ''): int|false {}
Expand Down
4 changes: 2 additions & 2 deletions ext/odbc/odbc_arginfo.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 14702a5bd87902871d456de2289f4ae236e5bfa5 */
* Stub hash: cf17952d8c3b88f218bbb8d1c21ba40079574c04 */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_close_all, 0, 0, IS_VOID, 0)
ZEND_END_ARG_INFO()
Expand Down Expand Up @@ -70,7 +70,7 @@ ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_odbc_result, 0, 2, MAY_BE_STRING|MAY_BE_BOOL|MAY_BE_NULL)
ZEND_ARG_INFO(0, result_id)
ZEND_ARG_INFO(0, field)
ZEND_ARG_TYPE_MASK(0, field, MAY_BE_STRING|MAY_BE_LONG, NULL)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_odbc_result_all, 0, 1, MAY_BE_LONG|MAY_BE_FALSE)
Expand Down
24 changes: 12 additions & 12 deletions ext/odbc/php_odbc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1736,31 +1736,31 @@ PHP_FUNCTION(odbc_fetch_row)
PHP_FUNCTION(odbc_result)
{
char *field;
zend_string *field_str;
zend_string *field_str, *pv_field_str;
zend_long pv_field_long;
Comment on lines -1739 to +1740
Copy link
Contributor

@carusogabriel carusogabriel Aug 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: moving from zvals to zend_(string|int|etc..) types, are they cheaper?

Also, is that a good practice to follow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carusogabriel I don't think they are noticably cheaper. The real benefit is being able to accept arguments whose type is validated according to the standard semantics, rather than in a custom way.

int field_ind;
SQLSMALLINT sql_c_type = SQL_C_CHAR;
odbc_result *result;
int i = 0;
RETCODE rc;
SQLLEN fieldsize;
zval *pv_res, *pv_field;
zval *pv_res;
#ifdef HAVE_SQL_EXTENDED_FETCH
SQLULEN crow;
SQLUSMALLINT RowStatus[1];
#endif

field_ind = -1;
field = NULL;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "rz", &pv_res, &pv_field) == FAILURE) {
RETURN_THROWS();
}
ZEND_PARSE_PARAMETERS_START(2, 2)
Z_PARAM_RESOURCE(pv_res)
Z_PARAM_STR_OR_LONG(pv_field_str, pv_field_long)
ZEND_PARSE_PARAMETERS_END();

if (Z_TYPE_P(pv_field) == IS_STRING) {
field = Z_STRVAL_P(pv_field);
if (pv_field_str) {
field = ZSTR_VAL(pv_field_str);
field_ind = -1;
} else {
convert_to_long_ex(pv_field);
field_ind = Z_LVAL_P(pv_field) - 1;
field = NULL;
field_ind = (int) pv_field_long - 1;
}

if ((result = (odbc_result *)zend_fetch_resource(Z_RES_P(pv_res), "ODBC result", le_result)) == NULL) {
Expand Down
2 changes: 1 addition & 1 deletion ext/xml/tests/bug72714.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ parse(3015809298423721);
parse(20);
?>
--EXPECTF--
Notice: xml_parser_set_option(): tagstart ignored, because it is out of range in %s%ebug72714.php on line %d
Warning: xml_parser_set_option(): tagstart ignored, because it is out of range in %s on line %d
string(9) "NS1:TOTAL"
string(0) ""
6 changes: 3 additions & 3 deletions ext/xml/tests/xml_parser_set_option_variation3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ $values = array(
// loop through each element of the array for value

foreach($values as $value) {
echo @"\nArg value $value \n";
var_dump( xml_parser_set_option($parser, $option, $value) );
};
echo @"\nArg value $value \n";
var_dump(xml_parser_set_option($parser, $option, $value));
}

fclose($fp);
xml_parser_free($parser);
Expand Down
3 changes: 2 additions & 1 deletion ext/xml/xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -1427,7 +1427,7 @@ PHP_FUNCTION(xml_parser_set_option)
case PHP_XML_OPTION_SKIP_TAGSTART:
parser->toffset = zval_get_long(val);
if (parser->toffset < 0) {
php_error_docref(NULL, E_NOTICE, "tagstart ignored, because it is out of range");
php_error_docref(NULL, E_WARNING, "tagstart ignored, because it is out of range");
parser->toffset = 0;
}
break;
Expand All @@ -1445,6 +1445,7 @@ PHP_FUNCTION(xml_parser_set_option)
zend_argument_value_error(3, "is not a supported target encoding");
RETURN_THROWS();
}

parser->target_encoding = enc->name;
break;
}
Expand Down
14 changes: 4 additions & 10 deletions ext/xsl/php_xsl.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@ public function transformToUri(object $document, string $uri) {}
*/
public function transformToXml(object $document) {}

/**
* @param string|array $name
* @return bool
*/
public function setParameter(string $namespace, $name, string $value = UNKNOWN) {}
/** @return bool */
public function setParameter(string $namespace, array|string $name, ?string $value = null) {}

/** @return string|false */
public function getParameter(string $namespace, string $name) {}
Expand All @@ -40,11 +37,8 @@ public function removeParameter(string $namespace, string $name) {}
/** @return bool */
public function hasExsltSupport() {}

/**
* @param string|array|null $restrict
* @return void
*/
public function registerPHPFunctions($restrict = null) {}
/** @return void */
public function registerPHPFunctions(array|string|null $restrict = null) {}

/** @return bool */
public function setProfiling(?string $filename) {}
Expand Down
8 changes: 4 additions & 4 deletions ext/xsl/php_xsl_arginfo.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 13fd80938fec3bea7ac4bbcfb6e0b69b230fba72 */
* Stub hash: 4a3997bafb6c17714ee94443837be2d2842386e2 */

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_importStylesheet, 0, 0, 1)
ZEND_ARG_TYPE_INFO(0, stylesheet, IS_OBJECT, 0)
Expand All @@ -21,8 +21,8 @@ ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_setParameter, 0, 0, 2)
ZEND_ARG_TYPE_INFO(0, namespace, IS_STRING, 0)
ZEND_ARG_INFO(0, name)
ZEND_ARG_TYPE_INFO(0, value, IS_STRING, 0)
ZEND_ARG_TYPE_MASK(0, name, MAY_BE_ARRAY|MAY_BE_STRING, NULL)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, value, IS_STRING, 1, "null")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_getParameter, 0, 0, 2)
Expand All @@ -36,7 +36,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_hasExsltSupport, 0, 0, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_registerPHPFunctions, 0, 0, 0)
ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, restrict, "null")
ZEND_ARG_TYPE_MASK(0, restrict, MAY_BE_ARRAY|MAY_BE_STRING|MAY_BE_NULL, "null")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XSLTProcessor_setProfiling, 0, 0, 1)
Expand Down
17 changes: 10 additions & 7 deletions ext/xsl/tests/xsltprocessor_setparameter-nostring.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@ Check xsltprocessor::setparameter error handling with no-string
Memleak: http://bugs.php.net/bug.php?id=48221
--SKIPIF--
<?php
if (!extension_loaded('xsl')) {
die("skip\n");
}
if (!extension_loaded('xsl')) {
die("skip\n");
}
?>
--FILE--
<?php
include __DIR__ .'/prepare.inc';
$proc->importStylesheet($xsl);
var_dump($proc->setParameter('', array(4, 'abc')));
try {
$proc->setParameter('', array(4, 'abc'));
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}
$proc->transformToXml($dom);
?>
--EXPECTF--
Warning: XSLTProcessor::setParameter(): Invalid parameter array in %s on line %d
bool(false)
--EXPECT--
XSLTProcessor::setParameter(): Argument #2 ($name) must contain only string keys
--CREDITS--
Christian Weiske, cweiske@php.net
PHP Testfest Berlin 2009-05-09
39 changes: 26 additions & 13 deletions ext/xsl/xsltprocessor.c
Original file line number Diff line number Diff line change
Expand Up @@ -666,21 +666,35 @@ PHP_METHOD(XSLTProcessor, setParameter)
{

zval *id = ZEND_THIS;
zval *array_value, *entry, new_string;
zval *entry, new_string;
HashTable *array_value;
xsl_object *intern;
char *namespace;
size_t namespace_len;
zend_string *string_key, *name, *value;
zend_string *string_key, *name, *value = NULL;

ZEND_PARSE_PARAMETERS_START(2, 3)
Z_PARAM_STRING(namespace, namespace_len)
Z_PARAM_STR_OR_ARRAY_HT(name, array_value)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_NULL(value)
ZEND_PARSE_PARAMETERS_END();

intern = Z_XSL_P(id);

if (array_value) {
if (value) {
zend_argument_value_error(3, "must be null when argument #2 ($name) is an array");
RETURN_THROWS();
}

if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "sa", &namespace, &namespace_len, &array_value) == SUCCESS) {
intern = Z_XSL_P(id);
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(array_value), string_key, entry) {
ZEND_HASH_FOREACH_STR_KEY_VAL(array_value, string_key, entry) {
zval tmp;
zend_string *str;

if (string_key == NULL) {
php_error_docref(NULL, E_WARNING, "Invalid parameter array");
RETURN_FALSE;
zend_argument_type_error(2, "must contain only string keys");
RETURN_THROWS();
}
str = zval_try_get_string(entry);
if (UNEXPECTED(!str)) {
Expand All @@ -690,18 +704,17 @@ PHP_METHOD(XSLTProcessor, setParameter)
zend_hash_update(intern->parameter, string_key, &tmp);
} ZEND_HASH_FOREACH_END();
RETURN_TRUE;
} else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "sSS", &namespace, &namespace_len, &name, &value) == SUCCESS) {

intern = Z_XSL_P(id);
} else {
if (!value) {
zend_argument_value_error(3, "cannot be null when argument #2 ($name) is a string");
RETURN_THROWS();
}

ZVAL_STR_COPY(&new_string, value);

zend_hash_update(intern->parameter, name, &new_string);
RETURN_TRUE;
} else {
WRONG_PARAM_COUNT;
}

}
/* }}} end XSLTProcessor::setParameter */

Expand Down
Loading