Possible regression with crypto.randomBytes(size, null)

[sehrope]

• v9.0.0
• Linux 4.10.0-37-generic Add a Gitter chat badge to README.md #41~16.04.1-Ubuntu SMP Fri Oct 6 22:42:59 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
• crypto

The crypto.psuedoRandomBytes(...) and crypto.randomBytes(...) functions throw an ERR_INVALID_CALLBACK if the second parameter is null. In prior versions a null value for the second parameter would be treated the same as undefined causing the function to be invoked as if it had been invoked with just one parameter, i.e. crypto.randomBytes(size).

Reproduce via:

$ nvm use 8
Now using node v8.9.0 (npm v5.5.1)
$ node -e "console.log(require('crypto').randomBytes(4, null).toString('hex'))"
ef813d36

$ nvm use 9
Now using node v9.0.0 (npm v5.5.1)
$ node --version
v9.0.0
$ node -e "console.log(require('crypto').randomBytes(4, null).toString('hex'))"
internal/crypto/random.js:44
    throw new errors.TypeError('ERR_INVALID_CALLBACK');
    ^

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
    at Object.randomBytes (internal/crypto/random.js:44:11)
    at [eval]:1:31
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:152:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:641:30)
    at evalScript (bootstrap_node.js:470:27)
    at startup (bootstrap_node.js:167:9)
    at bootstrap_node.js:613:3

#16454 introduced the parameter validation change that is causing this.

I'm not sure if this should be considered a regression as the function is being invoked with two arguments (i.e. null where a callback is expected) but I didn't see anything in the changelog about it though so not sure if it was overlooked.

[maclover7]

cc @jasnell

[joyeecheung]

Personally I don't think it's a regression because null has never been documented as a valid value for cb, but it worth mentioning in the change log, eventhough it's already semver-major.

[jasnell]

Yeah I likely should have called that out in the notable changes because it is hidden. This was actually somewhat intentional to make the type validation tighter.

[sehrope]

Ok makes sense. I think calling it out explicitly in the changelog would be useful.