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

Replace Cyrillic c with c in rest of caCert variables #2492

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

tszucs-stripe
Copy link
Contributor

@tszucs-stripe tszucs-stripe commented Feb 11, 2022

Summary

Following along #2397, there were still a couple more Cyrillic letters checked-in, likely from copy-pasting the config.

Motivation

To ensure caCert is set from the config.

Testing

Verified presence of the letters with rg "[^\x00-\x7F]", changed them to c, and then verified again with ripgrep that the letters were no longer present.

Risks

Should be low-risk, worst case perhaps someone will hit an error now that their caCert file is set correctly.

Hotfix?

Potentially not, as this is an sqlite development config which if I understand correctly is a feature still in development. Will defer to Temporal for this.

@tszucs-stripe tszucs-stripe requested a review from a team as a code owner February 11, 2022 17:40
@CLAassistant
Copy link

CLAassistant commented Feb 11, 2022

CLA assistant check
All committers have signed the CLA.

@wxing1292 wxing1292 merged commit 855ef8e into temporalio:master Feb 11, 2022
@alexshtin
Copy link
Member

Too many Russians in the team, but source of this one is actually external contribution.

@alexshtin
Copy link
Member

Happily this is the only one case but I think we should add a linter/validator to check entire code base for anything but standard ASCII symbols.

@tszucs-stripe
Copy link
Contributor Author

Happily this is the only one case but I think we should add a linter/validator to check entire code base for anything but standard ASCII symbols.

@alexshtin that sounds like a great idea, especially since there are a diverse set of contributors to the repo, this looks like it is an easy mistake to make if you are using a non-English keyboard. I've seen other GitHub issues on OSS repos punt on these kind of linters with concern that it pushes everyone to only use English. I think from the security/reliability prospective it can make sense though, given the complexity of unicode.

Another idea is to restrict homoglyphs / similar looking unicode characters and bidirectional unicode characters. Rust has a good example of linters to do this (the Go ones I have seen aren't as comprehensive as these in coverage):

This would keep everything unicode friendly but protect from these kind of bugs.

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.

5 participants