Skip to content

Deprecate passing incorrect data types for options to ext/hash functions #15236

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

Merged
merged 1 commit into from
Aug 6, 2024
Merged
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
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ PHP NEWS
. Deprecated DOM_PHP_ERR constant. (nielsdos)
. Removed DOMImplementation::getFeature(). (nielsdos)

- Hash:
. Deprecated passing incorrect data types for options to ext/hash functions.
(nielsdos)

- PHPDBG:
. array out of bounds, stack overflow handled for segfault handler on windows.
(David Carlier)
Expand Down
4 changes: 4 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ PHP 8.4 UPGRADE NOTES
. Deprecated DOM_PHP_ERR constant.
RFC: https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_dom_php_err_constant

- Hash:
. Deprecated passing incorrect data types for options to ext/hash functions.
RFC: https://wiki.php.net/rfc/deprecations_php_8_4

- Intl:
. Calling intlcal_set() as well as calling IntlCalendar::set() with
more than 2 arguments is deprecated. Use either IntlCalendar::setDate()
Expand Down
39 changes: 27 additions & 12 deletions ext/hash/hash_murmur.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,13 @@ PHP_HASH_API void PHP_MURMUR3AInit(PHP_MURMUR3A_CTX *ctx, HashTable *args)
zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1);
/* This might be a bit too restrictive, but thinking that a seed might be set
once and for all, it should be done a clean way. */
if (seed && IS_LONG == Z_TYPE_P(seed)) {
ctx->h = (uint32_t)Z_LVAL_P(seed);
if (seed) {
if (IS_LONG == Z_TYPE_P(seed)) {
ctx->h = (uint32_t) Z_LVAL_P(seed);
} else {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0");
ctx->h = 0;
}
} else {
ctx->h = 0;
}
Expand Down Expand Up @@ -99,12 +104,17 @@ PHP_HASH_API void PHP_MURMUR3CInit(PHP_MURMUR3C_CTX *ctx, HashTable *args)
zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1);
/* This might be a bit too restrictive, but thinking that a seed might be set
once and for all, it should be done a clean way. */
if (seed && IS_LONG == Z_TYPE_P(seed)) {
uint32_t _seed = (uint32_t)Z_LVAL_P(seed);
ctx->h[0] = _seed;
ctx->h[1] = _seed;
ctx->h[2] = _seed;
ctx->h[3] = _seed;
if (seed) {
if (IS_LONG == Z_TYPE_P(seed)) {
uint32_t _seed = (uint32_t)Z_LVAL_P(seed);
ctx->h[0] = _seed;
ctx->h[1] = _seed;
ctx->h[2] = _seed;
ctx->h[3] = _seed;
} else {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0");
memset(&ctx->h, 0, sizeof ctx->h);
}
} else {
memset(&ctx->h, 0, sizeof ctx->h);
}
Expand Down Expand Up @@ -173,10 +183,15 @@ PHP_HASH_API void PHP_MURMUR3FInit(PHP_MURMUR3F_CTX *ctx, HashTable *args)
zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1);
/* This might be a bit too restrictive, but thinking that a seed might be set
once and for all, it should be done a clean way. */
if (seed && IS_LONG == Z_TYPE_P(seed)) {
uint64_t _seed = (uint64_t)Z_LVAL_P(seed);
ctx->h[0] = _seed;
ctx->h[1] = _seed;
if (seed) {
if (IS_LONG == Z_TYPE_P(seed)) {
uint64_t _seed = (uint64_t) Z_LVAL_P(seed);
ctx->h[0] = _seed;
ctx->h[1] = _seed;
} else {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0");
memset(&ctx->h, 0, sizeof ctx->h);
}
} else {
memset(&ctx->h, 0, sizeof ctx->h);
}
Expand Down
29 changes: 20 additions & 9 deletions ext/hash/hash_xxhash.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,17 @@ PHP_HASH_API void PHP_XXH32Init(PHP_XXH32_CTX *ctx, HashTable *args)
zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1);
/* This might be a bit too restrictive, but thinking that a seed might be set
once and for all, it should be done a clean way. */
if (seed && IS_LONG == Z_TYPE_P(seed)) {
XXH32_reset(&ctx->s, (XXH32_hash_t)Z_LVAL_P(seed));
} else {
XXH32_reset(&ctx->s, 0);
if (seed) {
if (IS_LONG == Z_TYPE_P(seed)) {
XXH32_reset(&ctx->s, (XXH32_hash_t)Z_LVAL_P(seed));
return;
} else {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0");
}
}
} else {
XXH32_reset(&ctx->s, 0);
}

