don't load directories #194
Conversation
due to some filesystems nature ( proemintantly VIP ), require_once was trying to load a directory I'm not sure why $abs_path was a directory, albeit from seeing in the code, the redux autoload class puts $abs_path as /uploads/ + /redux/version , so it might start from there, while this is a temporary fix for wp-admin not to break in some instances, i'm not sure if other issues are present in the plugin.
|
related to #190 |
|
@mohammedeisa can you test if this fix doesn't have an impact on other items? |
|
Dovy asked me to peek that this. So, here's the problem. This function is a fallback for get_contents in case wp_filesystem fails. Adding the regex to look for only .php files could, potentially, lock out anyone attempting to read file contents other than a PHP file. I would suggest adding 'is_file' to that if statement first and see if that solves the problem. |
Signed-off-by: Kevin Provance <[email protected]>
|
@lazardanlucian We've merged a fix into the repo here. Would you mind testing it quickly to see if it works for you on VIP? |
|
file_exists will always return true for extension-free values (e.g
directory names).
https://docs.wpvip.com/technical-references/vip-go-files-system/media-uploads/
Why would you 'require' any other file than php? require/include will parse php code.
If you really really feel like you don't know what you're doing and need to allow any files, maybe use
include/include_once which will not break;
require is identical to include except upon failure it will also produce a fatal E_COMPILE_ERROR level error. In other words, it will halt the script whereas include only emits a warning (E_WARNING) which allows the script to continue.
https://www.php.net/manual/en/function.require.php
https://www.php.net/manual/en/function.include.php
After my coffe I see you used is_file,
Something weird is happening, I tested it now with your fix, and without my fix, both seem to work now,
This might have more implications, like if the directory exists already ? i'm not sure how to generate one since we don't have access to the f.s.
But my point still stands with the require:
1 Why would you require other files than php ?
2 If the files are optional and might be broken, shouldn't you use include instead (which doesn't error out, skips bad files)
|
|
actually i see now that you want to use is_file, I will do a quick test, although reading on the subject, I edited my email response, sorry for my foggy mind, |
|
@lazardanlucian There are situations where people will want to require another type of file and read it into a buffer. Like with the raw field. If you could test it, I'd appreciate it. |
due to some filesystems nature ( proemintantly VIP ),
require_once was trying to load a directory
I'm not sure why $abs_path was a directory,
albeit from seeing in the code, the redux autoload class puts $abs_path as /uploads/ + /redux/version , so it might start from there,
while this is a temporary fix for wp-admin not to break in some instances, i'm not sure if other issues are present in the plugin.