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

[Enhancement] isHash should validate algorithm name #2295

Open
KUSHAGRARAJTIWARI opened this issue Sep 30, 2023 · 5 comments
Open

[Enhancement] isHash should validate algorithm name #2295

KUSHAGRARAJTIWARI opened this issue Sep 30, 2023 · 5 comments

Comments

@KUSHAGRARAJTIWARI
Copy link

KUSHAGRARAJTIWARI commented Sep 30, 2023

@WikiRik
I suggest having validation on the parameter algorithm in the isHash function.
The behavior below is not appropriate. The user of validator.js should get a proper error message. [Good for debugging]
image

Should I raise a pull request? It can be an enhancement.

@KUSHAGRARAJTIWARI KUSHAGRARAJTIWARI changed the title isHash should validate algorithm name [Enhancement] isHash should validate algorithm name Sep 30, 2023
@WikiRik
Copy link
Member

WikiRik commented Sep 30, 2023

Let's put this behind a new option for backwards compatibility. Some projects might use both for user input and then they expect a boolean returned instead of an error.

So isHash would get an option bag which consists of validateAlgorithm. I would recommend setting the default to true, and then people can set it to false if they don't want this new validation

@KUSHAGRARAJTIWARI
Copy link
Author

Let's put this behind a new option for backwards compatibility. Some projects might use both for user input and then they expect a boolean returned instead of an error.

So isHash would get an option bag which consists of validateAlgorithm. I would recommend setting the default to true, and then people can set it to false if they don't want this new validation

Can you please add an enhancement label to this issue and assign it to me?

@ITHIHAAS-RAMESH
Copy link

Let's put this behind a new option for backwards compatibility. Some projects might use both for user input and then they expect a boolean returned instead of an error.

So isHash would get an option bag which consists of validateAlgorithm. I would recommend setting the default to true, and then people can set it to false if they don't want this new validation

so, if validateAlgorithm is set to true, should the function throw an error saying "Invalid algorithm" and return false?

@WikiRik
Copy link
Member

WikiRik commented Sep 30, 2023

If it's set to true it should throw an Error, if it's set to false it will return false (the current behaviour)

@KUSHAGRARAJTIWARI
Copy link
Author

KUSHAGRARAJTIWARI commented Sep 30, 2023

@WikiRik
PR for enhancement suggested. Please review.
#2296

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