Skip to content

PHP Deprecated: Required parameter $contentType follows optional parameter $filename #21

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
bfoosness opened this issue Nov 19, 2020 · 5 comments

Comments

@bfoosness
Copy link

This line results in a deprecation error in PHP 8:

PHP Deprecated: Required parameter $contentType follows optional parameter $filename in /site/vendor/mikehaertl/php-tmpfile/src/File.php on line 73

public function send($filename = null, $contentType, $inline = false)

I'm happy to create a PR if you have a preferred solution. Changing the parameter order or requiring the first parameter would be a BC break. Alternatively, a default value of null could be added to $contentType (turning it into an optional parameter) and then an exception thrown if it remains null.

@mikehaertl
Copy link
Owner

Thanks, will have a look. Maybe we should use mime_content_type() if no $contentType is passed and let it default to application/octet-stream if that function also fails. What do you think?

I first thought about making the Content-Type header optional but I think this will break the download in some browsers so it should always be set.

@bfoosness
Copy link
Author

Yeah, if you think that's a reasonable default, that could be a good solution. There shouldn't be any calls to this method without a content type now, so any immediate consequences to this default value should be minimal to none, making it a safe change.

@bfoosness
Copy link
Author

I've created an initial attempt at a fix if you'd like to discuss it. Unfortunately there are no tests covering the send method.

@mikehaertl
Copy link
Owner

@bfoosness Just released 1.1.7 including this fix. Thanks for your help.

@bfoosness
Copy link
Author

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants