Check for open_basedir before reading /proc

[solracsf]

• Resolves: [Bug]: flood of errors is_file(): open_basedir restriction in effect. File(/proc/cpuinfo) after 25 > 26 update #37921

Summary

Check if open_basedir is set. If yes and is restrictive, or if can't be determined, set it to 0.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[kesselb]

It should also work to use strpos /proc and strpos /proc/cpuinfo on openbasedir without the explode 🤔

[kesselb]

I was wrong. To check properly for /proc and/or /proc/cpuinfo we indeed need to iterate over the entries and make sure to detect if the whole folder is allowed.

[kesselb]

I think we should remove getHardwareConcurrency and use a sane default.

A hint for preview_concurrency_all and preview_concurrency_new at https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html would be nice.

Every time a generator instance is loaded, we fetch /proc/cpuinfo to use the number of processor as default value. I think this is unnecessary.

We don't support FreeBSD. But TrueNAS which is based on FreeBSD. On FreeBSD /proc/cpuinfo is not available. We need a different way to retrieve the processors: https://github.com/nextcloud/serverinfo/blob/1146b21d13073c5e8799eb707096d12b5f682026/lib/OperatingSystems/FreeBSD.php#L81

My recommendation is to keep it simple.

[joshtrichards]

This whole section of code getHardwareConcurrency() is semi-irrelevant anyhow since there are defaults defined and the admin can also define the best (preferred) parameters via preview_concurrency_* already. This is just fallback code and shouldn't be getting executed every time.

The overall generator CPU balancing feature is sound, but the code elsewhere within it could use some refactoring.

The only reason the open_basedir trigger in this issue can't easily be headed off by the admin simply configuring the preview_concurrency_* settings is because of the order of operations the fallback is implemented in.

I also wouldn't bother with trying to process the contents of the open_basedir if it's found. I'd just fall back to the sane defaults (which the admin can still override if they wish as currently coded). Plus even after parsing open_basedir the processing of /proc could still fail for other reasons (like different OSes as @kesselb noted).

That said, K.I.S.S. - and rather than submitting a competing PR - here's the quick and dirty path I suggest until a more extensive refactor can happen:

joshtrichards@30c5f24

[solracsf]

I agree that simplication is (almost) ALWAYS the way to go, but this PR is only about open_basedir error suppression, nothing more. Feel free to submit a proper PR to simplify the whole thing or complete the actual checks for all OSes.

[joshtrichards]

Seems okay to me. Achieves the primary aim: allow some folks to add it to their open_basedir, but if they can't (or don't) it'll fall through to the defaults while still also permitting an explicit override to more appropriate local parameters by way of the preview_concurrency_* config parameters. And all without erroring out.

There are still some edge cases like if open_basedir = "/tmp:/home/user:/proc" which technically does permit cpuinfo access but would still fail this test, but given the above I don't think that matters nor is worth complicating the code further (plus it'd be sort of weird to lock things down with open_basedir yet then open up all of /proc IMO, but I've seen loads of weird things in the wild hah).

[taziobaker]

Could this work?:

public static function getHardwareConcurrency(): int {
static $width;
if (!isset($width)) {
if (function_exists('nproc')) {
// 'nproc' function is available, use it to determine the hardware concurrency
$width = (int) nproc();
} elseif (strtoupper(substr(PHP_OS, 0, 3)) === 'LIN') {
// Use 'sysctl' to query the number of CPU cores on Linux
$result = shell_exec('sysctl -n hw.ncpu');
if ($result !== null) {
$width = (int) trim($result);
} else {
$width = 0; // Handle the case when 'sysctl' is not available or fails.
}
} else {
// Handle other platforms or cases
$width = 0;
}
}
return $width;
}

This first checks if the nproc function is available. If it is, it uses nproc to determine the hardware concurrency. If nproc is not available and the operating system is Linux, it uses the sysctl approach to query the number of CPU cores. If neither method is available or if the code is running on a platform other than Linux, it sets the hardware concurrency to 0.

Just my ¢2

[szaimen]

/backport to stable27