Issue 2927: create a method for checking if filename is a directory and also checking open_basedir

[herbdool]

Fixes https://lab.civicrm.org/dev/core/-/issues/2927

Overview

Create a method for checking if filename is a directory and also checking open_basedir. Use it instead of is_dir() which throws errors if not within the open_basedir restrictions.

Alternative to #21591

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@agileware-justin do you have any comments on this given you seem to have been the one to hit the most openbase_dir issues

[agileware-justin]

Thanks @seamuslee001

My initial thought is should all instances of is_dir be replaced, as indicated by #21592

[herbdool]

@seamuslee001 @agileware-justin I agree with @demeritcowboy in this comment #21592 (comment) where they mention an attempt to remove the error suppression throughout the code. The PR #21592 is adding them, so it seems to be at cross-purposes. I took the approach suggested by "Another option 3 might be a wrapper function for is_dir that checks if open_basedir is being used."

[demeritcowboy]

Probably most of them are ok to replace, especially if they are doing something like hunting for a file, but it depends on the specific instance. For example this one should output the warning about open_basedir, since otherwise you have to hunt harder to figure out the problem.

civicrm-core/CRM/Extension/Container/Basic.php

Lines 105 to 109 in b822332

	if (empty($this->baseDir) || !is_dir($this->baseDir)) {
	$errors[] = [
	'title' => ts('Invalid Base Directory'),
	'message' => ts('An extension container has been defined with a blank directory.'),
	];

.

[demeritcowboy]

I'm willing to help out writing a test for this, since if replacing a core php function it should be unit-tested to act as close to is_dir as possible.

And I know I said to use strncasecmp() but now it technically can return the wrong thing on unix. I'm thinking now something like:

$compareFunc = 'strncmp';
if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
  $compareFunc = 'strncasecmp';
}
if ($compareFunc($fileName, $baseDir ...etc...

[herbdool]

@demeritcowboy thanks for your help. What do you think of my thought here https://lab.civicrm.org/dev/core/-/issues/2927#note_66686

[demeritcowboy]

This should be filePath with a capital P.

I ran the tests against this and this function would need to deal with paths that have .. or . in them, because this algorithm doesn't quite work with them. Using realpath() might work. There is an existing function CRM_Utils_File::isChildPath() but it doesn't work on windows (it only seems to be used lightly for the extension manager).

[herbdool]

Closing in favour of #22107