Skip to content

Fix typo ENABLE_UTF8MB -> ENABLE_UTF8MB4#1125

Merged
taniwallach merged 1 commit into
openwebwork:developfrom
mikeshulman:utf8mb4-typo
Oct 7, 2020
Merged

Fix typo ENABLE_UTF8MB -> ENABLE_UTF8MB4#1125
taniwallach merged 1 commit into
openwebwork:developfrom
mikeshulman:utf8mb4-typo

Conversation

@mikeshulman
Copy link
Copy Markdown
Contributor

See discussion at mikeshulman#1.

@drgrice1 drgrice1 requested review from mgage and taniwallach August 28, 2020 01:32
Copy link
Copy Markdown
Member

@taniwallach taniwallach left a comment

Choose a reason for hiding this comment

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

I think both changes are correct.

The fix is needed as the code is currently inconsistent, but it seems that many systems would not notice the bug. I suspect that systems where the database is not configured to default to utf8mb4 for the connection may have issues at present.

I certainly do not encounter any issues on a Docker setup before the patch, and I could put a 4-byte UTF-8 character into a students name.

Why?

  1. My database is configured directly to use utf8mb4 for the connection charset, etc. using docker-config/db/mariadb.cnf so the bug causing a failure to call SET NAMES 'utf8mb4'; is not causing trouble.
  2. It seems that Perl's DBD when when mysql_enable_utf8 is used (instead of mysql_enable_utf8mb4) does not really make any effort to prevent 4-byte UTF-8 characters from being passed between Perl and the database. Instead, to documentation instructs users of the limited 3-byte mySQL utf8 charset that such checks are to be added by the people writing code which uses the restricted (somewhat broken) utf8 character set.

When the C charset is used then you are responsible for 3-byte UTF-8 sequence checks on input perl scalar strings. Otherwise MySQL server can reject or modify the input statement!

@taniwallach taniwallach merged commit edf75ca into openwebwork:develop Oct 7, 2020
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