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

[HttpFoundation] fix FileBag under PHP 8.1#42112

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
derrabus merged 1 commit intosymfony:4.4fromalexpott:files-full-path-php8.1
Jul 15, 2021

Conversation

@alexpott
Copy link
Contributor

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix #...
LicenseMIT
Doc PRsymfony/symfony-docs#...

Seehttps://php.watch/versions/8.1/$_FILES-full-path - in \Symfony\Component\HttpFoundation\FileBag::convertFileInformation() we check against a list of hardcoded keys. This logic breaks in PHP 8.1 because of the new key.

@alexpott
Copy link
ContributorAuthor

I don't think is the correct fix. I think we might want to get this new information into \Symfony\Component\HttpFoundation\File\UploadedFile - but the list of hardcoded keys is not that fun.

@nicolas-grekasnicolas-grekas changed the titleIn PHP 8.1 the $_FILE global has a new key which breaks FileBag[HttpFoundation] fix FileBag under PHP 8.1Jul 15, 2021
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedJul 15, 2021
edited
Loading

Maybe this is good enough for 4.4, and we need to improveUploadedFile in 5.4 to handlefull_path?
Up for a PR?

derrabus reacted with thumbs up emoji

@alexpott
Copy link
ContributorAuthor

@nicolas-grekas taking this approach in 4.4 seems reasonable. I've not got time atm for PR for 5.4. Working through well over 1000 fails for the Drupal test suite on PHP 8.1 :D

@carsonbotcarsonbot changed the title[HttpFoundation] fix FileBag under PHP 8.1fix FileBag under PHP 8.1Jul 15, 2021
@carsonbotcarsonbot changed the titlefix FileBag under PHP 8.1[HttpFoundation] fix FileBag under PHP 8.1Jul 15, 2021
@derrabusderrabusforce-pushed thefiles-full-path-php8.1 branch from426c0c8 todc35049CompareJuly 15, 2021 17:37
@derrabus
Copy link
Member

Thank you@alexpott.

@derrabusderrabus merged commit6cd50ad intosymfony:4.4Jul 15, 2021
This was referencedJul 26, 2021
fabpot added a commit that referenced this pull requestDec 4, 2023
…ath()` to support directory uploads (danielburger1337)This PR was merged into the 7.1 branch.Discussion----------[HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      |  no| New feature?  | yes| Deprecations? | no| License       | MITThis removes the "hotfix"#42112 and now supports "full_path" PHP file uploads.Since PHP 8.1 is now the lowest allowed version of PHP, this shouldnt cause any problems.---Although I'm quite certain that I understand the messy FileBag code, I'm very concerned for the potential impact this change might have because of the widespread use of http-foundation.To the best of my knowledge, this won't break Symfony BC (yet) because this is all done by an `optional` argument in the constructor of UploadedFile (this should eventually be replaced by a mandatory argument though).Also, I'm not quite happy with the tests. Since this is only a "passthrough" of the PHP value, this _shouldnt_ be a huge problem.Someone should definitly go through this change very carefully. The tests pass and I also threw some "real world" application tests at it and everything seems to work just fine. But because of the nature of HttpFoundation and its intangled mess, I can't guarantee that I accounted for every possible or even simple edge case.---Though it would be nice to still see this in 7.0, I know that it will probably be pushed back to 7.1.Commits-------dac592b [HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads
symfony-splitter pushed a commit to symfony/form that referenced this pull requestDec 4, 2023
…ath()` to support directory uploads (danielburger1337)This PR was merged into the 7.1 branch.Discussion----------[HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      |  no| New feature?  | yes| Deprecations? | no| License       | MITThis removes the "hotfix"symfony/symfony#42112 and now supports "full_path" PHP file uploads.Since PHP 8.1 is now the lowest allowed version of PHP, this shouldnt cause any problems.---Although I'm quite certain that I understand the messy FileBag code, I'm very concerned for the potential impact this change might have because of the widespread use of http-foundation.To the best of my knowledge, this won't break Symfony BC (yet) because this is all done by an `optional` argument in the constructor of UploadedFile (this should eventually be replaced by a mandatory argument though).Also, I'm not quite happy with the tests. Since this is only a "passthrough" of the PHP value, this _shouldnt_ be a huge problem.Someone should definitly go through this change very carefully. The tests pass and I also threw some "real world" application tests at it and everything seems to work just fine. But because of the nature of HttpFoundation and its intangled mess, I can't guarantee that I accounted for every possible or even simple edge case.---Though it would be nice to still see this in 7.0, I know that it will probably be pushed back to 7.1.Commits-------dac592b513 [HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@derrabusderrabusderrabus approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@alexpott@nicolas-grekas@derrabus@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp