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

Allow chinese characters in saved_user_auth_info db table #701

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

rmiccoli
Copy link
Contributor

@rmiccoli rmiccoli commented Jan 24, 2024

in particular, character set of the info_val column has been changed from latin1 to utf8.

This PR solves issue #700.

The error occurs when using MySQL 5.7 since:

  • MySQL 5.7 has the default charset latin1 and default collation latin1_swedish_ci
  • MySQL 8.0 has the default charset utf8mb4 and default collation utf8mb4_0900_ai_ci (which also accepts Chinese characters)

References:

of info_val column in saved_user_auth_info table from latin1
to utf8 in order to accept also chinese characters
@rmiccoli rmiccoli changed the title Allow chinese characters in saved_user_auth_info table Allow chinese characters in saved_user_auth_info db table Jan 24, 2024
@enricovianello
Copy link
Member

Could you add a URL reference for the default MySQL charsets you mentioned? Thanks :)

Copy link
Member

@enricovianello enricovianello left a comment

Choose a reason for hiding this comment

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

Just a couple of clarifications

@@ -0,0 +1 @@
ALTER TABLE saved_user_auth_info MODIFY info_val varchar(256) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
Copy link
Member

Choose a reason for hiding this comment

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

Has this query some consequences on already present records?

Copy link
Contributor Author

@rmiccoli rmiccoli Jan 25, 2024

Choose a reason for hiding this comment

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

AFAIK, there should be no problem because all characters from latin1_swedish_ci are present in utf8mb4_unicode_ci. There might be problems with conversion if there were charset columns defined as foreign keys, but this is not the case.

I tried this query on our development instance (iam-dev) having that table already populated, and it succeeded.

@rmiccoli
Copy link
Contributor Author

rmiccoli commented Jan 25, 2024

Could you add a URL reference for the default MySQL charsets you mentioned? Thanks :)

URLs added in the PR description! :)

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@@ -0,0 +1 @@
ALTER TABLE saved_user_auth_info MODIFY info_val varchar(256) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
Copy link
Contributor

Choose a reason for hiding this comment

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

case-insensitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I left it case-insensitive as before. Isn't it good?

Copy link
Contributor

Choose a reason for hiding this comment

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

It depends on what that field represents

Copy link
Contributor Author

@rmiccoli rmiccoli Jan 25, 2024

Choose a reason for hiding this comment

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

It contains information on user authentication through a SAML IdP (e.g. organization name, UID, given name, mail, etc.) coming from the SAML assertion

@rmiccoli rmiccoli linked an issue Mar 8, 2024 that may be closed by this pull request
@enricovianello enricovianello merged commit 21845a8 into develop Mar 11, 2024
5 checks passed
@enricovianello enricovianello deleted the issue-700 branch March 11, 2024 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Java SQL incorrect string value
3 participants