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

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

Merged
fabpot merged 1 commit intosymfony:4.4frombradjones1:flush-after-close
Jul 16, 2022

Conversation

@bradjones1
Copy link
Contributor

@bradjones1bradjones1 commentedJul 13, 2022
edited
Loading

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

Currently,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, makingResponse::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.)

Flushes the system write buffers of PHP and whatever backend PHP is using (CGI, a web server, etc). This attempts to push current output all the way to the browser with a few caveats.

flush() may not be able to override the buffering scheme of your web server and it has no effect on any client-side buffering in the browser. It also doesn't affect PHP's userspace output buffering mechanism. This means ob_flush() should be called before flush() to flush the output buffers if they are in use.

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.

@carsonbotcarsonbot added this to the6.2 milestoneJul 13, 2022
@bradjones1bradjones1 changed the titleFlush backend output buffer after closing.[HttpFoundation] Flush backend output buffer after closing.Jul 13, 2022
@bradjones1
Copy link
ContributorAuthor

The PHP 8.2 test failure seems unrelated...

The Appveyor test seems to target Windows (?) and this is suspicious:

Symfony\Component\VarDumper\Tests\Server\ConnectionTest::testDumpSymfony\Component\Process\Exception\ProcessTimedOutException: The process "c:\php\php.exe" exceeded the timeout of 9 seconds.

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
Copy link

Hey!

I think@abdounikarim has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@fabpot
Copy link
Member

It looks like we should consider this as a bug, so this should probably be rebased on 4.4?

@bradjones1
Copy link
ContributorAuthor

Thanks for the review,@fabpot.

It looks like we should consider this as a bug, so this should probably be rebased on 4.4?

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.

@carsonbotcarsonbot changed the title[HttpFoundation] Flush backend output buffer after closing.Flush backend output buffer after closing.Jul 16, 2022
@fabpotfabpot modified the milestones:6.2,4.4Jul 16, 2022
@fabpot
Copy link
Member

Thank you@bradjones1.

nicolas-grekas added a commit that referenced this pull requestAug 30, 2022
…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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@lyrixxlyrixxAwaiting requested review from lyrixx

@ycerutoycerutoAwaiting requested review from yceruto

@wouterjwouterjAwaiting requested review from wouterj

@chalasrchalasrAwaiting requested review from chalasr

@dunglasdunglasAwaiting requested review from dunglas

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

3 participants

@bradjones1@carsonbot@fabpot

[8]ページ先頭

©2009-2025 Movatter.jp