Avoid possibility of ob_start() being called during output buffer processing (to avoid fatal error)

[westonruter]

Bug Description

In a support topic it was discovered that the AmpSchemaOrgMetadata transformer (and specifically AmpSchemaOrgMetadataConfiguration) can cause a fatal error:

PHP Fatal error:  ob_start(): Cannot use output buffering in output buffering display handlers in /app/public/content/plugins/gutenberg/build/block-library/blocks/post-comments-form.php on line 26

The issue is that the the transformer configurations are obtained during the output buffer callback, and so if there is any code that tries to do ob_start() in any WordPress code that applies the amp_schemaorg_metadata filter then this error will occur. For example, in Gutenberg the Post Comments Form block uses output buffering in its render_callback, so the following plugin code causes a fatal error on AMP pages:

add_filter( 'amp_schemaorg_metadata', function ( $metadata ) {
	$metadata['rendered_post_comments_form'] = do_blocks( '<!-- wp:post-comments-form /-->' );
	return $metadata;
} );

Or more simply without the Gutenberg dependency:

add_filter( 'amp_schemaorg_metadata', function ( $metadata ) {
	ob_start();
	echo 'Hello!';
	$metadata['buffered'] = ob_get_clean();
	return $metadata;
} );

This causes the fatal error.

Expected Behaviour

No call to ob_start() should be possible during AMP_Theme_Support::prepare_response().

Steps to reproduce

0. Activate the latest Gutenberg.
1. Add the above PHP code to a plugin.
2. View an AMP page.
3. See fatal error.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[schlessera]

No call to ob_start() should be possible during AMP_Theme_Support::prepare_response().

Not yet sure whether this is technically possible, but another way to solve this would be to exit the output buffering callback as soon as possible and defer the processing to another method outside of that callback.

What happens if you add another shutdown handler while within the output buffering callback?

[westonruter]

Humm, that does work at first glance: #6234. However, I am afraid this approach would cause big problems with caching plugins that rely on the content to be returned by the output buffer callback. I expect with this approach the caching plugins would just end up storing empty strings in the cache, instead of storing the processed page since it would be processed after output buffering has finished. At least, it will require much more testing than is suitable for a patch release.

[westonruter]

Note that a user tested the build and found it fixed the issue: https://wordpress.org/support/topic/gutenberg-10-6-0-amp-2-1-0-fatal-error/#post-14437465

[westonruter]

QA Passed

I added this to a plugin:

add_filter( 'amp_schemaorg_metadata', function ( $metadata ) {
	ob_start();
	echo 'Hello!';
	$metadata['buffered'] = ob_get_clean();
	return $metadata;
} );

When accessing an AMP page, there was no fatal error. The buffered key was added to the Schema.org metadata:

<script type="application/ld+json">{"@context":"http:\/\/schema.org","publisher":{"@type":"Organization","name":"WordPress Stable","logo":{"@type":"ImageObject","url":"https:\/\/wordpress-stable.lndo.site\/wp-content\/uploads\/2021\/04\/amp-wp-logo.png"}},"@type":"BlogPosting","mainEntityOfPage":"https:\/\/wordpress-stable.lndo.site\/2021\/04\/25\/image\/","headline":"Bison","datePublished":"2021-04-25T04:49:01+00:00","dateModified":"2021-04-25T04:50:07+00:00","author":{"@type":"Person","name":"stableadmin"},"image":{"@type":"ImageObject","url":"https:\/\/wordpress-stable.lndo.site\/wp-content\/uploads\/2021\/04\/American_bison_k5680-1-1200x783-1.jpg","width":1200,"height":783},"buffered":"Hello!"}</script>