XXH32_reset(&ctx->s, 0);
}

PHP_HASH_API void PHP_XXH32Update(PHP_XXH32_CTX *ctx, const unsigned char *in, size_t len)
Expand Down Expand Up @@ -112,12 +115,13 @@ PHP_HASH_API void PHP_XXH64Init(PHP_XXH64_CTX *ctx, HashTable *args)
once and for all, it should be done a clean way. */
if (seed && IS_LONG == Z_TYPE_P(seed)) {
XXH64_reset(&ctx->s, (XXH64_hash_t)Z_LVAL_P(seed));
return;
} else {
XXH64_reset(&ctx->s, 0);
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0");
}
} else {
XXH64_reset(&ctx->s, 0);
}

XXH64_reset(&ctx->s, 0);
}

PHP_HASH_API void PHP_XXH64Update(PHP_XXH64_CTX *ctx, const unsigned char *in, size_t len)
Expand Down Expand Up @@ -168,12 +172,19 @@ zend_always_inline static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *a
return;
}

if (_seed && IS_LONG != Z_TYPE_P(_seed)) {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is ignored");
}

if (_seed && IS_LONG == Z_TYPE_P(_seed)) {
/* This might be a bit too restrictive, but thinking that a seed might be set
once and for all, it should be done a clean way. */
func_init_seed(&ctx->s, (XXH64_hash_t)Z_LVAL_P(_seed));
return;
} else if (_secret) {
if (IS_STRING != Z_TYPE_P(_secret)) {
php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs");
}
zend_string *secret_string = zval_try_get_string(_secret);
if (UNEXPECTED(!secret_string)) {
ZEND_ASSERT(EG(exception));
Expand Down
16 changes: 16 additions & 0 deletions ext/hash/tests/murmur_seed_deprecation.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Hash: murmur3 seed deprecation of edge cases
--FILE--
<?php

foreach (["murmur3a", "murmur3c", "murmur3f"] as $a) {
hash_init($a, options: ["seed" => "42"]);
}

?>
--EXPECTF--
Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d
3 changes: 2 additions & 1 deletion ext/hash/tests/xxh3_convert_secret_to_string.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ try {
} catch (Throwable) {}
var_dump($x);
?>
--EXPECT--
--EXPECTF--
Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d
array(1) {
["secret"]=>
int(4)
Expand Down
6 changes: 5 additions & 1 deletion ext/hash/tests/xxhash_secret.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,16 @@ foreach (["xxh3", "xxh128"] as $a) {
}

?>
--EXPECT--
--EXPECTF--
string(67) "xxh3: Only one of seed or secret is to be passed for initialization"

Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d
string(23) "exception in __toString"
string(57) "xxh3: Secret length must be >= 136 bytes, 17 bytes passed"
8028aa834c03557a == 8028aa834c03557a == true
string(69) "xxh128: Only one of seed or secret is to be passed for initialization"

Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d
string(23) "exception in __toString"
string(59) "xxh128: Secret length must be >= 136 bytes, 17 bytes passed"
54279097795e7218093a05d4d781cbb9 == 54279097795e7218093a05d4d781cbb9 == true
28 changes: 28 additions & 0 deletions ext/hash/tests/xxhash_seed_deprecation.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
Hash: xxHash seed deprecation of edge cases
--FILE--
<?php

foreach (["xxh32", "xxh64", "xxh3", "xxh128"] as $a) {
hash_init($a, options: ["seed" => "42"]);
}

foreach (["xxh3", "xxh128"] as $a) {
try {
hash_init($a, options: ["secret" => 42]);
} catch (Throwable) {}
}

?>
--EXPECTF--
Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is ignored in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is ignored in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d

Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d
Loading