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

Fixed failure on updating database not supporting UTF8MB4 #8472

Closed
wants to merge 38 commits into from

Conversation

roland-d
Copy link
Contributor

Problem

Users running a MySQL database that does not support UTF8MB4 will get failures during an update to Joomla 3.5.0 because the queries to update the tables to UTF8MB4 are executed.

Fix

The fix is to modify the SQL queries that are run on databases not supporting UTF8MB4.

Testing

  1. Setup a Joomla 3.4.5 site with a MySQL database version that does not support UTF8MB4. For example mysqlnd lower than 5.0.9 or libmysql lower than 5.5.3.
  2. Download the Joomla 3.5.0 beta 1 package
  3. Change the files in the package with the files in this pull request
  4. Update the Joomla version to 3.5.0 beta 1
  5. The update should run without problems

Disclaimer

You may still see database problems on the database tab due to different field definitions. This is not handled in this fix but will be dealt with in another fix.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 3beecb4

Works like a charm!
Updated from 3.4.5 (copy of live site) to 3.5.0 beta 1 with a patched update container on a MySQL with
dbversion: 5.1.73-log
dbcollation: utf8_general_ci
dbconnectioncollation: utf8_general_ci
where it did not work before.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

P.S.: When I tested, there were no database problems detected on the database tab after the update had finished, and all 3 new plugins were installed (which also was not the case before and shows that now everything completed).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

P.P.S: Just to be on the safer side I just tested if something is broken for MySQL databases which DO support itf8mb4 (MySQL 5.5.46 with collations utf8mb4_general_ci in my case), and all is fine, also there no problems and nothing detected by the database tab.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

P.P.P.S: Now I am confused: When I did my test before for the MySQL 5.5. database which supports utf8mb4, I did install the update package from a folder. There were no problems shown on the database tab, but now I noticed why: It seems that the collations where not changed for utf8mb4 in the datavase (I checked the alias column of the menu table, which still was utf8_bin).

But when I install from the same starting point using the Upload package way, then I get problems shown in the database tab, which then can be fixed with the Fix button, then one last problem is shown, which also can be fixed with the button, and then database tab shows all OK. In the database the collation changes have been applied (menu table, alias column has utf8mb4_bin).

So either I made a big mistake when testing or there is a difference between "Upload&Install" and "Install from Folder" methods regarding executing the SQL statements from the SQL files.

@roland-d Do you have any idea? Should I alter my test result? or would it be another issue if I am right and there is this difference?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

switch ($format)
{
case 'mysql':
case 'pdomysql':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did pdomysql has this function?

Else i think we should add a $db->getClientVersion that retruns this value.

What is the difference between the server version ($db->getVersion) and mysqli_get_client_info();?

For sure this don't work for pdomysql if the function mysql_get_client_info(); is not available

@zero-24
Copy link
Contributor

zero-24 commented Nov 18, 2015

For ref this download here contains the current fixes by @roland-d So if you have a install with old mysql pleaase try this.

edit: updated the URL.

@richard67
Copy link
Member

@zero-24 @roland-d I just have checked the container and found a mismatch between paths in script.php and the container.

In administrator/components/com_admin/script.php line 1593:

$fileName = JPATH_ADMINISTRATOR . "/components/com_admin/sql/ut8mb4/$format/3.5.0-2015-07-01.sql";

In file administrator/components/com_installer/models/database.php, line 273:

$fileName = JPATH_ADMINISTRATOR . "/components/com_admin/sql/utf8mb4/$serverType/3.5.0-2015-07-01.sql";

And in the zip container you linked above: administrator\components\com_admin\sql\uft8mb4

3 places, 3 variants.

I suggest to use the one from administrator/components/com_installer/models/database.php ;-)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.


