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

Adds test case #466

Closed
wants to merge 1 commit into from
Closed

Adds test case #466

wants to merge 1 commit into from

Conversation

joshuata
Copy link
Contributor

Closes #23

@joshuata
Copy link
Contributor Author

I made a mistake while committing and included changes from another pull request. I will revert them and we can squash the commits when merging.

src/pbkdf2.rs Outdated
let iterations = 0;
let salt = "salt".as_bytes();
let mut out = vec![0u8; 2];
pbkdf2::derive(prf, iterations as u32, &salt, &secret, &mut out);
Copy link
Owner

@briansmith briansmith Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

NIT: don't use as u32 here. If you need a type annotation then use let iterations: u32 = 0; instead.

Also, please make a variant of the test that has all the parameters the same, except iterations == 1, and which doesn't panic, to help demonstrate that iterations == 0 is the reason for the panic. Add a comment to the control test like “// Control for pkbdf2_zero_iterations().”

Please add the contributor's statement to the end of your commit message; see https://github.com/briansmith/ring#contributing.

If you know how, please rebase this on master without the ASAN changes, e.g. using git rebase -i master and moving this commit to the top of the list. If you don't know how to do that, don't worry about it; I'll do it when I land the change.

Adds a test case for pbkdf2 where iterations == 0.

I agree to license my contributions to each file under the terms given
at the top of each file I changed.
@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Changes Unknown when pulling ac7bb7b on joshuata:23_Adds_test_casses into ** on briansmith:master**.

@coveralls
Copy link

coveralls commented Feb 24, 2017

Coverage Status

Coverage increased (+0.3%) to 93.97% when pulling ac7bb7b on joshuata:23_Adds_test_casses into 3ff7e28 on briansmith:master.

@coveralls
Copy link

coveralls commented Feb 24, 2017

Coverage Status

Coverage increased (+0.08%) to 93.8% when pulling ac7bb7b on joshuata:23_Adds_test_casses into 3ff7e28 on briansmith:master.

@briansmith
Copy link
Owner

Thanks! This landed as b141727.

@briansmith briansmith closed this Mar 9, 2017
@briansmith briansmith added this to the 0.7.n milestone Mar 9, 2017
@briansmith briansmith modified the milestones: 0.7.n, 0.7.3 Mar 20, 2017
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

Successfully merging this pull request may close these issues.

3 participants