Skip to content

hash_file() appears to be restricted to 3 arguments #11180

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
maecompany opened this issue May 2, 2023 · 4 comments · Fixed by ThePHPF/thephp.foundation#90
Closed

hash_file() appears to be restricted to 3 arguments #11180

maecompany opened this issue May 2, 2023 · 4 comments · Fixed by ThePHPF/thephp.foundation#90

Comments

@maecompany
Copy link

maecompany commented May 2, 2023

Description

According to hash_file(), the function takes in 4 arguments, the latest one being an $options array, however it does not seem to accept 4 arguments.

The following code:

<?php echo hash_file('md5', '/dev/null', false, []); ?>

Resulted in this output:

PHP Fatal error:  Uncaught ArgumentCountError: hash_file() expects at most 3 arguments, 4 given

But I expected this output instead:

... some hash

Shouldn't the ZEND_PARSE_PARAMETERS_START(2, 3) actually be ZEND_PARSE_PARAMETERS_START(2, 4) here:

ZEND_PARSE_PARAMETERS_START(2, 3)
?

PHP Version

PHP 8.1.18

Operating System

Ubuntu 23.04

@sschamp
Copy link

sschamp commented May 2, 2023

Looking at 110b4e9 it would indeed seem like an oversight where ZEND_PARSE_PARAMETERS_START was updated for hash() but not for hash_file()

@nielsdos
Copy link
Member

nielsdos commented May 2, 2023

Feel free to submit a PR :)
EDIT: if you do not wish to make a PR let us know and we'll do the fix.
If you do want to make a PR, please also include a regression test. :)

@alecpl
Copy link

alecpl commented May 4, 2023

BTW, the mentioned commit that introduced the 4th argument also did not include a test for hash() call with 4 arguments. Unless I'm mistaken and it is not required when do a test using named arguments syntax.

@nielsdos
Copy link
Member

nielsdos commented May 6, 2023

BTW, the mentioned commit that introduced the 4th argument also did not include a test for hash() call with 4 arguments. Unless I'm mistaken and it is not required when do a test using named arguments syntax.

The options argument only affects murmurhash right now, and there is a test for that.

I'll go ahead and fix hash_file and add a test.

@nielsdos nielsdos self-assigned this May 6, 2023
nielsdos added a commit to nielsdos/php-src that referenced this issue May 6, 2023
nielsdos added a commit that referenced this issue May 7, 2023
* PHP-8.1:
  Fix GH-11180: hash_file() appears to be restricted to 3 arguments
nielsdos added a commit that referenced this issue May 7, 2023
* PHP-8.2:
  Fix GH-11180: hash_file() appears to be restricted to 3 arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
@alecpl @nielsdos @Girgias @sschamp @maecompany and others