dev/core#2927 - Avoid warnings for is_dir() when open_basedir is in effect

[demeritcowboy]

Overview

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

Alternate approach to #21923 and #21592

There are situations where you don't have control over open_basedir, and civi will sometimes iterate over folders trying to find something and will hit ones outside open_basedir. This produces lots of notices. This approach uses a local error-handler to silence only messages about open_basedir while still keeping all the functionality of the real is_dir.

Before

1. Set open_basedir to e.g. /var/www (something that contains enough to have all the files civi would normally access). Often include_path will include pear, which is usually outside that, so will suffice for testing purposes.
2. Restart web server.
3. For testing purposes visit /civicrm/api3.
4. Lots of notices on screen, assuming drupal 7 and you have error_reporting and drupal set to display php errors.

After

With this PR, you'll still get one notice because for demonstration purposes I've only replaced is_dir() in one file, but you won't see the other notices.

Technical Details

I believe this has an advantage over the previous approaches because (a) it only silences open_basedir, and (b) it doesn't require trying to duplicate some of the core php functionality.

Comments

Has test, which is largely based on php's own tests for is_dir. This should ensure that if somebody changes the new isDir function it should still conform to the same results as the real is_dir() except of course complaining about open_basedir.

As I've noted in the function comment, there are times when you DO want to know about this error, because you're expecting that the folder either should exist or be usable, and so it suggests something else is wrong and without the error it might be harder to track down or know something is wrong. So this function shouldn't be used everywhere is_dir() is used, just in places where you know it might legitimately be reaching outside open_basedir.

[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]

Jenkins re test this please

[herbdool]

This makes sense to me to use this approach.

A follow-up would be to replace some calls to is_dir where this would be better.

[demeritcowboy]

Thanks. I'll clean up and rebase.

[totten]

Should restore_error_handler(); be inside a finally{} block?

[demeritcowboy]

Finally I get to use one of those. Yes that's a good idea.

[totten]

Very thorough test cases!

Merging.

[demeritcowboy]

Thanks!