dwolfhub
All PRs merged and fixing all issues
Merges #57 #62 #63 fixing conflicts and a few bugs introduced within. Additionally fixes the problem with empty password. Thus, all currently open PRs and issues are handled in this.
Fixes #56 Fixes #64
@dwolfhub Are you still maintaining this? We had to publish a fork because no PRs get merged here and those fixes were crucial for our primary project.
https://pypi.org/project/zxcvbn-covert/
@dwolfhub Getting a bit of mixed messages here, since you suddenly did merge one of the PRs but did not write anything here. If you'd prefer, we can take over the maintenance of this package. We do not plan to do any big changes, only bug fixes if any further issues are reported, aiming to closely follow the original Javascript implementation.
One of the PRs merged here bumped the version number to 5.0.0, so if you do release another version, I would suggest bumping your version number higher than that. The bump in major version is justified because support for old Python versions was dropped. If you intend to keep maintaining this, we will not push further versions to PyPi, to avoid further confusion with versions. Our package still installs under zxcvbn module name, so zxcvbn-covert conflicts with your zxcvbn-python.
@covert-encryption thanks the submission. i'd like to avoid one giant PR that includes lots of different changes, including merging multiple PRs into it.. i also have some questions about why you chose to use dictionaries instead of lists in some places and also changing date times to perf counters. it would be a lot easier, i think, if you submitted PRs for individual changes and helped me understand your reasoning.
@covert-encryption thanks the submission. i'd like to avoid one giant PR that includes lots of different changes, including merging multiple PRs into it..
This was a practical need for making a release, as the source PRs were in already in conflict and I wanted to have them merged before making any further fixes.
i also have some questions about why you chose to use dictionaries instead of lists in some places and also changing date times to perf counters.
I believe you mean the use of sets, as introduced by @septatrix in #57 and @fuhrysteve in #62. The former makes the ordering of the returned leet substitutions not matter (I did not check whether the test would otherwise fail). The latter uses a set to remove any duplicate entries of user input (although it has a side effect of also changing the order, possibly affecting scoring).
perf_counter is the appropriate way to measure runtime in Python, and should be preferred over calendar dates (already touched in #62). This has the unintended side effect that the runtime is now returned as a float rather than as timedelta, that needs to be fixed to maintain compatibility.
Now that is sorted out but some mypy issues remain. This is not something that I worked on but I can have a look at fixing them if you want (as a separate PR built on top of this).
The last commit is an important workaround to a Python 3.9.2 bug that prevents loading zxcvbn. While the problematic import introduced in #57 is technically correct and works on more recent Python versions (3.9.9 at least is OK), it is better to avoid asterisk imports entirely, and thus avoid this compatibility problem too.
This also increases the urgency of another release, so we are hoping that @dwolfhub would take action, as we really need all of these bugs fixed.
Development is moved to main branch to avoid pushing more commits on this PR (based on master of our fork). We just released zxcvbn-covert 0.5.1 with everything on this PR as well as some additional work to have typing checks pass cleanly, and a minor update of setup.py.
@covert-encryption I have some availability and would be happy to help get all your work released
@dwolfhub Cool. Do you wish for the typing fixes to be pushed on this PR as well, avoiding that test failure?
Also I (faintly) recall seeing some Python 3.8 feature being used in typing, so this might not work with 3.6 or 3.7 anymore. Need to have a look and get all the version numbers (CI, setup.py and readme) in sync.
Yes let’s get the tests to pass and get it merged. I’d like to support 3.7-3.10.
Yes, https://docs.python.org/3/library/typing.html#typing.TypedDict which is heavily used on zxcvbn.types is new in Python 3.8. However, even Debian is already on 3.9, other distros already made 3.10 the default, MacOS ships at least 3.8 (cannot check the latest OS versions) and a very few people have older than 3.8 anywhere. Although this certainly could be a concern for some (corporate) users of the module.
That should do it for the tests. I did not sync version numbers yet.
You could also add the official typing_extensions as a conditional dependency for older python versions
You could also add the official
typing_extensionsas a conditional dependency for older python versions
Would you wish to provide a patch for that? I don't even have anything older than 3.9 to test with :)
As far as other things go, it could still be supporting 3.6 (the bare minimum for typing and the ordered dict) or 3.7 but really needs actual testing to be sure (too many changes to review everything manually). TypedDict was just one thing I casually spotted.
Sorry to ping on an old PR, but is there any movement here?
I noticed that this was open while trying to determine if this library had 3.10 and newer in CI yet; it looks like 3.11 could also be added now.
Let me know if I can help at all; I'm happy to either help out with this branch or in any other way I can 🙂
