[HttpKernel] Fix StreamedResponse with chunks support in HttpKernelBrowser #62036
Conversation
There was a problem hiding this comment.
Looks good to me.
I revised my mind because the failing tests are due to this PR.
- The
ob_startcallback shall return a string but here nothing is returned. - Also, in case of an exception the buffer is still open.
The current behavious is not perfect because of the last bullet.
A possible solution:
$content = '';
ob_start(static function (string $chunk, int $phase): string {
return $chunk;
});
try {
$response->sendContent();
$buffered = ob_get_contents();
} finally {
ob_end_clean();
}
$final = $buffered !== false ? $buffered : $content;@mtarld any thoughts on this PR?
wuchen90
force-pushed
the
fix-http-kernel-browser
branch
2 times, most recently
from
d9ab24c to
7bfcc2c
Compare
wuchen90
force-pushed
the
fix-http-kernel-browser
branch
from
7bfcc2c to
4c27082
Compare
wuchen90
force-pushed
the
fix-http-kernel-browser
branch
from
4c27082 to
fd8fb64
Compare
nicolas-grekas
force-pushed
the
fix-http-kernel-browser
branch
from
fd8fb64 to
2932941
Compare
|
Thank you @wuchen90. |
|
Hi @nicolas-grekas, As mentioned in #62036 (review), ob_start callback is expected to return a |
|
I fixed this concern in bfe4671 |
|
I missed it. Thank you for the clarification. |
|
This change broke our tests, we loosely have a test like that ob_start();
$client->request('GET', "/photos/$slug");
$content = ob_get_clean();With this refactor the $content is empty. I don't know if the iterator should be rewinded. |
5 participants
This PR fixes changes done in #60262 that breaks
HttpKernelBrowserwhen it comes to deal withStreamedResponsecreated with an iterator.