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.

Sign up for GitHub

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

Jump to bottom

Exactly match upstream dropbox library's functionality and algorithms #32

Merged
merged 115 commits into from
Jan 27, 2020
Merged

Exactly match upstream dropbox library's functionality and algorithms #32

merged 115 commits into from
Jan 27, 2020

Conversation

mkopinsky
Copy link
Collaborator

Addresses #15

@mkopinsky
Initial work trying to make zxcvbn-php match the upstream library
f62f3e3
@mkopinsky
Add the data files from upstream
166b17a
@mkopinsky
Add unmodified data scripts from upstream
b9e8bb7 
The other files in upstream's data-scripts dir are used to generate
data/*.txt, which we should just copy when they update them.
These files are used (in upstream) to build src/adjacency_graphs.coffee
and src/frequency_lists.coffee, and will be modified in the PHP port to
instead build src/Matchers/frequency_lists.json and
src/Matchers/adjacency_graphs.json
@mkopinsky
Update the python scripts to generate json instead of coffee script
0d67738 
adjacency_graphs.json is unmodified. Frequency_lists.json now contains
additional datasets
@mkopinsky
Update notes re data files
0e5a62c
@mkopinsky
Fill in TimeEstimator::displayTime(), based directly on upstream's co…
44dbbde 
…ffeescript implementation
@mkopinsky
Update todo in notes
3590c08
@mkopinsky
Actually pass in $seconds to the closure... oops
7a532e3
@mkopinsky
Fix bad variable reference
74ac8e6
@mkopinsky
Move the callback to make this compatible with PHP 5
5284fd2
@mkopinsky
WIP: Translating coffee script tests into PHP
5736b87
@mkopinsky
WIP: Fleshing out the Feedback stuff, putting it in the individual Ma…
f123ffb 
…tch classes
@mkopinsky
Add some more tests for RepeatMatch, fix one small bug
6e36b73
@mkopinsky
Change order of arguments to assert to match phpunit expectation
77fa053
@mkopinsky
Add tests for multiple character repeats
5b09c1c
@mkopinsky
Get RepeatMatchTest to pass without errors/warnings
a9f230e
@mkopinsky
Add Feedback to remaining Matchers
1898086
@mkopinsky
Get rid of DigitMatch since it doesn't correspond to anything in upst…
f23bed7 
…ream
@mkopinsky
Make this class actually abstract
ab804fe
@mkopinsky
WIP porting l33t test from coffeescript to PHP
0ae780d
@mkopinsky
Add remaining L33t tests
237847a
@mkopinsky
Disable the scorer test for now - it will be rewritten as part of the…
b761b0f 
… re-port
@mkopinsky
Pin the version of phpunit
133b954
@mkopinsky
Make the phpunit version constraint less strict, so this can run on o…
c67d579 
…lder versions of PHP
@mkopinsky
Get rid of typehints - these break in PHP 5.6
72bd4e4
@mkopinsky
Try again to set a version constraint that travis can use for PHP 5.4
baf244c
@mkopinsky
Use the right distribution when running PHP 5.3 on travis
cfa3c6b
@mkopinsky
Remove some more typehints for PHP 5.4 compatibility
9e565d0
@mkopinsky
Try again to get travis to build properly with PHP 5.3
cc509d2
@mkopinsky
Update DateTest to match upstream's tests. So far, all of them fail.
96be2fc
clamburger and others added 13 commits July 18, 2018 19:29
@clamburger
Use a sorting algorithm that's as close to upstream as possible
f2f540b 
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
Don't use array_fill with a length of zero
259025d 
This works around a PHP 5.5 issue
@clamburger
Fixed infinite loop caused by multibyte offsets in preg_match_all
af11603
@clamburger
Improve performance of repeated matches by caching the JSON files
a8a0113
@clamburger
Fixed notice when using the mbstring extension instead of the polyfill
63c2aa4
@clamburger
Sort the optimal arrays to match how JavaScript behaves
e34690f
@clamburger
Fixed instance where the non-multibyte-safe version of strrev was used
e3d2272
@clamburger
A bit of cleanup and add tests for untested methods
7f7c701
@mkopinsky
Get rid of ScorerInterface which is no longer used
ee56150
@mkopinsky
Get rid of some no-longer-relevant comments relating to the rewrite
aea25a8
@mkopinsky
Get rid of WIP notes about rewrite
09033ca
@mkopinsky
Merge pull request #3 from mkopinsky/match-upstream
3d11c27 
Match upstream
@mkopinsky
Merge branch 'master' of https://github.com/bjeavons/zxcvbn-php
2ad3416 
# Conflicts:
#	.travis.yml
#	composer.json
#	composer.lock
#	src/Matcher.php
#	src/Matchers/Bruteforce.php
#	src/Matchers/DateMatch.php
#	src/Matchers/DictionaryMatch.php
#	src/Matchers/DigitMatch.php
#	src/Matchers/L33tMatch.php
#	src/Matchers/Match.php
#	src/Matchers/MatchInterface.php
#	src/Matchers/RepeatMatch.php
#	src/Matchers/SequenceMatch.php
#	src/Matchers/SpatialMatch.php
#	src/Matchers/YearMatch.php
#	src/Scorer.php
#	src/ScorerInterface.php
#	src/Searcher.php
#	src/Zxcvbn.php
#	test/MatcherTest.php
#	test/Matchers/BruteforceTest.php
#	test/Matchers/DateTest.php
#	test/Matchers/DictionaryTest.php
#	test/Matchers/DigitTest.php
#	test/Matchers/L33tTest.php
#	test/Matchers/RepeatTest.php
#	test/Matchers/SequenceTest.php
#	test/Matchers/SpatialTest.php
#	test/Matchers/YearTest.php
#	test/ScorerTest.php
#	test/SearcherTest.php
#	test/ZxcvbnTest.php
@mkopinsky mkopinsky mentioned this pull request Sep 6, 2018
Closed
@timwsuqld
Copy link

Any progress on getting this merged in?

@mkopinsky
Copy link
Collaborator Author

This PR has now been open for 6 weeks, and active work on the branch has been going on since March. I've gone ahead and published my fork on packagist - https://packagist.org/packages/mkopinsky/zxcvbn-php

@peter279k
Copy link
Contributor

peter279k commented Apr 7, 2019 •
edited

Actually, these commits in this PR are very huge.

It should clear them and I don't think other people commits should include in this PR :).

@mkopinsky
Copy link
Collaborator Author

The amount of changes indeed is huge, but that's what was necessary to exactly match the upstream js library.

@peter279k
Copy link
Contributor

Hi @mkopinsky, thank you for your reply.

I think this PR should be cleaned up the commits and squashed all commits to one commit.

In general, the repository owner doesn't want to merge a PR with the large commits :-).

@mkopinsky
Copy link
Collaborator Author

The issue with this PR (in my mind) isn't the number of commits but the number of changes. If @bjeavons says that he'd prefer me to squash commits, I'd be happy to do so.

@peter279k
Copy link
Contributor

Hi @mkopinsky.

Ok. Thank you for answer and just wait for the repository author.

@Hate9
Copy link

Hate9 commented Nov 22, 2019

Hey, can @bjeavons please take a look and answer? I'm using the library and I'd really like to be able to update my app so that the PHP side matches the JS.

@mkopinsky
Copy link
Collaborator Author

@bjeavons has indicated that he doesn't intend on merging this.

If you want your PHP side to match the JS, switch to mkopinsky/zxcvbn-php in your composer.json

@Hate9
Copy link

Hate9 commented Nov 22, 2019 •
edited

@mkopinsky It's a shame that he didn't want to merge it, but I'm glad I can just update to your repo.
Any idea why he didn't want to merge the PR?

EDIT: Also, just switched to your repo, and it works like a charm. 😊

@mkopinsky
Copy link
Collaborator Author

#15 (comment)

@bjeavons bjeavons merged commit d5ebb26 into bjeavons:master Jan 27, 2020
@tgr tgr mentioned this pull request Mar 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mkopinsky @timwsuqld @peter279k @Hate9 @bjeavons @clamburger