Skip to content
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

[8.x] Add flush handler to output buffer for streamed test response (bugfix) #42481

Merged
merged 2 commits into from
May 24, 2022

Conversation

nagmat84
Copy link
Contributor

This PR fixes a bug which may lead to missed content for a streamed test response.

Typically, code which wants to ensure that its output is really streamed to browser contains code lines like these

if (ob_get_level() > 0) {
  ob_flush();
}
flush();

This is the recommended best practice to ensure that all buffers are flushed and the content is sent to the browser:

  • PHP Manual:

    This means ob_flush() should be called before flush() to flush the output buffers if they are in use.

  • Symfony Manual:

    The flush() function does not flush buffering. If ob_start() has been called before or the output_buffering php.ini option is enabled, you must call ob_flush() before flush().

The current implementation of the test response does not work if one follows this practice. In particular, ob_flush sends all content of the output buffer to stdout in test mode and clears the buffer. The method ob_get_clean() as used by the current implementation will always return the empty string. This means TestResponse misses all output.

The correct way is to register a handler with ob_start which is called whenever the buffer is flushed. PHP Manual:

An optional callback function may be specified. This function takes a string as a parameter and should return a string. The function will be called when the output buffer is flushed (sent) or cleaned (with ob_flush(), [...] or similar function) or when the output buffer is flushed to the browser at the end of the request. [...]. When callback is called, it will receive the contents of the output buffer as its parameter and is expected to return a new output buffer as a result, which will be sent to the browser.

Note, the bugfix always returns the empty string. During tests there is on browser and the output would be sent to stdout, if we were not returning the empty string.

@nagmat84 nagmat84 changed the title Add flush handler to output buffer for streamed test response [8.x] Add flush handler to output buffer for streamed test response (bugfix) May 23, 2022
@nagmat84
Copy link
Contributor Author

Fixes #42482

@nagmat84
Copy link
Contributor Author

The bugfix also needs to be merged into the 9.x branch.

@driesvints
Copy link
Member

@nagmat84 we merge 8.x changes into 9.x 👍

@taylorotwell taylorotwell merged commit 76e0d60 into laravel:8.x May 24, 2022
@nagmat84 nagmat84 deleted the fix-streamed-test-response branch May 24, 2022 17:41
slavarazum pushed a commit to qruto/laravel-wave that referenced this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants