array_rand does not use secure random

[PHPGangsta]

Hi Steve,

One test is failing, because it tries to generate a secret with length 0, and mcrypt_create_iv() does not allow that: testCreateSecretLengthCanBeSpecified() See failed Travis build.

If you fix the test (it should start from 1 instead of 0), all tests should pass.

[RobThree]

A secret length of 0 (or 1 for that matter) makes no sense anyway; the (very) minimum value should be (around) 80 bits* and I would even consider throwing an exception on anything less. I also find it pretty arbitrary to test for values 0...100 (why 100 and not... 97? 109? 2091? 729183019?). It would make more sense to test for specific ("edge") cases and commonly used cases:

• Make sure you throw an exception on anything less than, say, 80 bits (80 bits is Google's recommended minimum (if I recall correctly))
• Test for common values (like 80, 128, 160, 256, 384, 512, etc... bits or the equivalent number of chars (see * below))
• Test for upper bounds that can/will cause exceptions in underlying implementations (like mcrypt_create_iv() or openssl_random_pseudo_bytes ()) or bounds that you set yourself (like, say, 4096) to keep things sane.

* Make sure you don't fall for the 1 character = 8 bits of entropy. Unless you use the entire 0..255 range of the byte (which you don't) you must count the actual bits of entropy (which is a nice round 5.0 at 32 characters available) per character. That is exactly why Google recommends at least 16 chars for the secret: 16 chars x 5 bits of entropy = 80 bits.

[jippi]

:+1:

[Sc00bz]

Umm I guess I can't just throw an exception because that causes the Travis CI build to failed. Will changing it to die(...) make it fail too?

[RobThree]

You could die or return an error code of some sort or whatever indicates that something is wrong. But having said that: exceptions can, ofcourse, also be unit-tested. In fact: you can indicate that you expect some method to throw an exception (or even more specific: you expect it to throw Exception of type X); the unit-test will PASS on that exception and FAIL if the exception isn't thrown. So throwing an exception doesn't automatically imply you can't unittest that code.

You can see here for example (noted by the @expectedException on several unittests) how to unittest code that is supposed to throw an exception.

[Conver]

+1