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

Change usages of ToLower into ToLowerInvariant #33

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

ersagunkuruca
Copy link
Contributor

This fixes a very hard-to-spot bug where Web3Auth & OpenLogin started to give various configuration errors on some specific devices. Here is where we reported it: https://web3auth.io/community/t/invalid-constructor-params-invalid-environment-settings/6643

It turns out all those devices were Turkish, and the character "ı" (or "dotless i" as non-Turkish people would say) was the reason.

You see, in Turkish, lowercasing/uppercasing rules are different, because we have a distinction between "I" and "İ" where "I".ToLower() == "ı" and "İ".ToLower() == "i" which makes sense in Turkish, and "I".ToLower() == "i" makes sense for any other language, but obviously these two rules don't mix well. And ToLower() uses the current culture information of the device. Which can make network.ToString().ToLower() into "maınnet" and buildEnv.ToString() into "productıon" "stagıng" or "testıng".

To sum up: ToLower uses current device's region settings for lowercasing a string, which can be different for some Turkic languages. ToLowerInvariant is what should be used where this behavior doesn't make sense.

This fixes a very hard-to-spot bug where Web3Auth & OpenLogin started to give various configuration errors on some specific devices. Here is where we reported it:
https://web3auth.io/community/t/invalid-constructor-params-invalid-environment-settings/6643

It turns out all those devices were Turkish, and the character "ı" (or "dotless i" as non-Turkish people would say) was the reason. 

You see, in Turkish, lowercasing/uppercasing rules are different, because we have a distinction between "I" and "İ" where "I".ToLower() == "ı" and "İ".ToLower() == "i" which makes sense in Turkish, and "I".ToLower() == "i" makes sense for any other language, but obviously these two rules don't mix well. And ToLower() uses the current culture information of the device. Which can make network.ToString().ToLower() into "maınnet" and buildEnv.ToString() into "productıon" "stagıng" or "testıng".

To sum up: ToLower uses current device's region settings for lowercasing a string, which can be different for some Turkic languages. ToLowerInvariant is what should be used where this behavior doesn't make sense.
@chaitanyapotti
Copy link
Member

Thanks for this PR

Copy link
Member

@chaitanyapotti chaitanyapotti left a comment

Choose a reason for hiding this comment

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

lgtm

@chaitanyapotti chaitanyapotti merged commit 5f89d13 into Web3Auth:master Jan 12, 2024
@ersagunkuruca
Copy link
Contributor Author

Thanks for the quick approval. Also sorry about the sloppy commit, it's because I edited the file on Github, my bad.

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.

None yet

2 participants