Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Flush backend output buffer after closing.#46931
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
bradjones1 commentedJul 13, 2022
The PHP 8.2 test failure seems unrelated... The Appveyor test seems to target Windows (?) and this is suspicious: I'll postpone digging too deeply into that until I get feedback on the general approach, though. If this is something Symfony will concern itself with, then we can address that. |
carsonbot commentedJul 14, 2022
Hey! I think@abdounikarim has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
fabpot commentedJul 15, 2022
It looks like we should consider this as a bug, so this should probably be rebased on 4.4? |
Uh oh!
There was an error while loading.Please reload this page.
bradjones1 commentedJul 15, 2022
Thanks for the review,@fabpot.
I wasn't sure if this was a purposeful omission or not (I erred on the side of yes, it was) so I can recategorize and retarget as a bug. |
fabpot commentedJul 16, 2022
Thank you@bradjones1. |
…utputBuffers (nicolas-grekas)This PR was merged into the 4.4 branch.Discussion----------[HttpFoundation] move flushing outside of Response::closeOutputBuffers| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Fix change introduced in#46931Commits-------118acea [HttpFoundation] move flushing outside of Response::closeOutputBuffers
Uh oh!
There was an error while loading.Please reload this page.
noyesCurrently,
Response::send()does some backend/server specific shutdown, with the default case being closing userland output buffers. PHP's output buffering may or may not beOnby default.I recently discovered in the course of debugging my Drupal 9 project that closing output buffers may not be enough to flush the response's content to the client. In my case, output buffering wasn't enabled at all, making
Response::closeOutputBuffers()a no-op. In either case, it appears necessary to also callflush(). This is important if the calling script/frameworkwishes to perform additional work after sending the request but before shutting down.According to thePHP docs for flush(), this seems to be necessary in addition to closing all active output buffers (if used.)
If it is Symfony's intent that the "final"
flush()call be done by the implementing code/extending framework, it would be good to explicitly note that.