Skip to content

lib: coerce -0 to +0 in various APIs #63556

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.

Sign up for GitHub

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

Open
panva wants to merge 5 commits into
base: main
Choose a base branch
from
Open

lib: coerce -0 to +0 in various APIs #63556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac85237
crypto: coerce -0 to +0 before native calls
panva May 25, 2026
7e2ac12
dns: coerce -0 to +0 in lookup and resolver inputs
panva May 25, 2026
673d7ec
fs: coerce -0 to +0 in mode flags and watch intervals
panva May 25, 2026
6693324
net: coerce -0 to +0 in BlockList prefixes
panva May 25, 2026
ace7877
zlib: coerce -0 to +0 for crc32 seeds
panva May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions lib/dns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ function lookup(hostname, options, callback) {
validateFunction(callback, 'callback');

validateOneOf(options, 'family', validFamilies);
family = options;
// Coerce -0 to +0.
family = options + 0;
} else if (options !== undefined && typeof options !== 'object') {
validateFunction(arguments.length === 2 ? options : callback, 'callback');
throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
Expand All @@ -179,7 +180,8 @@ function lookup(hostname, options, callback) {
break;
default:
validateOneOf(options.family, 'options.family', validFamilies);
family = options.family;
// Coerce -0 to +0.
family = options.family + 0;
break;
}
}
Expand Down Expand Up @@ -274,7 +276,8 @@ function lookupService(address, port, callback) {

validateFunction(callback, 'callback');

port = +port;
// Coerce -0 to +0.
port = +port + 0;

const req = new GetNameInfoReqWrap();
req.callback = callback;
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/blocklist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class BlockList {
validateInt32(prefix, 'prefix', 0, 128);
break;
}
// Coerce -0 to +0.
prefix += 0;
this[kHandle].addSubnet(network[kSocketAddressHandle], prefix);
}

