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

Use utf8mb4 for the connection etc #1001

Merged
merged 3 commits into from Apr 6, 2024
Merged

Conversation

bramley
Copy link
Contributor

@bramley bramley commented Nov 5, 2023

Description

This change sets the character set used for the client-server exchanges to utf8mb4 instead of the current utf8, which is an alias for utf8mb3.

Using utf8mb4 will allow a wider range of characters, supplementary characters, to be sent, specifically emoji characters. If once-off database table changes are made then subject lines can include emoji characters.

This change is backward compatible, it has no effect when supplementary characters are not used.

Related Issue

#1000

Screenshots (if appropriate):

@michield
Copy link
Member

michield commented Nov 5, 2023

Does this affect the version of Mysql/Mariadb that is a minimum requirement?

@michield michield mentioned this pull request Nov 5, 2023
@bramley
Copy link
Contributor Author

bramley commented Nov 5, 2023

It seems to have been introduced in MySQL 5.5 from 2010
https://dev.mysql.com/blog-archive/mysql-8-0-when-to-use-utf8mb3-over-utf8mb4/
and specifically in 5.5.3 from this PDF file of release notes

The utf8mb4 character set has been added. This is similar to utf8, but its encoding allows up to four
bytes per character to enable support for supplementary characters

@michield
Copy link
Member

It needs to change here

as well/ In fact, "initialise" should use the Sql_Create_Table from mysqli.inc but we can fix that some other time

After that, I was able to put mb4 characters in the subject and content and it worked

@michield michield self-requested a review December 29, 2023 17:43
Copy link
Member

@michield michield left a comment

Choose a reason for hiding this comment

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

If you can edit those two lines as well. Maybe it should be a config option at some point

@michield
Copy link
Member

additionally, we may want to add something to the upgrade script to convert all tables to utf8mb4, although we can also do it as a seperate step like we did before https://github.com/michield/phplist3/blob/master/public_html/lists/admin/converttoutf8.php

@bramley
Copy link
Contributor Author

bramley commented Dec 30, 2023

These suggestions are going beyond the original scope of the change, which was simply to avoid undoing a manual change made to a phplist file when upgrading. The conversion of an existing database to utf8mb4 would have been made manually.

I don't know how "safe" it would be to try to automate the conversion to utf8mb4 and any reasons why that might fail. It seems safer to leave that to each installation to do so that it is entirely under their control.

Using utf8mb4 for new installations seems sensible though but might need more investigation. This mysql documentation page lists some possible consequences

https://dev.mysql.com/doc/refman/8.0/en/charset-unicode-conversion.html

@michield
Copy link
Member

Ok, let's leave the scope for this small, and we can tackle conversion and other mb4 related work in a different PR.

@bramley
Copy link
Contributor Author

bramley commented Feb 10, 2024

I have been looking at this recently because a system I support had a problem with a campaign body including a 4-byte UTF-8 character, so I applied the changes manually.

