Skip to content
This repository has been archived by the owner on Feb 10, 2021. It is now read-only.

Match upstream #3

Merged
merged 32 commits into from Aug 1, 2018
Merged

Match upstream #3

merged 32 commits into from Aug 1, 2018

Conversation

clamburger
Copy link
Collaborator

With these last few changes we should now match upstream!

@coveralls
Copy link

coveralls commented Jul 12, 2018

Pull Request Test Coverage Report for Build 41

  • 171 of 171 (100.0%) changed or added relevant lines in 14 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+11.3%) to 99.864%

Totals Coverage Status
Change from base Build 18: 11.3%
Covered Lines: 735
Relevant Lines: 736

💛 - Coveralls

@mkopinsky
Copy link
Owner

Thanks for all your work! I'd like to review this side-by-side with upstream before officially releasing.

One thing we should address (either by deleting or updating) is that the ZxcvbnTest is still skipped.

@mkopinsky
Copy link
Owner

re that broken Travis build - it's a one-line workaround to get around that failure in PHP 5.5, but I think we should just support 5.6+. 5.5 has been unsupported for almost 2 years.

mkopinsky and others added 17 commits July 12, 2018 09:59
We had a much simpler algorithm for detecting character substitutions,
however it failed when dealing with certain characters. As such, this
change just ports upstream's logic, with only minor changes.
This affected passwords where there was a SpatialMatch that did
encompass the first character, and the first character in the
password was a shifted character
It's impossible for us to match all browsers exactly (since the way that
elements that compare as equal as sorted is implementation-defined in
JavaScript), but this gets us pretty close.
@clamburger
Copy link
Collaborator Author

@mkopinsky I think this is done in terms of matching upstream and is ready to merge. I've tested against the top 1 million passwords from the rockyou password list and haven't found any further discrepancies.

@mkopinsky
Copy link
Owner

Can you add the scripts or documentation on how you did that comparison? I think that would be a useful tool for the future when/if upstream releases a new version.

@mkopinsky mkopinsky merged commit 3d11c27 into master Aug 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants