Skip to content
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

Update admin.php #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update admin.php #2

wants to merge 1 commit into from

Conversation

k1r10n
Copy link

@k1r10n k1r10n commented Dec 20, 2023

did not work on 6.4.2 WordPress.

AH01071: Got error 'PHP message: PHP Fatal error: Uncaught TypeError: unserialize(): Argument #1 ($data) must be of type string, array given in /wp-content/plugins/wp-add-mime-types/includes/admin.php:27

The error message indicates that unserialize() is expecting a string as its argument, but it's receiving an array. To fix this issue, removed unserialize() as if it's already an array, so it will be assigned directly to $mime_type_values

did not work on 6.4.2 WordPress. 

AH01071: Got error 'PHP message: PHP Fatal error:  Uncaught TypeError: unserialize(): Argument kimipooh#1 ($data) must be of type string, array given in /wp-content/plugins/wp-add-mime-types/includes/admin.php:27

The error message indicates that unserialize() is expecting a string as its argument, but it's receiving an array. To fix this issue, removed unserialize() as if it's already an array, so it will be assigned directly to $mime_type_values
@kimipooh
Copy link
Owner

Thank you for the pull request and comment.

I think the problem may be a type conversion mismatch in variable initialization.

You’d like to replace the following code on line 25 with
$mime_type_values = "";

I think the error will go away by changing it to the following
$mime_type_values = array();

Please check it.
We have released version 3.1.0 with the above fixes.

@k1r10n
Copy link
Author

k1r10n commented Dec 20, 2023

no, unserialize() expects a string but an array is given so it is already unserialized into a PHP object and no need to call unserialize().

removing unserialize() make it work again

@kimipooh
Copy link
Owner

In my environment, the data before unserializing was not yet unserialized, and it was necessary to unserialize it with the unserialize function. We confirmed that if the data is not unserialized, the set value cannot be read and the plugin setting will be empty.
I also looked at the plugin settings saved in the database, which were also serialized.

Also, in this environment, I do not see the error you presented.
WordPress 6.4.2
PHP 8.2.0

What version of PHP are you using?

@k1r10n
Copy link
Author

k1r10n commented Dec 20, 2023

Ok, I probably need to upgrade my php from PHP 8.1.x to the new one. I will use the patch I suggested for now until I migrate to the new version of php.

FYI: I tested the new release 3.1.0 in my environment - same error:
AH01071: Got error 'PHP message: PHP Fatal error: Uncaught TypeError: unserialize(): Argument #1 ($data) must be of type string, array given in /wp-content/plugins/wp-add-mime-types/includes/admin.php:27

@kimipooh
Copy link
Owner

In my local environment with PHP 8.1.13, it works without error.

There is a possibility that the data stored in the DB of the plugin may have gone wrong, so please try to delete the plugin after backing up the plugin settings. Doing so will erase the plugin settings from the DB. Then try installing the plugin again.

As for the proposed patch, it will work, but you will have a situation where the data stored in the DB will be different from the unpatched one.

Of course, I think it would be necessary to handle errors if non-string data is included when unserializing. I'll see what I can do about that.

@kimipooh
Copy link
Owner

Please try adding the following code to the line after the code "$mime_type_values = array();" on line 25 below to see if the error disappears.

if($settings === false) $settings = array();

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 this pull request may close these issues.

2 participants