Specify Encoding for Version String (#87)

[m4dc4p]

Added an annotation to the sodium_version_string() method so it
is always read as an ASCII string. Avoids some issues on Windows
with reading multi-byte strings by default.

Added a test to make sure version string matches the expected
pattern.

For issue #87.

[m4dc4p]

To test this fix, first download https://download.libsodium.org/libsodium/releases/libsodium-1.0.16-msvc.zip and (from that archive) copy x64\Release\v141\dynamic\libsodium.dll (or Win32, if on a 32-bit OS) to a directory on your PATH so Kalium will find the DLL.

1. To see test failures, clone master and run tests without this fix. A bunch will fail due to the version string not being parsed correctly.
2. Switch to this branch and run all tests again. They will all pass.

[dmitry-timofeev]

Great, thank you!

We faced the same issue, when tried to use kalium on Windows with the latest version of the libsodium. @abstractj , would it be possible to integrate this patch? Thanks!

[dmitry-timofeev]

Could we use a matcher to simplify code and improve error reporting:

assertThat(version, matchesPattern("^\\d+\\.\\d+\\.\\d+$"));

[dmitry-timofeev]

I'd add static imports for such commonly used and familiar methods (like assertX, mock/verify, various matchers).

[m4dc4p]

I've moved on to the lazy sodium library. I'd suggest checking that out, this project seems dead.

…

On Thu, Jul 5, 2018, 8:57 AM Dmitry Timofeev ***@***.***> wrote: ***@***.**** commented on this pull request. Great, thank you! We faced the same issue, when tried to use kalium on Windows with the latest version of the libsodium. @abstractj <https://github.com/abstractj> , would it be possible to integrate this patch? Thanks! ------------------------------ In src/test/java/org/abstractj/kalium/crypto/UtilTest.java <#88 (comment)>: > @@ -31,4 +34,10 @@ public void testPrependZeros() throws Exception { public void testDataNull() { Util.checkLength(null, 3); } + + @test + public void testSodiumVersion() { + Assert.assertTrue(NaCl.sodium().sodium_version_string() + " did not match expected pattern.", Could we use a matcher to simplify code and improve error reporting: assertThat(version, matchesPattern("^\\d+\\.\\d+\\.\\d+$")); ------------------------------ In src/test/java/org/abstractj/kalium/crypto/UtilTest.java <#88 (comment)>: > @@ -31,4 +34,10 @@ public void testPrependZeros() throws Exception { public void testDataNull() { Util.checkLength(null, 3); } + + @test + public void testSodiumVersion() { + Assert.assertTrue(NaCl.sodium().sodium_version_string() + " did not match expected pattern.", I'd add static imports for such commonly used and familiar methods (like assertX, mock/verify, various matchers). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#88 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABJAYMsKW5Ne0RLQClu3cWnZZkZyHUNks5uDjdRgaJpZM4RjyvY> .

[dmitry-timofeev]

@m4dc4p , I see, thank you very much for the recommendation and the investigation of this issue. Will check that library and see if it works for us!