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

ConfigurationBuilder.getDefaultXYZ returns internal instances #65

Open
per-erik opened this issue Nov 12, 2021 · 4 comments
Open

ConfigurationBuilder.getDefaultXYZ returns internal instances #65

per-erik opened this issue Nov 12, 2021 · 4 comments

Comments

@per-erik
Copy link

Because ConfigurationBuilder.getDefaultXYZ returns the internal instances (and because of the way the documentation suggests to add stuff to the configuration builder), one can easily add for example matchers multiple times.

public int estimate(String password) {
    List<PasswordMatcher> matchers = ConfigurationBuilder.getDefaultPasswordMatchers();
            matchers.add(new PasswordMatcher() {
                @Override
                public List<Match> match(Configuration configuration, String password) {
                    System.out.println("Called with: " + password);
                    return new ArrayList<>();
                }
            });
    Configuration config = new ConfigurationBuilder()
        .setPasswordMatchers(matchers)
        .createConfiguration();
    Nbvcxz tester = new Nbvcxz(config)
    Result result = tester.estimate(password);
    return result.getBasicScore(); // Yes I've seen that you dislike this, but just to illustrate! :)
}

Running the above once will print "Called with ...". Running it a second time will print "Called with ..." twice, a third time trice, a fourth time will print it four times and so on.

One can get around this by either creating the configuration once (and not every time a test is made) or by wrapping the return value of ConfiguratioBuilder.getDefaultPasswordMatchers() in a new ArrayList.

This is not a big bug but given the documentation I spent a good half hour scratching my head as to why I was getting multiple calls to my "PasswordMatcher".

Best Regards,
Per-Erik

@Tostino
Copy link
Collaborator

Tostino commented Nov 12, 2021

Appreciate the bug report. Will look into it and get that fixed.

@ashleyfrieze
Copy link

I think the above may also result in a lack of thread safety. I was getting arraylist concurrent modification exceptions.

@Tostino
Copy link
Collaborator

Tostino commented Dec 10, 2021

Yeah, it absolutely would. I just got back from a two week vacation, so I have a bit to catch up on, but will try and get this sorted as quickly as I can. Pull requests are always welcome if someone gets to this before me.

@Tostino
Copy link
Collaborator

Tostino commented Jan 31, 2023

Took me a little bit, but have a fix in-progress: #82

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

No branches or pull requests

3 participants