vt-middleware / passay

Password policy enforcement for Java.
http://www.passay.org
Other
275 stars 63 forks source link

Some_refactoring_and_adding_archaic_cyrillic_language #132

Closed Jaime-Pinkman closed 2 years ago

Jaime-Pinkman commented 2 years ago

Hello everyone! I am using this lib in my projects, and we allow to our customers to use in password Cyrillic symbols. And we noticed, that our system gets some errors if customer use some symbols (like ѣ and others), which are Cyrillic, but they used near in ~1700s and don't consisted in modern Cyrillic languages. I moved it to ArchaicCyrillicSequenceData and ArchaicCyrillicCharacterData and created new Cyrillic ones without these letters. Also I grouped classes into packages for better reading the code. Please comment and review. Thanks!

dfish3r commented 2 years ago

Hello @Jaime-Pinkman, thanks for the patch. Unfortunately this restructuring is a breaking API change. While we may consider something like this for the next major version, we don't have a major version planned right now. Can you provide a PR for only the Cyrillic changes?

Jaime-Pinkman commented 2 years ago

Hello @Jaime-Pinkman, thanks for the patch. Unfortunately this restructuring is a breaking API change. While we may consider something like this for the next major version, we don't have a major version planned right now. Can you provide a PR for only the Cyrillic changes?

Hi, I reverted all my changes except the Cyrillic ones. I spoke with my colleagues, and we decided not to change original CyrillicSequenceData and CharacterData. Instead of it is better solution to create New classes without these archaic symbols for backward compatibility. These classes has New... prefix in their names.

dfish3r commented 2 years ago

@hiteule do you have any feedback on whether these characters іѣѳѵ and ІѢѲѴ are still applicable to the modern Cyrillic alphabet?

dfish3r commented 2 years ago

I'm going to merge this, but rename NewCyrillicCharacterData to CyrillicModernCharacterData. Thank you for the PR.