Looking at the conversion to utf8 page, and the same code in the upgrade page, then trying to use that to upgrade to utf8mb4 showed an issue with the SQL used to change each column

            Sql_Query(sprintf('alter table %s change column %s %s %s default character set utf8',

This syntax requires to repeat the full column definition. Any columns that have been defined as not null or with a default value are losing those on this upgrade. Further, the upgrade is applied on the first upgrade of a new installation, even though that is already utf8. So it looks like some rework is needed.

What is simple and probably quite safe is to upgrade only the columns that are used for a campaign where an admin is likely to want to include a 4-byte UTF-8 character. These are the subject, message and textmessage fields on the messages table, and the data field on the message_data table.

@bramley
Copy link
Contributor Author

bramley commented Feb 11, 2024

I have added another commit that sets the character set for the message subject and content. These seem to be the fields to which people want to add 4-byte UTF-8 characters such as emojis.

I have tested this change by creating a new database, and upgrading an existing one. In both cases I could copy/paste 😀 into the campaign content, save the campaign, then send a test email successfully.

@marianaballa marianaballa changed the base branch from main to release-3.6.15 March 27, 2024 15:45
@michield michield merged commit 5c3a4ba into phpList:release-3.6.15 Apr 6, 2024
4 of 5 checks passed
@phpListDockerBot
Copy link
Contributor

This pull request has been mentioned on phpList Discuss. There might be relevant details there:

https://discuss.phplist.org/t/3-6-15-release-candidate-is-available-for-testing/9473/1

@phpListDockerBot
Copy link
Contributor

This pull request has been mentioned on phpList Discuss. There might be relevant details there:

https://discuss.phplist.org/t/phplist-3-6-15-has-been-released/9495/1

marianaballa added a commit that referenced this pull request Apr 26, 2024
* Translations for 3.6.15  (#1032)

* Translated using Weblate (English)

Currently translated at 91.4% (1950 of 2132 strings)

Translation: phpList/phpList3
Translate-URL: http://translate.phplist.org/projects/phplist/phplist3/en/

* Translated using Weblate (French)

Currently translated at 99.8% (2128 of 2132 strings)

Translation: phpList/phpList3
Translate-URL: http://translate.phplist.org/projects/phplist/phplist3/fr/

---------

Co-authored-by: Duncan Cameron <phplist@dcameron.me.uk>
Co-authored-by: Alain Rihs <alainrihs@sunrise.ch>

* Support for indicating and getting feedback for e-mail test messages (#1031)

* Update sendemaillib.php

1. Appended a test subject indicator to test messages
1. Added a reply-to address to test messages that have no manual reply-to: using the logged in admin's address or at least the general admin's

* Update sendemaillib.php

Rephrased variable name

* Update sendemaillib.php

Switched to using $admin_auth

* Allowing subscribers to be filtered by confirmed and/or blacklisted (#1030)

* Update users.php

Allowed to filter by confirmed and/or non blacklisted - and not just by unconfirmed and/or blacklisted

* Changed users to subscribers

* Bouncemgt - allowing processing only existing bounces + a related new rule action (#1028)

* Update bouncemgt.php

Added &justexisting=true

* Update processbounces.php

1. Added support for &justexisting=true
1. Added support for new bounce rule action

* Update lib.php

Added support for new bounce action

* Update bouncemgt.php

Added non default title (otherwise it takes the wrong one)

* Update processbounces.php

1. Replaced goto with if-else
1. Hardcoded "-1" instead of supplying it in a sprintf value

* Hardcoding defaults for older PHP versions

* Removed modern solution

* Update Common plugin and Segment plugin (#1024)

* Define timestamp columns explicitly (#1019)

* Define timestamp fields explicitly to avoid problem with the mysql setting explicit_defaults_for_timestamp

* Remove setting of timestamp fields that are automatically updated

* update CI to remove old PHP versions and add 8.3 (#1004)

* Escape single quote in error message (#1003)

* Allow ajax page links to have a title, defaulting to the link description (#1002)

Fixes #996

* Update CONTRIBUTING.md (#994)

Removed obsolete references

* update UUID class to the latest upstream (#990)

* update UUID class to the latest upstream

* clean up old files

* use the list order, even when grouping by category (#1025)

* restore ability to create other super users (#1014)

* restore ability to create other super users

* correctly initialise the privileges array

* Bounces' subscriber' status indicator + allowing to confirm right from bounces (#1029)

* Update listbounces.php

Added support for confirmed/blacklisted indicator

* Update bounces.php

Added confirmed/blacklisted indicator

* Update bounce.php

1. Added confirmed/blacklisted indicator
1. Added support for confirming user from a bounce

* Update bounce.php

1. Avoided ternary if because translation system doesn't support it
1. Used the newer s() function

* Update listbounces.php

Added curly brackets

* Used potential translation

* Php8fixes 202401 (#1026)

* remove deprecated ini_set call

* stop possible warning

* avoid warning

* avoid warning

* cast to int

* avoid warning on existing being null

* force template to be an integer

* suppress warnings

* check on valid var and cast to int

* give buttons an ID, so they can be targetted with testing

* avoid warning on empty array index

* add notification by email when an admin logs in from a new IP address. (#1027)

* add notification by email when an admin logs in from a new IP address.

* check IP per admin

* force columns to be not null

* prevent blocking login on an non-upgraded system and send login alert just to admin, or superuser

* keep newlines in translation as they are

* make shorter lines, so it renders a bit better

* Remove redundant upgrade steps (#1020)

* Remove steps that are unnecessary due to the 3.2.0 being the minimum upgrade version

* Keep silent when there are no subscriber UUIDs to generate

* Remove other unnecessary upgrade steps

---------

Co-authored-by: Michiel Dethmers <michiel@phplist.com>

* Use utf8mb4 for the connection etc (#1001)

* Use utf8mb4 for the connection etc

* Support utf8mb4 in campaign subject and content

---------

Co-authored-by: Michiel Dethmers <michiel@phplist.com>

* use PHP8.2 to build

* use latest phplint

* update docker build from bookworm

* set version

* avoid the admin being kicked out after upgrade (#1033)

* mark update translations as @wip

---------

Co-authored-by: Duncan Cameron <phplist@dcameron.me.uk>
Co-authored-by: Alain Rihs <alainrihs@sunrise.ch>
Co-authored-by: lwcorp <lwcorp@users.noreply.github.com>
Co-authored-by: Duncan Cameron <3147688+bramley@users.noreply.github.com>
Co-authored-by: Michiel Dethmers <michiel@phplist.com>
@bramley bramley deleted the utf8mb4 branch May 4, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants