-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. |
Hey! I think @abdounikarim has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
It looks like we should consider this as a bug, so this should probably be rebased on 4.4? |
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. |
5c484b7
to
d78b586
Compare
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 #46931 Commits ------- 118acea [HttpFoundation] move flushing outside of Response::closeOutputBuffers
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 beOn
by 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/framework wishes to perform additional work after sending the request but before shutting down.According to the PHP 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.