if ($format == 'mysql' && version_compare(JVERSION, '3.5.0', 'lt'))
{
$fileName = JPATH_ADMINISTRATOR . "/components/com_admin/sql/ut8mb4/$format/3.5.0-2015-07-01.sql";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be utf8mb4 and not ut8mb4 in the path, i.e. the "f" is missing here.

@richard67
Copy link
Member

@roland-d Sent a PR to your branch with correction of script.php as mentioned in my comment above.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

@zero-24 Please pack the container again with the correct folder utf8mb4 as soon as Roland has merged my PR into his repository, see my comment above.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@ghost
Copy link

ghost commented Nov 18, 2015

tested this item ✅ successfully. No more Message of Failure about UTF8MB4.

@richard67
Copy link
Member

I have tested this item 🔴 unsuccessfully on 3beecb4

I have to alter my test result so this does not get RTC until the wrong path in script.php is corrected (see my comments and PR before).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@zero-24
Copy link
Contributor

zero-24 commented Nov 18, 2015

Thanks i will check it soon 👍

@roland-d
Copy link
Contributor Author

Thanks for testing guys, I have updated the PR with the correct path and removed the pdomysql check because the UTF8MB4 doesn't apply there.

@roland-d
Copy link
Contributor Author

@richard67 I have done the install from folder test and the upload test and all the results are good. What you are seeing I think is cached data, at least that is what happened to me. I do believe the current fixes are good.

@richard67
Copy link
Member

@roland-d Am still testing, too. I do not think all is cached data. But the current fixes are good anyway.

@richard67
Copy link
Member

@roland-d Just as a reminder: I do both tests for MySQL with and without support for utf8mb4, and with the first one I had the problems. I know this PR here deals with the second one, but it should not break things for the first one, that's why I test both.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@roland-d
Copy link
Contributor Author

@richard67 Correct, I have tested both as well. Will re-run the tests again later. Now I am dealing with the differences in the tables after an update compared to a clean install.

@zjw
Copy link
Contributor

zjw commented Nov 18, 2015

I had also found that 3beecb4 "worked", but only because the typo resulted in it doing nothing. I fix the typo, and I get errors on updating from 3.4.5:

Message

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=-- Convert all tables to UTF-8 Multibyte (utf8mb4) ALTER TABLE `osvlt_assets` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE `osvlt_associations` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE `osvlt_banners` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE `osvlt_banner_clients` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;
...

Reviewing again the config that I'm testing with -- a CentOS 6 system with:

  • PHP 5.4.16
  • mysqlnd 5.0.10 - 20111026 - $Id: e707c415db32080b3752b232487a435ee0372157 $, as returned by mysqli_get_client_info().
  • mysql-server-5.1.73

I find that support for utf8mb4 was added to mysqlnd here: php/php-src@277c7d4, and the version at that time was mysqlnd 5.0.7-dev. The tags on that commit show that it was first included in php 5.4.0. By the time that php 5.4.0 was released, mysqlnd was bumped up to msyqlnd 5.0.10. I would think that should be considered the first stable version released with utf8mb4 support.

The release notes for mysql 5.5.3 confirm that it was the first release supporting utf8mb4.

So in my case, I'm using a mysql client that supports utf8mb4, but a server that does not. The Joomla code as patched here is only looking at my client version with mysqli_get_client_info() and incorrectly concludes that utf8mb4 is supported. The code should also be checking the server version with version_compare($db->getVersion(), '5.5.3', '>='). For me, getVersion() returns 5.1.73.

This server version check should also be added to serverClaimsUtf8mb4Support() in libraries/joomla/database/driver/mysql.php and in libraries/joomla/database/driver/mysqli.php.

As for pdomysql, you should take a look at __construct() in libraries/joomla/database/driver/pdomysql.php. It tests for utf8mb4 support by attempting to connect.

@richard67
Copy link
Member

Here the results of my recent 4 tests, with and without support for utf8mb4 and with upload&install package and install from folder.

Test 1 - MySQL 5.5.46 (supports utf8mb4) - Upload & Install package

Step 1. Instead of installation finished: Internal Server Error

Step 2. Database tab: 4 problems found

Database schema version (3.4.0-2015-02-26) does not match CMS version (3.5.0-2015-11-05).
Database update version (3.4.5) does not match CMS version (3.5.0-beta).
Table 'xxxxx_contentitem_tag_map' should not have index 'idx_tag'. (From file 3.5.0-2015-10-26.sql.)
Table 'xxxxx_contentitem_tag_map' should not have index 'idx_type'. (From file 3.5.0-2015-10-26.sql.)

Other information:

Database schema version (in #__schemas): 3.4.0-2015-02-26.
Update version (in #__extensions): 3.4.5.
Database driver: mysqli.
79 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 3. Fix => 1 problem found

Table 'xxxxx_user_profiles' does not have column 'profile_value' with type 'TEXT'. (From file 3.3.4-2014-08-03.sql.)

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
80 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 4. Fix => Database table structure is up to date.

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
81 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 5. Check plugins: 3 new plugins from 3.5 (editor-button menu, statistics and update notification) NOT installed.

Step 6. Check collation in database (menu table, alias column): utf8mb4_bin, lenght 191 => OK.

Test 2 - MySQL 5.5.46 (supports utf8mb4) - Install from folder

Step 1. Message "Installation of the file was successful."

Step 2. Database tab: 1 problem found

Table 'xxxxx_user_profiles' does not have column 'profile_value' with type 'TEXT'. (From file 3.3.4-2014-08-03.sql.)

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
80 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 3. Fix => Database table structure is up to date.

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
81 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 4. Check plugins: 3 new plugins from 3.5 (editor-button menu, statistics and update notification) installed => OK.

Step 5. Check collation in database (menu table, alias column): utf8mb4_bin, lenght 191 => OK.

Test 3 - MySQL 5.1.73 (does NOT support utf8mb4) - Upload & Install package

Step 1. Like step 1 of test 1

Step 2. Like step 2 of test 1

Step 3. Fix => Database table structure is up to date, i.e. the 1 problem for table xxxxx_user_profiles which required a second fix in test 1 did not appear here.

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
81 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 5. Check plugins: 3 new plugins from 3.5 (editor-button menu, statistics and update notification) NOT installed.

Test 4 - MySQL 5.1.73 (does NOT support utf8mb4) - Install from folder

Step 1. Messages

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=-- Convert all tables to UTF-8 Multibyte (utf8mb4) ALTER TABLE xxxxx_assets CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE xxxxx_associations CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE xxxxx_banners CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

... (lots of similar messages for other tables and columns)

Database query failed (error # 1115): Unknown character set: 'utf8mb4' SQL=ALTER TABLE xxxxx_ucm_content MODIFY core_alias varchar(191) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '';

Installation of the file was successful.

Step 2. Database tab: Database table structure is up to date.

Other information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta.
Database driver: mysqli.
81 database changes were checked successfully.
148 database changes did not alter table structure and were skipped.

Step 3. Check plugins: 3 new plugins from 3.5 (editor-button menu, statistics and update notification) installed => OK.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

@roland-d From my point of view it should be decided by PLT (good that you are a member) before release of 3.5.0 if Joomla! wants to face site owners or admins with data loss and, if so, how far it shall go, apply it like it is with this PR now, i.e. shorten more texts than necessary, or try to optimize that, as @zjw 's comment explains.

If you or PLT decide to apply further changes on this PR (or do it with another one), everything has to be tested again, i.e. both new install and update on both kinds of databases for at least database drivers mysqli and pdomysql if not also mysql (without i), so we have 8 to 16 tests to be done, all with comparison of data structures in sql export files afterwards.

So or so, if it is taken as it is now or if further optimizations as suggested above by @zjw are applied, the release notes of 3.5.0 have to clearly list all table columns where data losses may appear from my point of view.

Releasing this PR as it is now with 3.5.0 and not telling anything about it will very likely cause a lot of complains, because such data losses may not be detected when checking things after upgrade and then a few weeks or months later when they are detected it may be too late to fall back to old data, and forums will be full with support requests.

What can be done in order not to further delay the next beta release is to release the beta with status of this PR and then before the final 3.5.0 work on it further and optimize things, because there is no update path from any beta to the next beta.

But it has to be finally decided before 3.5.0 which way it goes.

I will be offline for the next hours but later I am available for further discussion and testing.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@roland-d
Copy link
Contributor Author

I have put the issue before the PLT so we can decide how to move forward. In my opinion we can't cancel this request and leave sites vulnerable just because sites may have aliases beyond the 192 character limit.

My idea would be to add a check in the preflight to see if there are any aliases longer than 192 and if so we can cancel the installation with a message informing the user. What that message is going to be is to be determined of course.

@mbabker
Copy link
Contributor

mbabker commented Nov 26, 2015

My idea would be to add a check in the preflight to see if there are any aliases longer than 192 and if so we can cancel the installation with a message informing the user.

You can't. The way the core update component works, the update is unpacked in full then the upgrade.finalise task is called which triggers a mocked version of a JInstallerAdapter upgrade path.

@roland-d
Copy link
Contributor Author

@mbabker Thanks for the feedback.

@richard67
Copy link
Member

One way to check and notify could be to release a 3.4.6 which does the check and shows the result in a postinstall message so site admins could react before they upgrade then to 3.5.0, but this would be 1. much additional work maybe and 2. maybe delay the project schedule and 3. only be a theoretical solution since it is not granted every site is going the way from <= 3.4.5 via 3.4.6 to 3.5.0, at least when I see how many questions in the support forum refer to outdated version and even 2.5.x I daubt if this would really help.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@Bakual
Copy link
Contributor

Bakual commented Nov 26, 2015

We could force a step between using the update XML files. There we can target exactly which version of Joomla should an update be shown. So to Joomla <= 3.4.5 only the 3.4.6 would be shown and 3.5.0 to those on 3.4.6.
But it's a bit complex and I actually doubt users will read the postinstall message before they upgrade further to 3.5.0. So it would just create a tedious step for most users without actually being useful in the end.

@richard67
Copy link
Member

Yes, I have the same daubt, but I thought I should mention it here for discussion.

@zjw
Copy link
Contributor

zjw commented Nov 28, 2015

I have prepared another PR: roland-d#7. It addresses the issues that I brought up earlier.

Downsizing of columns to varchar(191) has been eliminated, except for #__session.session_id and #__user_keys.series, where it does no harm. The size of a session_id is dependent upon the PHP configuration (session.hash_function and session.hash_bits_per_character), but the worst-case scenario is a 512-bit hash (sha512 or whirlpool) and an encoding of 4 bits per character, resulting in a 128 character session_id, so there is no danger of data loss there. The series column contains a 20 character key generated in PlgAuthenticationCookie::onUserAfterLogin(), so there is no danger of data loss there, either.

I had mentioned that there was a particular problem with reducing key sizes to 191 characters in cases where those keys were UNIQUE. The solution, in my opinion, is to not declare those keys UNIQUE. For the most part, the Joomla code is already performing adequate checks for duplicate entries before inserts and updates for the 2 columns at issue (#__menu.alias and #__redirect_links.old_url). I have improved that code some so that duplicates should not occur.

{
$db = JFactory::getDbo();

$utf8mb4IsSupported = $this->serverClaimsUtf8mb4Support($db->name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we don't do $db->hasUTF8mb4Support() here instead?

@zjw
Copy link
Contributor

zjw commented Dec 6, 2015

@wilsonge: The problem is when updating from 3.4.5 or before. In those cases, $db->hasUTF8mb4Support() doesn't exist. Consequently, everything needed for conversion must be contained in script.php, even though that seems like a duplication of code.

@mbabker
Copy link
Contributor

mbabker commented Dec 6, 2015

The script file is not triggered until the filesystem has been updated to 3.5 unless you are updating through the Extension Manager.

@zjw
Copy link
Contributor

zjw commented Dec 6, 2015

@mbabker: Yes, but the old (3.4.5) database classes have already been loaded before the script is triggered, so I don't think updating the class files makes any difference. The old classes are still the ones in use.

@mbabker
Copy link
Contributor

mbabker commented Dec 6, 2015

The <3.5.0 code is ONLY used if you are upgrading via the Extension Manager. Because of the use of AJAX in the update component, the package is extracted in full in several smaller steps then a request to the update component's update.finalise task is made and this is the point where the install script and database update queries are executed. This request is 100% in the context of a 3.5.0 filesystem.

@zjw
Copy link
Contributor

zjw commented Dec 6, 2015

Ah -- that makes sense.

I suspect that people performing update tests of this PR are doing so via the extension manager. That method seems like it needs to be supported.

@richard67
Copy link
Member

@roland-d @zjw and others involved into this PR:

I have added xml files for using the patched update container (links see above somewhere) to the donwload location, so the container can be used for update via Joomla! Update Component, i.e. it is not necessary to use the Extension Installer anymore for the update tests.

To do so, select "Custom URL" as update channel in the Joomla! Update Component's options, and enter "http://test5.richard-fath.de/list_test.xml" as URL:

screen shot 2015-12-09 at 06 48 50

After "Save & Close" you should see then:

screen shot 2015-12-09 at 06 49 52

The update can be started then in the usual way.

This method changes the URL in update sites table. To come back to the standard, change the update channel back to what it was before (e.g. Testing).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

P.S.: Of course I have tested again with the latest container how I described in my previous comment, and it was successful.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@richard67
Copy link
Member

@roland-d @zjw and others involved into this PR:

I have created new full installation and update containers in case if testing this PR goes on somehow, e.g. with regular Joomla! Update method (i.e. not Extension Installer) or as a base for new containers after any further commits.

The containers patched by this PR#s changes are based on latest staging, i.e. they also include the security fixes done with 3.4.6.

For update tests with Joomla! Update, use "http://test5.richard-fath.de/list_test.xml" as custom update URL as I described in my comment above.

For full installation tests or update tests with the Extension Installer (from folder or upload & install package) use following links:

Joomla_3.5.0-beta-Beta-Full_Package_utf8mb4_fix9.zip

Joomla_3.5.0-beta-Beta-Update_Package_utf8mb4_fix9.zip

Note the changed package name "..._fix9.zip" in case you used "Install from URL" for update tests using the Extension Manager. The links of the old packages "..._fix8.zip" will produce a 410 "gone" now.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8472.

@swiffer
Copy link

swiffer commented Dec 20, 2015

at least for me updating worked great from joomla 3.4.5 to 3.5 fix 9 on a host where utf8mb4 is not available

-- update

receive the following error when applying an old database (utf8) to a already patched 3.5.0 fix9 and doing database repairs on an utf8mb4 compatible server afterwards

(Fehler # 1071)!: Specified key was too long; max key length is 767 bytes SQL=ALTER TABLE #__redirect_links CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

All other alter table commands went fine!

@roland-d
Copy link
Contributor Author

roland-d commented Jan 4, 2016

Just to give an update, I have not forgotten about this but there are a lot more things to be considered/checked. I will be back soon with an update on how we will proceed.

@ghost
Copy link

ghost commented Jan 4, 2016

thx

On 04.01.2016, at 18:09, RolandD notifications@github.com wrote:

Just to give an update, I have not forgotten about this but there are a lot more things to be considered/checked. I will be back soon with an update on how we will proceed.


Reply to this email directly or view it on GitHub.

@richard67
Copy link
Member

@roland-d Any news? Meanwhile Beta 2 is out and this PR here was claimed to be the show stopper for 3.5, so I have expected this PR here going into the Beta 2.

@brianteeman
Copy link
Contributor

Please see the release notes
On 28 Jan 2016 11:14 am, "Richard Fath" notifications@github.com wrote:

@roland-d https://github.com/roland-d Any news? Meanwhile Beta 2 is out
and this PR here was claimed to be the show stopper for 3.5, so I have
expected this PR here going into the Beta 2.


Reply to this email directly or view it on GitHub
#8472 (comment).

@roland-d
Copy link
Contributor Author

@richard67 The status is that there will be a number of new PRs that will superseed this PR. We have been working behind the scenes to get this issue really cleared up. Once the PRs are ready to be tested, I will link them from here.

You are correct, this is a showstopper for 3.5 and will be fixed before the stable is released.

@wilsonge
Copy link
Contributor

Closing this based on us merging #9045 as this is now dated. We can come back to this if it doesn't work out in beta 3

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.