Skip to content
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.

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

dev/core#2927 - Avoid flooding logs with open_basedir in effect #22277

Merged
merged 1 commit into from
Feb 16, 2022

Conversation

demeritcowboy
Copy link
Contributor

Overview

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

Followup to #22107 and its inspirations

Before

Lots of log entries about open_basedir when e.g. visiting api explorer

After

Less

Technical Details

Comments

@agileware-justin @herbdool These are the only ones that seem to try to access directories that would be outside the civi tree or extension folders or places where files are kept. Such places would need to be inside open_basedir for civi to function in the first place, so leaving them at is_dir seems better.

@civibot
Copy link

civibot bot commented Dec 19, 2021

(Standard links)

@civibot civibot bot added the master label Dec 19, 2021
@demeritcowboy
Copy link
Contributor Author

@agileware-justin Do you have time to test this PR on the site(s) where you had the problem?

@agileware-justin
Copy link
Contributor

agileware-justin commented Feb 2, 2022

Thanks @demeritcowboy Sorry, I didn't see this PR.

We have these PRs in our build and that's been deployed.
#21591
#22107

Possibly, looks like we need to include getEntityNames and loadEntity in this PR.

I'll review this one too.

@demeritcowboy
Copy link
Contributor Author

Yep those 2 funcs are in this PR.

@agileware-justin
Copy link
Contributor

Testing looks good so far @demeritcowboy - no more spam.

@agileware-justin
Copy link
Contributor

Have not seen any changes to the amount of log spamming after removing #21591 and adding this PR. So things are still relatively quiet in the web server log, which is good.

And I'm not seeing any new errors in CiviCRM. Just all the old ones. So that's good too.

Tentative thumbs up.

@mattwire mattwire merged commit 8975e3e into civicrm:master Feb 16, 2022
@demeritcowboy
Copy link
Contributor Author

Thanks!

@demeritcowboy demeritcowboy deleted the isdir2 branch February 16, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants