pidusage doesn't resolve if pids array gets modified meanwhile updateCpu() gets called

[piranna]

API Platform version(s) affected:

Probably all, detected while using Ubuntu 23.10

Description

pids array argument is checked not being zero length at

pidusage/lib/stats.js

Lines 71 to 73 in 61480f7

	if (pids.length === 0) {
	return callback(new TypeError('You must provide at least one pid'))
	}

, but it's passed and used as reference in all the code, so when calling async function updateCpu() at

pidusage/lib/procfile.js

Line 125 in 61480f7

updateCpu(cpuInfo, function (err, result) {

, it can be possible that the array gets modified in application code (for example, due to a race condition), so when calling its callback, pids can be a zero length array without getting noticed. This has the effect that fns object will be empty instead of being filled at

pidusage/lib/procfile.js

Lines 131 to 135 in 61480f7

	pids.forEach(function (pid, i) {
	fns[pid] = function (cb) {
	readProcFile(pid, options, cb)
	}
	})

, and later when calling to parallel() helper, it will never call to the each() function instead of doing it at

pidusage/lib/helpers/parallel.js

Lines 30 to 34 in 61480f7

	keys.forEach(function (key) {
	fns[key](function (err, res) {
	each(key, err, res)
	})
	})

, and the done() callback will never be called back.

Possible Solution

There are several, not incompatible alternatives:

• create a new array instead of modify the original one at

  pidusage/lib/stats.js

  Lines 75 to 80 in 61480f7

  	for (let i = 0; i < pids.length; i++) {
  	pids[i] = parseInt(pids[i], 10)
  	if (isNaN(pids[i]) || pids[i] < 0) {
  	return callback(new TypeError('One of the pids provided is invalid'))
  	}
  	}

  when parsing the pid strings. Using Array.map() here is just enough. Problem is, if process exits later, maybe we would be trying to access to some info of a non-existing process, that could lead to some other bigger problems down the line, for example when reading procfile, but maybe that would be done on purpose
• check (again) the array is still non zero length at updateCpu() callback, and call done() with an error if that happens. Alternatively, return an empty array instead
• check that fns is not an empty array or object in [parallel()helper](https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/helpers/parallel.js#L12), and calldone()` with an error if that happens. That would be the most versatile solution, but would detect it later. Alternatively, return an empty array instead

Personally I would implement at least 2 and 3 for safety, and if we want to get procfile errors on the benefict of having an inmutable input data, also number 1, maybe returning a structure similar to the one returned by Promise.allSettled() that combines both results and errors at the same time.

[soyuka]

would love a pr if you're up to it!

[piranna]

Just to fic this, or a full redactor? There are some needed updates, but others like using Promise.allSettled() would be backward incompatible with older versions of Node.js... but for maintained ones It would be totally safe.

[piranna]

In fact, i'm thinking the best option would be to return an empty object or array if it's zero length, also when provided like this, so i would remové the throwed exceptuon