Add sniff to warn against using output buffering

[jrfnl]

Output buffering in the context of WP is notoriously problematic.

In PHP any number of output buffers can be created. These will be nested in the order in which they are opened and any call to close/flush an output buffer will close the inner most output buffer - or depending on the function called - close all buffers.

Also, whether output buffering will work at all, let alone how, is severely impacted by the values of the related ini directives of the system the code is run on, which in most cases in the context of WP is outside of the control of whoever writes the code.

Typical problems (simplified):

Case 1

Plugin A and B both hook their functions to arbitrary WordPress hooks.
Plugin A opens an output buffer, plugin B opens an output buffer.
Plugin A tries to close their output buffer before plugin B has closed theirs.
Both plugins receive the buffer expected by the other plugin.

Case 2

Output buffering is off system-wide (via the ini directives) and the ini directives are not checked before using the output buffering functions.

Case 3

Plugin A and B both hook their functions to arbitrary WordPress hooks.
Plugin A opens an output buffer, plugin B opens an output buffer.
Plugin A closes output buffering using ob_end_flush() or ob_end_clean(), destroying the output buffers of all other plugins/themes using output buffering as well.

Etc.

Sniffing for this

I believe it would be a good idea to have a sniff check for usage of the output buffering functions.

This could be done simply by adding a function list somewhere, though a more intelligent solution would be preferred.

Rough line of thinking (needs refinement):

• Always error when ob_end_flush() or ob_end_clean() are used.
• If ob_start() is called within the scope of a function and the buffer is closed within the same scope and no hooks are called in between (i.e. no WP functions are called as most will offer hooks), it's probably ok.
• If output buffering functions are used in combination with level checks, it's probably ok.
• Warn in all other circumstances.

The sniff should (initially) be added to the Extra ruleset.

Ref: http://php.net/manual/en/book.outcontrol.php

[benvaassen]

Two examples of why this would be helpful can be found here:

• Yoast SEO Meta Box blank  Yoast/wordpress-seo#10446
• Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers in  Yoast/wordpress-seo#10592

[jrfnl]

The issue described in both the above links: Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers.

This issue happens when a plugin/theme opens an output buffer with a preset callback, i.e. ob_start( $output_callback ) and the callback causes WP/plugin/theme functionality to be triggered which in turn uses output buffers.

Or as the PHP manual states:

ob_end_clean(), ob_end_flush(), ob_clean(), ob_flush() and ob_start() may not be called from a callback function. If you call them from callback function, the behavior is undefined. If you would like to delete the contents of a buffer, return "" (a null string) from callback function. You can't even call functions using the output buffering functions like print_r($expression, true) or highlight_file($filename, true) from a callback function.

Ref: http://php.net/manual/en/function.ob-start.php

As it is neigh impossible to determine - without extensive backtrace analysis - whether some code is being run in the context of an output buffering callback function, this basically prevents any other plugin from also using output buffering and yields white screens of death for the end user.

The only way around this issue (for other plugins wanting to use OB) is PHP 7.0+ only as there the fatal error has become an E_RECOVERABLE_ERROR which could be caught with a try/catch construct.

IMHO, the sniff should detect calling ob_start() with the first parameter set and should for the above reasons report this as an error.