Try to create config directories if non existant #3115
Conversation
The patch will allow dompdf to try creating tmpDir, fontDir and fontCache if they do not exist on the filesystem - and will trigger a warning if they can't be created. Even if the Options.php file mention that pathes should exist, a number of people will set the options and won't even see that the folders weren't created. Trying to create them by default will : - Avoid bugs / lack of performances when directories are created - Will warn the user that his configurations are not suitable if necessary. Later, it might also be possible to change the trigger_error to an exception, but right now, it would be a BC break.
|
This seems reasonable. |
There was a problem hiding this comment.
Thinking on this again, I wonder if we just require that the user create the necessary directories prior to setting them in Dompdf. The main reason being that if we really want to do this we should probably allow the user to specify the permissions, which complicates things a bit. We could, instead of making the directory, just throw an exception if the specified directory is invalid so that the user knows they forgot to do something.
I'm still open to this change, just trying to think through potential unintended consequences.
| @@ -848,6 +848,9 @@ public function getDpi() | |||
| */ | |||
| public function setFontCache($fontCache) | |||
| { | |||
| if (!is_dir($fontCache) && !mkdir($fontCache)) { | |||
There was a problem hiding this comment.
Should we make the directory recursively (e.g., mkdir($dir, 0777, true))?
There was a problem hiding this comment.
Yep, that would be definitely better, even if the 777 permission might be problematic somehow
Yep, I agree - however, I didn't want to have a BC break by preventing people to run if the folder doesn't exist. Might it be a simple warning message for the next version, and a break on the next one? |
The patch will allow dompdf to try creating tmpDir, fontDir and fontCache if they do not exist on the filesystem - and will trigger a warning if they can't be created.
Even if the Options.php file mention that pathes should exist, a number of people will set the options and won't even see that the folders weren't created.
Trying to create them by default will :
Later, it might also be possible to change the trigger_error to an exception, but right now, it would be a BC break.