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

Update InitialAvatar.php #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pobegov
Copy link

@pobegov pobegov commented Nov 30, 2022

Discovered when porting an existing project to PHP 8.1.

It seems that with open_basedir restrictions set, a call to file_exists() to any path outside of the paths set in open_basedir setting, a warning will be thrown. Which is not very clean.

also is there a linux/unix box where there is a directory on root level called /fonts/? I do not know any... so commenting this case out :)

@pobegov
Copy link
Author

pobegov commented Jan 23, 2023

Hm... "continuous-integration/styleci/pr — Issues have been identified with 1 file" is funny, shows me a diff of the whole file as if I have modified it's styling. Which is not true, you can see on the only commit that I made ... I have only added two lines of code. Something is fishy somewhere

@LasseRafn
Copy link
Owner

Hi, thanks for this!

Where is "/fonts/" called?

I only see it with current directory appended (DIR), or dependent on set value.

I'll move to 8.1 soon, so will test it out and see what happens -- and try with this PR as well

You can ignore the CI part 👍🏻 I should be able to ask it to format code before merge.

@pobegov
Copy link
Author

pobegov commented Jan 23, 2023

In the latest version which I've got from composer, in the file

src/InitialAvatar.php

on line 677 there is the following check:

if ( file_exists( $fontFile ) ) { ... }

which is then followed by two further checks prepending the DIR and also a slash

if ( file_exists( __DIR__ . $fontFile ) ) { ... }
if ( file_exists( __DIR__ . '/' . $fontFile ) ) { ... }

the first check which attempts to see if the font-file exists actually looks in a directory relative to "/", which was totally fine until PHP 8.1, probably open_basedir was not throwing a warning if it could not check a location and returned false instead. The change in behaviour which came with PHP 8.1 always yields a warning. I have taken the liberty to remove the check completely, works like charm in my project with over 100K users.

Thank you for a great library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants