Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Filesystem] Fix dumpFilestat failed error hitting custom handler#54878
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Sincesymfony#54471, dumpFile will trigger a `fileperms(): stat failed`error when writing to a filename that does not yet exist. Thiswas silenced from PHP's default handler with the `@` operator.However, the error is still passed to any custom handler that theapplication has registered, and can therefore cause exceptions orspurious logging depending on the implementation of the handler.The better solution, which is consistent with all other calls tonative functions in this class, would be to use `self::box` tocatch and ignore the potential error so that it never leaksoutside this class.
acoulton commentedMay 10, 2024
As far as I can see the coding standards and test failures are unrelated to the change in this branch - but please let me know if you want any changes. |
xabbuh commentedMay 10, 2024
The proposed solution works for me but:
This is still an error in the custom error handler if it doesn't respect the silencing of errors and should be fixed nonetheless as an error handler throwing no matter if the error was silenced can still throw too eagerly. |
acoulton commentedMay 13, 2024
That's probably true. At the same time, I don't think a library should be relying on the silencing operator to suppress errors that are emitted during expected usage of a method - in particular if it hasn't done so in the past, since this might break existing applications where the over-eager throw has not historically caused any problems. |
stat failed error hitting custom handlerstat failed error hitting custom handlerstat failed error hitting custom handlerstat failed error hitting custom handlernicolas-grekas commentedMay 16, 2024
Thank you@acoulton. |
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [symfony/filesystem](https://symfony.com)([source](https://togithub.com/symfony/filesystem)) | `6.4.6` -> `6.4.8`|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>symfony/filesystem (symfony/filesystem)</summary>###[`v6.4.8`](https://togithub.com/symfony/filesystem/releases/tag/v6.4.8)[CompareSource](https://togithub.com/symfony/filesystem/compare/v6.4.7...v6.4.8)**Changelog**(symfony/filesystem@v6.4.7...v6.4.8)- bug[symfony/symfony#54878](https://togithub.com/symfony/symfony/issues/54878)\[Filesystem] Fix dumpFile `stat failed` error hitting custom handler(@​acoulton)- bug[symfony/symfony#54759](https://togithub.com/symfony/symfony/issues/54759)\[Filesystem] better distinguish URL schemes and Windows drive letters([@​xabbuh](https://togithub.com/xabbuh))###[`v6.4.7`](https://togithub.com/symfony/filesystem/releases/tag/v6.4.7)[CompareSource](https://togithub.com/symfony/filesystem/compare/v6.4.6...v6.4.7)**Changelog**(symfony/filesystem@v6.4.6...v6.4.7)- bug[symfony/symfony#54471](https://togithub.com/symfony/symfony/issues/54471)\[Filesystem] Strengthen the check of file permissions in `dumpFile`(@​alexandre-daubois)</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Enabled.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/Lendable/composer-license-checker).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNzcuOCIsInVwZGF0ZWRJblZlciI6IjM3LjM3Ny44IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
%5d(https%3a%2f%2frenovatebot.com){kind=link}
Since#54471, dumpFile will trigger a
fileperms(): stat failederror when writing to a filename that does not yet exist. This was silenced from PHP's default handler with the@operator.However, the error is still passed to any custom handler that the application has registered, and can therefore cause exceptions or spurious logging depending on the implementation of the handler.
The better solution, which is consistent with all other calls to native functions in this class, would be to use
self::boxto catch and ignore the potential error so that it never leaks outside this class.