XOOPS / XoopsCore25

XOOPS Core 2.5.x (current release is 2.5.11: https://github.com/XOOPS/XoopsCore25/releases)
GNU General Public License v2.0
71 stars 59 forks source link

Xoops 2.5.11 Going to Administration Menu -> Avatars throws error #1463

Closed prbt2016 closed 6 months ago

prbt2016 commented 6 months ago

Hello ,

I was in the process of manual installation of Xoops 2.5.11 on Centos 7 with Apache 2.2 , the following PHP'S 5.6 , 7.0 , 7.1, 7.2, 7.3 , 7.4, MYSQL 5.5.

I checked that installer shows PHP 5.6.0 or above required .

Also checked official docs https://xoops.gitbook.io/xoops-install-upgrade/installation/requirements where PHP 5.3.9 is mentioned as min PHP and 7.1 is mentioned as recommended PHP.

However after a successful install , when I visit Administration Menu -> Avatars , following error is thrown on PHP 5.6- 7.3.

On PHP 5.6 I see no error printed , the output on {{URL}}/modules/system/admin.php?fct=avatars is blank.

On PHP 7.0 to 7.3 the following error is thrown i.e :

A problem has occurred on our server! Page is currently unavailable

We are working on a fix Please come back soon ...

Error : ParseError: syntax error, unexpected 'string' (T_STRING), expecting variable (T_VARIABLE)

Screenshot as below :

image

For your kind information, Avatar functionality works correctly only with PHP 7.4 .

Are the requirements changed?. If yes, what is the minimum PHP requirement for Xoops?.

Also it would be great if that's updated on offical docs here https://xoops.gitbook.io/xoops-install-upgrade/installation/requirements

So that users won't face any issues.

Regards.

mambax7 commented 6 months ago

Thank you for reporting the issue. I'll test it and get back to you. In the meantime, I've changed the PHP info in the documentation, bumping it up to PHP 7.4+, so we can avoid any potential issues, while I test your case: https://github.com/XoopsDocs/xoops-install-upgrade/blob/master/installation/requirements.md

mambax7 commented 6 months ago

OK, I found the issue. In the file \modules\system\class\avatar.php, we had:

public string $className = '';

which should be:

public $className = '';

since typed properties are allowed only since PHP 7.4

Thank you again for letting us know about this issue, and if you find any other inconsistencies, please let us know

prbt2016 commented 6 months ago

Hello @mambax7 ,

I checked the fix https://github.com/XOOPS/XoopsCore25/commit/cecdcef87b8ddf9c2d713b7645d892bc87962147 and it works with PHP 5.6 - 7.4 every version.

In the meantime, I've changed the PHP info in the documentation, bumping it up to PHP 7.4+, so we can avoid any potential issues, while I test your case: https://github.com/XoopsDocs/xoops-install-upgrade/blob/master/installation/requirements.md

Are you going to revert PHP requirements here https://github.com/XoopsDocs/xoops-install-upgrade/blob/master/installation/requirements.md and here https://xoops.gitbook.io/xoops-install-upgrade/installation/requirements back to PHP 5.6 or keep it PHP 7.4 ?.

Since for PHP 7.4 that fix for type property isn't needed.

Kindly let me know .

Regards.

mambax7 commented 6 months ago

Thank you so much for testing and letting us know that it works.

Based on the 2024 PHP usage, and because the older PHP versions are not being maintained anymore and no fixes are provided, I'll leave it as is. This will hopefully encourage people to move to newer (and safer) versions of PHP.

FYI - the next major release of XOOPS (probably called 2.7.0) will require PHP 8.2 as a minimum. But till then, we'll maintain 2.5.11 and keep releasing fixes, if needed.

prbt2016 commented 6 months ago

Hello @mambax7 ,

Thanks for your advice . Shall use 7.4.

However, I have one doubt .

While fresh install of Xoops 2.5.11 , I observed under phpmyadmin that the default collation is utf8mb4_general_ci for all the tables.

However , on one of my server, I have one old install of Xoops 2.5.10 and I was trying to upgrade it to 2.5.11 . The upgrade was successful.

However when I check the collate post upgrade to 2.5.11 , the collate still remains utf8_general_ci.

Is this fine or is it an issue?.

prbt2016 commented 5 months ago

Any update on the query which I have asked regarding upgrade. Kindly let me know.

mambax7 commented 5 months ago

Sorry for the delay - I didn't see it earlier. Leaving the collation as utf8_general_ci post-upgrade is not inherently problematic, and XOOPS should functions correctly. However, in the future, updating to utf8mb4_general_ci can provide better character support, like emojis

-utf8mb4_general_ci: Supports a larger set of characters, including emojis and other symbols, using 4 bytes per character. It's generally recommended for modern applications due to its comprehensive character support. - utf8_general_ci: Uses 3 bytes per character and does not support certain characters that utf8mb4 does. It's an older collation that might have been used in the past.

If you want to update the collation, you can do it manually in PhpMyAdmin, or you can run a SQL query in PhpMyAdmin (after you do a backup!), something like this:

  1. Access SQL Tab:

    • In phpMyAdmin, select the database.
    • Click on the "SQL" tab to open the query editor.
  2. Run SQL Queries to Change Collation:

    • Change Database Collation:
      ALTER DATABASE `your_database_name` CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;
    • Change Tables Collation: For each table, run:
      
      ALTER TABLE `your_table_name` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;
prbt2016 commented 5 months ago

Hello @mambax7 ,

Thanks for your kind reply. But that would be a tedious task to do all that manually. Is there any way by which you could implement the changes in code so that this collate conversion be done automatically from utf8 to utf8mb4_general_ci. That would be really great. Is that possible ?.

Kindly let me know.

mambax7 commented 5 months ago

Try this script: https://github.com/montuy337513/convert_mysql It should do the job for you