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

Too much recursion exception is being thrown when a string with 28 characters and more is entred. #58

Open
saifqasa123 opened this issue Oct 28, 2021 · 6 comments

Comments

@saifqasa123
Copy link

saifqasa123 commented Oct 28, 2021

Recursion exception is being thrown when a password with 28 characters long has been entered for validation.

Here is a snippet from my code:
const pwordValidator = new passwordValidator();
pwordValidator .has() .digits(Number(minimumNumericCharacters) || defaultMinimumNumericChars); pwordValidator.validate(password);
Here is the exception:
Uncaught (in promise) InternalError: too much recursion _process lib.js:13 digits lib.js:80 _isPasswordValidFor index.js:26 validate index.js:81 isPasswordValid UserAccounts.tsx:314

@tarunbatra
Copy link
Owner

tarunbatra commented Oct 29, 2021

I have not been able to reproduce this. I used:

const pwordValidator = new passwordValidator();
pwordValidator.has().digits(10);

const pass = 'a very long password indeed. I dont know how many characters are these. Should be enough. 1234!'
console.log(pwordValidator.validate(pass));

Can you share what version of password-validator are you using and a reproducible code snippet?

@saifqasa123
Copy link
Author

I have tried 5.1.1 and 5.1.2 versions and I got the same result.
Here is the code snippet
` const isPasswordValid = () => {

if(!isValidClass3(password, MIN_PASSWORD_LENGTH, MAX_PASSWORD_LENGTH, false)) {
  return false;
}

const {
  minimumPasswordLength, // 8
  minimumLowerCaseCharacters, // 0
  minimumUpperCaseCharacters, // 0
  minimumNumericCharacters, // 29, 28. No exception when this criteria is assigned 27 or less  
  minimumSpecialCharacters, // 0
} = passwordComplexity;

pwordValidator
  .is()
  .min(Number(minimumPasswordLength) || defaultMinimumPasswordLength)
  .has()
  .not()
  .spaces();

Number(minimumLowerCaseCharacters) > 0 &&
  pwordValidator
    .has()
    .lowercase(
      Number(minimumLowerCaseCharacters) || defaultMinimumLowercaseChars
    );

Number(minimumUpperCaseCharacters) > 0 &&
  pwordValidator
    .has()
    .uppercase(
      Number(minimumUpperCaseCharacters) || defaultMinimumUppercaseChars
    );

Number(minimumNumericCharacters) > 0 &&
  pwordValidator
    .has()
    .digits(Number(minimumNumericCharacters) || defaultMinimumNumericChars);

Number(minimumSpecialCharacters) > 0 &&
  pwordValidator
    .has()
    .symbols(
      Number(minimumSpecialCharacters) || defaultMinimumSpecialChars
    );

return pwordValidator.validate(password);

};`

@saifqasa123
Copy link
Author

saifqasa123 commented Nov 1, 2021

Here is another code snippet that illustrates the recursive issue.
`const isPasswordValid = (password) => {
const pwordValidator = new passwordValidator();
pwordValidator
.is().min(8)
.has().not().spaces()
.has().digits(10);

return pwordValidator.validate(password);
};

console.time('10 digits');
const testBack = isPasswordValid("1234567890")
console.timeEnd('10 digits');
console.log(testBack);`

Output:
10 digits: 0.593ms
true

`const isPasswordValid = (password) => {
const pwordValidator = new passwordValidator();
pwordValidator
.is().min(8)
.has().not().spaces()
.has().digits(20);

return pwordValidator.validate(password);
};

console.time('20 digits');
const testBack = isPasswordValid("12345678901234567890")
console.timeEnd('20 digits');
console.log(testBack);`

Output:
20 digits: 0.612ms
true

`const isPasswordValid = (password) => {
const pwordValidator = new passwordValidator();
pwordValidator
.is().min(8)
.has().not().spaces()
.has().digits(30);

return pwordValidator.validate(password);
};

console.time('30 digits');
const testBack = isPasswordValid("123456789012345678901234567890")
console.timeEnd('30 digits');
console.log(testBack);`

Output:
30 digits: 1:01.206 (m:ss.mmm)
true

`const isPasswordValid = (password) => {
const pwordValidator = new passwordValidator();
pwordValidator
.is().min(8)
.has().not().spaces()
.has().digits(40);

return pwordValidator.validate(password);
};

console.time('40 digits');
const testBack = isPasswordValid("1234567890123456789012345678901234567890")
console.timeEnd('40 digits');
console.log(testBack);`

Output:
40 digits: 2:44.475 (m:ss.mmm)
false

In nodejs, increment the digit criteria by 10 would increase the performance time significantly.
In browser, it would throw too much recursion exception.

@tarunbatra
Copy link
Owner

Thanks for the snippet. I am able to reproduce this performance issue and trying to identify the reason behind it. RegExp in my understanding shouldn't be this slow. Feel free if you want to get involved in fixing this as well.

@tarunbatra
Copy link
Owner

My initial investigation indicates that it is a case of catastrophic backtracking. The issue is reproducible here.

I feel a solution could be changing the pattern to something like this but I am not sure if it will cover all the cases. Also I feel this is an edge case because passwords are not usually this long.

@saifqasa123
Copy link
Author

The same issue occurs when the symbols method has a big value.

const pwordValidator = new passwordValidator(); pwordValidator .has().not().spaces() .has().symbols(40);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants