[Process] Workaround buggy PHP warning #16182
Conversation
This temporary workaround is intended to address an uncaught exception that may occur, until https://bugs.php.net/bug.php?id=69240 is fixed. Obviously the `@` operator is another option, but I'm erring towards the notion that the Symfony authors/maintainers may wish to implement more elegant exception-handling logic than simply using `@`.
|
catching ErrorException is the wrong implementation. Warnings are not turned into exceptions in production |
| } | ||
| } | ||
| catch (\ErrorException $e) { |
There was a problem hiding this comment.
who triggers the exception?
There was a problem hiding this comment.
is_dir, see the comment above.
There was a problem hiding this comment.
is_dir triggers warnings, not exceptions. A custom error handler must be registered for an exception to be thrown.
The implementation with an @ is required (same as @stof comments)
There was a problem hiding this comment.
@nicolas-grekas I replaced the try/catch with an @.
The relevant stack frame contains HandleExceptions->handleError('2', 'is_dir(): open_basedir restriction in effect. File(/srv/www/example.com/web) is not within the allowed path(s) ... where HandleExceptions is an instance of Illuminate\Foundation\Bootstrap\HandleExceptions.
|
@stof Okay; my apologies. I am not at all familiar with the Symfony code-base, nor the methodologies that it employs. As much as I would love to read-up and offer a more appropriate solution, I simply lack the time. Perhaps you or another contributor is able to discern the problem and fix it correctly. @nicolas-grekas From what I'm able to determine, PHP's |
There you go.
| @@ -56,7 +56,7 @@ public function find($name, $default = null, array $extraDirs = array()) | |||
| $searchPath = explode(PATH_SEPARATOR, ini_get('open_basedir')); | |||
| $dirs = array(); | |||
| foreach ($searchPath as $path) { | |||
| if (is_dir($path)) { | |||
| if (@is_dir($path)) { | |||
There was a problem hiding this comment.
LGTM, can you just add a comment like this one please?
// Silencing against https://bugs.php.net/69240
There was a problem hiding this comment.
Done. Please let me know if you had something different in mind. Thanks!
|
👍 thanks |
|
👍 |
|
Thanks for the quick turnaround! 🍻 |
|
Thank you @cbj4074. |
This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #16182). Discussion ---------- [Process] Workaround buggy PHP warning | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This temporary workaround is intended to address an uncaught exception that may occur, until https://bugs.php.net/bug.php?id=69240 is fixed. Obviously the `@` operator is another option, but I'm erring towards the notion that the Symfony authors/maintainers may wish to implement more elegant exception-handling logic than simply using `@`. Commits ------- b1bd093 [Process] Workaround buggy PHP warning
… (cbj4074) This PR was merged into the 2.7 branch. Discussion ---------- [Process] Suppress warnings when open_basedir is non-empty | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | If PHP is configured *with a non-empty open_basedir* value that does not permit access to the target location, these calls to is_executable() throw warnings. While Symfony may not raise exceptions for warnings in production environments, other frameworks (such as Laravel) do, in which case any of these checks causes a show-stopping 500 error. We fixed a similar issue in the ExecutableFinder class via #16182 . This has always been an issue, but symfony/process@709e15e made it more likely that a warning is triggered. Commits ------- 34f136e Suppress warnings when open_basedir is non-empty
If PHP is configured *with a non-empty open_basedir* value that does not permit access to the target location, these calls to is_executable() throw warnings. While Symfony may not raise exceptions for warnings in production environments, other frameworks (such as Laravel) do, in which case any of these checks causes a show-stopping 500 error. We fixed a similar issue in the ExecutableFinder class via symfony/symfony#16182 . This has always been an issue, but 709e15e made it more likely that a warning is triggered.
… (cbj4074) This PR was merged into the 2.7 branch. Discussion ---------- [Process] Suppress warnings when open_basedir is non-empty | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | If PHP is configured *with a non-empty open_basedir* value that does not permit access to the target location, these calls to is_executable() throw warnings. While Symfony may not raise exceptions for warnings in production environments, other frameworks (such as Laravel) do, in which case any of these checks causes a show-stopping 500 error. We fixed a similar issue in the ExecutableFinder class via symfony/symfony#16182 . This has always been an issue, but 709e15e made it more likely that a warning is triggered. Commits ------- 34f136e01b Suppress warnings when open_basedir is non-empty
This temporary workaround is intended to address an uncaught exception that may occur, until https://bugs.php.net/bug.php?id=69240 is fixed.
Obviously the
@operator is another option, but I'm erring towards the notion that the Symfony authors/maintainers may wish to implement more elegant exception-handling logic than simply using@.