Expand Down
9 changes: 7 additions & 2 deletions lib/internal/crypto/cipher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ function getUIntOption(options, key) {
if (options && (value = options[key]) != null) {
if (value >>> 0 !== value)
throw new ERR_INVALID_ARG_VALUE(`options.${key}`, value);
return value;
// Coerce -0 to +0.
return value + 0;
}
return -1;
}
Expand Down Expand Up @@ -256,12 +257,16 @@ const kMinNid = 1;
const kMaxNid = 2_147_483_647;
function getCipherInfo(nameOrNid, options = {}) {
validateObject(options, 'options');
const { keyLength, ivLength } = options;
let { keyLength, ivLength } = options;
if (keyLength !== undefined) {
validateUint32(keyLength, 'options.keyLength');
// Coerce -0 to +0.
keyLength += 0;
}
if (ivLength !== undefined) {
validateUint32(ivLength, 'options.ivLength');
// Coerce -0 to +0.
ivLength += 0;
}

const type = typeof nameOrNid;
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/crypto/diffiehellman.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,11 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) {
// rejected with ERR_OSSL_BN_BITS_TOO_SMALL) by OpenSSL. The glue code
// in node_crypto.cc accepts values that are IsInt32() for that reason
// and that's why we do that here too.
if (typeof sizeOrKey === 'number')
if (typeof sizeOrKey === 'number') {
validateInt32(sizeOrKey, 'sizeOrKey');
// Coerce -0 to +0.
sizeOrKey += 0;
}

if (keyEncoding && !Buffer.isEncoding(keyEncoding) &&
keyEncoding !== 'buffer') {
Expand Down
9 changes: 7 additions & 2 deletions lib/internal/crypto/hash.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,13 @@ function Hash(algorithm, options) {
const isCopy = algorithm instanceof _Hash;
if (!isCopy)
validateString(algorithm, 'algorithm');
const xofLen = typeof options === 'object' && options !== null ?
let xofLen = typeof options === 'object' && options !== null ?
options.outputLength : undefined;
if (xofLen !== undefined)
if (xofLen !== undefined) {
validateUint32(xofLen, 'options.outputLength');
// Coerce -0 to +0.
xofLen += 0;
}
// Lookup the cached ID from JS land because it's faster than decoding
// the string in C++ land.
const algorithmId = isCopy ? -1 : getCachedHashId(algorithm);
Expand Down Expand Up @@ -285,6 +288,8 @@ function hash(algorithm, input, options) {

if (outputLength !== undefined) {
validateUint32(outputLength, 'outputLength');
// Coerce -0 to +0.
outputLength += 0;
}

if (outputLength === undefined) {
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/crypto/hkdf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ const validateParameters = hideStackFrames((hash, key, salt, info, length) => {
info = validateByteSource.withoutStackTrace(info, 'info');

validateInteger.withoutStackTrace(length, 'length', 0, kMaxLength);
// Coerce -0 to +0.
length += 0;

if (info.byteLength > 1024) {
throw new ERR_OUT_OF_RANGE.HideStackFramesError(
Expand Down
32 changes: 24 additions & 8 deletions lib/internal/crypto/keygen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,18 @@ function createJob(mode, type, options) {
case 'rsa-pss':
{
validateObject(options, 'options');
const { modulusLength } = options;
let { modulusLength } = options;
validateUint32(modulusLength, 'options.modulusLength');
// Coerce -0 to +0.
modulusLength += 0;

let { publicExponent } = options;
if (publicExponent == null) {
publicExponent = 0x10001;
} else {
validateUint32(publicExponent, 'options.publicExponent');
// Coerce -0 to +0.
publicExponent += 0;
}

if (type === 'rsa') {
Expand All @@ -238,12 +242,14 @@ function createJob(mode, type, options) {
...encoding);
}

const {
hashAlgorithm, mgf1HashAlgorithm, saltLength,
} = options;
const { hashAlgorithm, mgf1HashAlgorithm } = options;
let { saltLength } = options;

if (saltLength !== undefined)
if (saltLength !== undefined) {
validateInt32(saltLength, 'options.saltLength', 0);
// Coerce -0 to +0.
saltLength += 0;
}
if (hashAlgorithm !== undefined)
validateString(hashAlgorithm, 'options.hashAlgorithm');
if (mgf1HashAlgorithm !== undefined)
Expand Down Expand Up @@ -284,14 +290,19 @@ function createJob(mode, type, options) {
case 'dsa':
{
validateObject(options, 'options');
const { modulusLength } = options;
let { modulusLength } = options;
validateUint32(modulusLength, 'options.modulusLength');
// Coerce -0 to +0.
modulusLength += 0;

let { divisorLength } = options;
if (divisorLength == null) {
divisorLength = -1;
} else
} else {
validateInt32(divisorLength, 'options.divisorLength', 0);
// Coerce -0 to +0.
divisorLength += 0;
}

return new DsaKeyPairGenJob(
mode,
Expand Down Expand Up @@ -321,7 +332,8 @@ function createJob(mode, type, options) {
case 'dh':
{
validateObject(options, 'options');
const { group, primeLength, prime, generator } = options;
const { group, prime } = options;
let { primeLength, generator } = options;
if (group != null) {
if (prime != null)
throw new ERR_INCOMPATIBLE_OPTION_PAIR('group', 'prime');
Expand All @@ -342,13 +354,17 @@ function createJob(mode, type, options) {
validateBuffer(prime, 'options.prime');
} else if (primeLength != null) {
validateInt32(primeLength, 'options.primeLength', 0);
// Coerce -0 to +0.
primeLength += 0;
} else {
throw new ERR_MISSING_OPTION(
'At least one of the group, prime, or primeLength options');
}

if (generator != null) {
validateInt32(generator, 'options.generator', 0);
// Coerce -0 to +0.
generator += 0;
}
return new DhKeyPairGenJob(
mode,
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/crypto/random.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,12 +603,14 @@ function checkPrime(candidate, options = kEmptyObject, callback) {
}
validateFunction(callback, 'callback');
validateObject(options, 'options');
const {
let {
checks = 0,
} = options;

// The checks option is unsigned but must fit into a signed C int for OpenSSL.
validateInt32(checks, 'options.checks', 0);
// Coerce -0 to +0.
checks += 0;

const job = new CheckPrimeJob(kCryptoJobAsync, candidate, checks);
job.ondone = callback;
Expand All @@ -632,12 +634,14 @@ function checkPrimeSync(candidate, options = kEmptyObject) {
);
}
validateObject(options, 'options');
const {
let {
checks = 0,
} = options;

// The checks option is unsigned but must fit into a signed C int for OpenSSL.
validateInt32(checks, 'options.checks', 0);
// Coerce -0 to +0.
checks += 0;

const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks);
const { 0: err, 1: result } = job.run();
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/dns/promises.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ function lookup(hostname, options) {

if (typeof options === 'number') {
validateOneOf(options, 'family', validFamilies);
family = options;
// Coerce -0 to +0.
family = options + 0;
} else if (options !== undefined && typeof options !== 'object') {
throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
} else {
Expand All @@ -216,7 +217,8 @@ function lookup(hostname, options) {
}
if (options?.family != null) {
validateOneOf(options.family, 'options.family', validFamilies);
family = options.family;
// Coerce -0 to +0.
family = options.family + 0;
}
if (options?.all != null) {
validateBoolean(options.all, 'options.all');
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/dns/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,18 @@ const {
} = require('internal/v8/startup_snapshot');

function validateTimeout(options) {
const { timeout = -1 } = { ...options };
let { timeout = -1 } = { ...options };
validateInt32(timeout, 'options.timeout', -1);
// Coerce -0 to +0.
timeout += 0;
return timeout;
}

function validateMaxTimeout(options) {
const { maxTimeout = 0 } = { ...options };
let { maxTimeout = 0 } = { ...options };
validateUint32(maxTimeout, 'options.maxTimeout');
// Coerce -0 to +0.
maxTimeout += 0;
return maxTimeout;
}

Expand Down
3 changes: 2 additions & 1 deletion lib/internal/fs/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,8 @@ function getStatFsFromBinding(stats) {
function stringToFlags(flags, name = 'flags') {
if (typeof flags === 'number') {
validateInt32(flags, name);
return flags;
// Coerce -0 to +0.
return flags + 0;
}

if (flags == null) {
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/fs/watchers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ StatWatcher.prototype[kFSStatWatcherStart] = function(filename,

filename = getValidatedPath(filename, 'filename');
validateUint32(interval, 'interval');
// Coerce -0 to +0.
interval += 0;
Comment on lines 178 to +180
Copy link
Copy Markdown
Contributor

@aduh95 aduh95 May 25, 2026

Choose a reason for hiding this comment

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

Shouldn't we update validateUint32 to reject -0 instead?

Copy link
Copy Markdown
Member Author

@panva panva May 25, 2026

Choose a reason for hiding this comment

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

I have no particular sympathy for either or.

Copy link
Copy Markdown
Contributor

@aduh95 aduh95 May 25, 2026

Choose a reason for hiding this comment

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

Having it part of the helper is probably more robust than relying on humans to remember for checking for -0

Copy link
Copy Markdown
Contributor

@TheOneTheOnlyJJ TheOneTheOnlyJJ May 25, 2026

Choose a reason for hiding this comment

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

Having it part of the helper is probably more robust than relying on humans to remember for checking for -0

Maybe even have it part of all uint conversion helpers?

Copy link
Copy Markdown
Member Author

@panva panva May 26, 2026

Choose a reason for hiding this comment

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

we can certainly start blanket rejecting -0 from these validators in a semver-major followup if you feel like picking it up. #63531 already went the same route of this PR and is in the commit-queue

const err = this._handle.start(toNamespacedPath(filename), interval);
if (err) {
const error = new UVException({
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/validators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ function parseFileMode(value, name, def) {
}

validateUint32(value, name);
return value;
// Coerce -0 to +0.
return value + 0;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions lib/zlib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,8 @@ function crc32(data, value = 0) {
throw new ERR_INVALID_ARG_TYPE('data', ['Buffer', 'TypedArray', 'DataView', 'string'], data);
}
validateUint32(value, 'value');
// Coerce -0 to +0.
value += 0;
return crc32Native(data, value);
}

Expand Down
Loading
Loading