Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] Fix BinaryFileResponse content type detection logic#47746
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
[HttpFoundation] Fix BinaryFileResponse content type detection logic#47746
Uh oh!
There was an error while loading.Please reload this page.
Conversation
2555feb to87f58aeComparefabpot commentedSep 30, 2022
Thank you@X-Coder264. |
junowilderness commentedOct 3, 2022
@X-Coder264 Our automated tests detected this one also. You found the root cause faster than I did. Thank you. |
alcohol commentedOct 4, 2022
Heh just ran into this one today when upgrading from 4.4.45 to 4.4.46; added a conflict rule for now to our composer.json - cheers for the quick fix! |
Trismegiste commentedOct 5, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Same problem here with an upgrade from5.4.11 to5.4.13 With this simple controller which should return a PNG : /** @Route("/yolo") */publicfunctionyolo() {returnnewBinaryFileResponse('/www/var/storage/test/yolo.png'); } The content-type of the picture is It works OK when the picture is in a html page (the browser is fixing the faulty content-type on the fly) but my functional tests do not pass. If I force the content-type with a Will this fix be released in 5.4.14 ? |
xabbuh commentedOct 6, 2022
yes |
gimler commentedOct 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Trismegiste i had the same problem quick fix manual add mime type |
Trismegiste commentedOct 6, 2022
Thank you@xabbuh
Thank you@gimler but I think I'll keep 5.4.11 and upgrade later |
derrabus commentedOct 6, 2022
… or use |
marin246 commentedOct 11, 2022
When will this be released? |
xabbuh commentedOct 11, 2022
Patch releases happen once per month. |
X-Coder264 commentedOct 11, 2022
Is there an exception to that? This is not some rare/small/obscure issue, this is a pretty big issue with a lot of impact as almost every project uses the Honestly I was a bit surprised that a new patch version was not released immediately after the fix was merged. |
xabbuh commentedOct 12, 2022
releases for all maintained versions containing this fixed have just been released |
Trismegiste commentedOct 12, 2022
Tested right now. |

Uh oh!
There was an error while loading.Please reload this page.
I've just upgraded to 5.4.13 (from 5.4.8) and a lot of my tests started failing. Upon further debugging I've found the issue to be caused by#47516
Prior to that PR
was called before the
parent::prepare($request);call.After that PR
parent::prepare($request);was called first and that would callwhich would set the content type from the request so by the time
is executed again in the BinaryFileResponse
preparemethod thatifstatement never evaluates totruebecause theContent-Typeheader was already set (to the wrong one).This PR fixes this issue by making sure the order of the method calls is correct.