Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Filesystem] Strengthen the check of file permissions indumpFile#54471

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

Merged
nicolas-grekas merged 1 commit intosymfony:5.4fromalexandre-daubois:fs-guard
Apr 4, 2024

Conversation

@alexandre-daubois
Copy link
Member

QA
Branch?5.4
Bug fix?yes
New feature?no
Deprecations?no
IssuesFix#54444
LicenseMIT

fileperms() can fail and returnfalse, I think we should strengthen the checks on its return value when using it to avoid undesirable behavior.

ebuildy reacted with heart emoji
@nicolas-grekas
Copy link
Member

Thank you@alexandre-daubois.

@nicolas-grekasnicolas-grekas merged commite483eee intosymfony:5.4Apr 4, 2024
@ebuildy
Copy link
Contributor

Fantastic this is merged ! Thanks you guys

alexandre-daubois reacted with rocket emoji

This was referencedApr 29, 2024
@oojacoboo
Copy link

So, somehow this change has broken our container caching layer. Looked into it a bit and I'm not even seeing any permissions issues with the actual files being dumped/written. They have user write permissions (644) and the proper user ownership.

fileperms(): stat failed for /tmp/cache/apiDevProjectContainer.php in /srv/www/vendor/symfony/filesystem/Filesystem.php:689 ErrorException called

@acoulton
Copy link
Contributor

@oojacoboo we're getting that too - I assume you have a custom error handler registered and it's intercepting the (expected) error generated when writing a non-existent file. I've proposed a fix in#54878

@oojacoboo
Copy link

@acoulton yep, custom error handler. Not quite sure why that makes a difference though. Is Symfony framework just ignoring it?

@xabbuh
Copy link
Member

@oojacoboo Your custom error handler does not seem to honour if an error is silenced.

oojacoboo reacted with thumbs up emoji

@oojacoboo
Copy link

@xabbuh thanks. We actually had error suppression handled. PHP8 changed the return onerror_reporting, and it now returns the value of a bitwise operation.

https://www.php.net/manual/en/language.operators.errorcontrol.php

Was able to get it resolved. Thanks for the tip.

xabbuh reacted with thumbs up emoji

nicolas-grekas pushed a commit that referenced this pull requestMay 16, 2024
Since#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.
nicolas-grekas added a commit that referenced this pull requestMay 16, 2024
…om handler (acoulton)This PR was merged into the 5.4 branch.Discussion----------[Filesystem] Fix dumpFile `stat failed` error hitting custom handler| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | See below| License       | MITSince#54471, dumpFile will trigger a `fileperms(): stat failed` error 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::box` to catch and ignore the potential error so that it never leaks outside this class.Commits-------247182a [Filesystem] Fix dumpFile `stat failed` error hitting custom handler
@shakaran
Copy link
Contributor

Related#57077 for 7.0.7

MindfulPol pushed a commit to MindfulPol/symfony that referenced this pull requestJun 1, 2024
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.
github-merge-queuebot pushed a commit to Lendable/composer-license-checker that referenced this pull requestJun 3, 2024
[![MendRenovate](https://app.renovatebot.com/images/banner.svg)](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`|[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2ffilesystem/6.4.8?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2ffilesystem/6.4.8?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2ffilesystem/6.4.6/6.4.8?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2ffilesystem/6.4.6/6.4.8?slim=true)](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(@&#8203;acoulton)- bug[symfony/symfony#54759](https://togithub.com/symfony/symfony/issues/54759)\[Filesystem] better distinguish URL schemes and Windows drive letters([@&#8203;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`(@&#8203;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>
symfonyaml pushed a commit to symfonyaml/symfony that referenced this pull requestOct 21, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

+1 more reviewer

@rosierrosierrosier left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

9 participants

@alexandre-daubois@nicolas-grekas@ebuildy@oojacoboo@acoulton@xabbuh@shakaran@rosier@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp