phxql / argon2-jvm

Argon2 Binding for the JVM
GNU Lesser General Public License v3.0
330 stars 32 forks source link

Feature/argon2 contexts secret and associated data #86

Closed suniastar closed 3 years ago

suniastar commented 3 years ago

I've added some advanced hash and verify methods to support argon2's secret and associated data. I needed this for one of my projects and also saw issue #83.

Added:

Modified:

Testing:

As this is one of my first real Github contributions feedback is much appreciated.

suniastar commented 3 years ago

BTW, is there a reason for the current java target and source version 1.6?

suniastar commented 3 years ago

I don't know why but there seems to be an issue with the mac Github checks. Only a few lines of code where changed with my last commit but the tests seem to be stuck or don't provide an error message.

EDIT: Seems to be a temporary issue. All checks after the last commit passed.

phxql commented 3 years ago

Wow, thanks for that contribution! I'm gonna take a look and test it.

Regarding to Java 1.6: That was done to support Android, afaik they need 1.6 bytecode.

suniastar commented 3 years ago

Should I revert it to 1.6 in this case or will you do it as part of your review?

Just FYI: Android uses 1.6 bytecode by default but most developers (incl. Google) suggest setting target and source compatibility to at least 1.8 in order to support all API features. https://developer.android.com/studio/write/java8-support

phxql commented 3 years ago

Should I revert it to 1.6 in this case or will you do it as part of your review?

Just FYI: Android uses 1.6 bytecode by default but most developers (incl. Google) suggest setting target and source compatibility to at least 1.8 in order to support all API features. https://developer.android.com/studio/write/java8-support

I would prefer to stay on 1.6 just to be on the safe side. What's the problem with 1.6?

I added some comments to your changes, please review. And thanks again for your contribution, it's great!

suniastar commented 3 years ago

Thanks for the review. I've adjusted the things we discussed in the conversations. Please take a look.

Let me also give you some praise for your dedicated work. Not every reviewer/project owner would have seen that I accidentally swapped the java doc for argon2i_ctx and argon2d_ctx.

phxql commented 3 years ago

Thank you! I've merged it into develop branch :)