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

[Validator] Use Mime component to determine mime type for file validator#36868

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
fabpot merged 1 commit intosymfony:4.4frompierredup:validator-error-message
May 30, 2020

Conversation

@pierredup
Copy link
Contributor

@pierreduppierredup commentedMay 19, 2020•
edited
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsN/A
LicenseMIT
Doc PRN/A

When validating the mime type for a file, the Validator component relies on theSymfony\Component\HttpFoundation\File\File class, but if the HttpFoundation component is not installed, then you just get the error

PHP Fatal error:  Uncaught Error: Class 'Symfony\Component\HttpFoundation\File\File' not found

This PR uses the Mime component to get the mime type for a file and throws an exception if the Mime component is not installed.

@pierreduppierredup changed the titleThrow exception when validating file mime types and the HttpFoundation component is not installed[Validator] Throw exception when validating file mime types and the HttpFoundation component is not installedMay 19, 2020
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedMay 19, 2020•
edited
Loading

Now that the Mime component is standalone, can't we skip the dependency on HttpFoundation instead?

apfelbox reacted with thumbs up emoji

@pierreduppierredupforce-pushed thevalidator-error-message branch fromcdc74bf to4c06e60CompareMay 19, 2020 09:55
@pierreduppierredup changed the title[Validator] Throw exception when validating file mime types and the HttpFoundation component is not installed[Validator] Use Mime component to determine mime type for file validatorMay 19, 2020
@pierreduppierredupforce-pushed thevalidator-error-message branch from4c06e60 todf554cdCompareMay 19, 2020 09:58
@pierredup
Copy link
ContributorAuthor

@nicolas-grekas Done

@nicolas-grekas
Copy link
Member

What about considering it as a bugfix?

@pierreduppierredupforce-pushed thevalidator-error-message branch fromcf33702 to3577b28CompareMay 19, 2020 10:35
@pierredup
Copy link
ContributorAuthor

What about considering it as a bugfix?

I wasn't sure if it counted as a bugfix or new feature. When in doubt, I just consider something a new feature :)

But I'm not sure if a bugfix against 3.4 can use the Mime component, so should I create a new PR for 3.4 and have the original exception message forHttpFoundation, then just rebase and retarget 4.4 here?

@nicolas-grekas
Copy link
Member

I would suggest targeting 4.4 and forget about 3.4.

pierredup and apfelbox reacted with thumbs up emoji

@pierreduppierredupforce-pushed thevalidator-error-message branch from3577b28 to62ade6aCompareMay 19, 2020 10:41
@pierreduppierredup changed the base branch frommaster to4.4May 19, 2020 10:42
@nicolas-grekasnicolas-grekas added this to the4.4 milestoneMay 22, 2020
@pierreduppierredupforce-pushed thevalidator-error-message branch 4 times, most recently from84357d4 to05416a2CompareMay 29, 2020 10:57
@pierredup
Copy link
ContributorAuthor

Status: Ready for review

@nicolas-grekas
Copy link
Member

I pushed a CS change. The code looks good, but the tests need some love, seefailures on appveyor when the finfo extension is not enabled.

pierredup reacted with thumbs up emoji

@pierreduppierredupforce-pushed thevalidator-error-message branch 2 times, most recently from35fc85e to09dc302CompareMay 29, 2020 12:09
@pierreduppierredupforce-pushed thevalidator-error-message branch from09dc302 to4728833CompareMay 29, 2020 12:11
@pierredup
Copy link
ContributorAuthor

pierredup commentedMay 29, 2020•
edited
Loading

I reverted the changes to the tests to use mocks again, and just changed the ordering of the checks

@fabpot
Copy link
Member

Thank you@pierredup.

@fabpotfabpot merged commit5e0e9a8 intosymfony:4.4May 30, 2020
This was referencedMay 31, 2020
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

@fabpotfabpotfabpot approved these changes

@xabbuhxabbuhxabbuh approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@pierredup@nicolas-grekas@fabpot@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp