From b3ce59ec563b16d00407669812f075dbcab7c4c3 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Mon, 24 Jun 2024 13:23:05 -0700
Subject: [PATCH 01/12] Try considering PHP_OUTPUT_HANDLER_FINAL in output
 buffer callback

---
 .../optimization-detective/optimization.php   | 23 +++++-----
 .../tests/test-optimization.php               | 42 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index 442a7a6226..2e75924826 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -32,16 +32,19 @@
  */
 function od_buffer_output( string $passthrough ): string {
 	ob_start(
-		static function ( string $output ): string {
-			/**
-			 * Filters the template output buffer prior to sending to the client.
-			 *
-			 * @since 0.1.0
-			 *
-			 * @param string $output Output buffer.
-			 * @return string Filtered output buffer.
-			 */
-			return (string) apply_filters( 'od_template_output_buffer', $output );
+		static function ( string $output, ?int $phase ): string {
+			if ( $phase & PHP_OUTPUT_HANDLER_FINAL ) {
+				/**
+				 * Filters the template output buffer prior to sending to the client.
+				 *
+				 * @since 0.1.0
+				 *
+				 * @param string $output Output buffer.
+				 * @return string Filtered output buffer.
+				 */
+				$output = (string) apply_filters( 'od_template_output_buffer', $output );
+			}
+			return $output;
 		}
 	);
 	return $passthrough;
diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php
index ff62951a5c..af67949a05 100644
--- a/plugins/optimization-detective/tests/test-optimization.php
+++ b/plugins/optimization-detective/tests/test-optimization.php
@@ -54,10 +54,12 @@ public function test_od_buffer_output(): void {
 		// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
 		ob_start();
 
+		$filter_invoked = false;
 		add_filter(
 			'od_template_output_buffer',
-			function ( $buffer ) use ( $original, $expected ) {
+			function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
 				$this->assertSame( $original, $buffer );
+				$filter_invoked = true;
 				return $expected;
 			}
 		);
@@ -71,6 +73,44 @@ function ( $buffer ) use ( $original, $expected ) {
 
 		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer.
 		$this->assertSame( $expected, $buffer );
+		$this->assertTrue( $filter_invoked );
+	}
+
+	/**
+	 * Make output is buffered and that it is also filtered.
+	 *
+	 * @covers ::od_buffer_output
+	 */
+	public function test_od_buffer_output_not_finalized(): void {
+		$original = 'Hello My World!';
+		$override = 'Ciao mondo!';
+
+		// In order to test, a wrapping output buffer is required because ob_get_clean() does not invoke the output
+		// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
+		ob_start();
+
+		$filter_invoked = false;
+		add_filter(
+			'od_template_output_buffer',
+			function ( $buffer ) use ( $original, &$filter_invoked ) {
+				$this->assertSame( $original, $buffer );
+				$filter_invoked = true;
+				return '¡Hola Mi Mundo!';
+			}
+		);
+
+		$original_ob_level = ob_get_level();
+		od_buffer_output( '' );
+		$this->assertSame( $original_ob_level + 1, ob_get_level(), 'Expected call to ob_start().' );
+		echo $original;
+
+		ob_clean(); // Note the lack of flush here.
+		echo $override;
+
+		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer.
+
+		$this->assertFalse( $filter_invoked );
+		$this->assertSame( $override, $buffer );
 	}
 
 	/**

From 13e068d2673bad734dadf622a1fc3c1eb1d9736d Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Thu, 1 Aug 2024 18:18:41 -0700
Subject: [PATCH 02/12] Iterate on output buffer handling

---
 .../optimization-detective/optimization.php   |  2 +-
 .../tests/test-optimization.php               | 28 +++++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index 2e75924826..e3a60b1891 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -33,7 +33,7 @@
 function od_buffer_output( string $passthrough ): string {
 	ob_start(
 		static function ( string $output, ?int $phase ): string {
-			if ( $phase & PHP_OUTPUT_HANDLER_FINAL ) {
+			if ( ( $phase & PHP_OUTPUT_HANDLER_FINAL ) > 0 ) {
 				/**
 				 * Filters the template output buffer prior to sending to the client.
 				 *
diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php
index af67949a05..a1a621fac5 100644
--- a/plugins/optimization-detective/tests/test-optimization.php
+++ b/plugins/optimization-detective/tests/test-optimization.php
@@ -77,40 +77,44 @@ function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
 	}
 
 	/**
-	 * Make output is buffered and that it is also filtered.
+	 * Test that calling ob_clean() will discard previous buffer and never send it into the od_template_output_buffer filter.
 	 *
 	 * @covers ::od_buffer_output
 	 */
 	public function test_od_buffer_output_not_finalized(): void {
-		$original = 'Hello My World!';
-		$override = 'Ciao mondo!';
+		$original          = 'Hello My World!';
+		$template_override = 'Ciao mondo!';
+		$filter_override   = '¡Hola Mi Mundo!';
 
 		// In order to test, a wrapping output buffer is required because ob_get_clean() does not invoke the output
 		// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
 		ob_start();
 
-		$filter_invoked = false;
+		$filter_count = 0;
 		add_filter(
 			'od_template_output_buffer',
-			function ( $buffer ) use ( $original, &$filter_invoked ) {
-				$this->assertSame( $original, $buffer );
-				$filter_invoked = true;
-				return '¡Hola Mi Mundo!';
+			function ( $buffer ) use ( $template_override, $filter_override, &$filter_count ) {
+				$this->assertSame( $template_override, $buffer, 'Expected the original template output to never get passed into the buffer callback since ob_clean() was called after the original was printed.' );
+				$filter_count++;
+				return $filter_override;
 			}
 		);
 
 		$original_ob_level = ob_get_level();
 		od_buffer_output( '' );
 		$this->assertSame( $original_ob_level + 1, ob_get_level(), 'Expected call to ob_start().' );
-		echo $original;
+		echo $original; // This should never be passed into the od_template_output_buffer filter.
 
+		// Abort the original content printed above.
 		ob_clean(); // Note the lack of flush here.
-		echo $override;
+		echo $template_override; // This should get passed into the od_template_output_buffer filter.
 
 		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer.
 
-		$this->assertFalse( $filter_invoked );
-		$this->assertSame( $override, $buffer );
+		$this->assertSame( 1, $filter_count, 'Expected filter to be called once.' );
+		$this->assertSame( $filter_override, $buffer, 'Excepted return value of filter to be the resulting value for the buffer.' ); // TODO: The output buffer callback returns $filter_override, but the $buffer for some reason is actually equal to $template_override. Why?
+
+		ob_end_clean(); // Close the wrapper buffer.
 	}
 
 	/**

From 523fc7adbf191be14edb3a7f11ca5f082646fca8 Mon Sep 17 00:00:00 2001
From: Roland Soos <roland@nextendweb.com>
Date: Fri, 2 Aug 2024 07:40:13 +0200
Subject: [PATCH 03/12] Update test-optimization.php

---
 plugins/optimization-detective/tests/test-optimization.php | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php
index a1a621fac5..f212be8415 100644
--- a/plugins/optimization-detective/tests/test-optimization.php
+++ b/plugins/optimization-detective/tests/test-optimization.php
@@ -109,6 +109,8 @@ function ( $buffer ) use ( $template_override, $filter_override, &$filter_count
 		ob_clean(); // Note the lack of flush here.
 		echo $template_override; // This should get passed into the od_template_output_buffer filter.
 
+		ob_end_flush();
+
 		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer.
 
 		$this->assertSame( 1, $filter_count, 'Expected filter to be called once.' );

From 15ca2f0621285f9924a6d1e34655664bd6ccebe5 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Mon, 5 Aug 2024 17:30:17 -0700
Subject: [PATCH 04/12] Remove final ob_end_clean() and add ob_get_level()
 assertions

---
 .../tests/test-optimization.php                | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php
index f212be8415..4601cfc83d 100644
--- a/plugins/optimization-detective/tests/test-optimization.php
+++ b/plugins/optimization-detective/tests/test-optimization.php
@@ -88,7 +88,9 @@ public function test_od_buffer_output_not_finalized(): void {
 
 		// In order to test, a wrapping output buffer is required because ob_get_clean() does not invoke the output
 		// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
+		$initial_level = ob_get_level();
 		ob_start();
+		$this->assertSame( $initial_level + 1, ob_get_level() );
 
 		$filter_count = 0;
 		add_filter(
@@ -100,23 +102,23 @@ function ( $buffer ) use ( $template_override, $filter_override, &$filter_count
 			}
 		);
 
-		$original_ob_level = ob_get_level();
 		od_buffer_output( '' );
-		$this->assertSame( $original_ob_level + 1, ob_get_level(), 'Expected call to ob_start().' );
+		$this->assertSame( $initial_level + 2, ob_get_level() );
 		echo $original; // This should never be passed into the od_template_output_buffer filter.
 
 		// Abort the original content printed above.
-		ob_clean(); // Note the lack of flush here.
+		ob_clean(); // Note the lack of flush here and the lack of ending the buffer.
+		$this->assertSame( $initial_level + 2, ob_get_level() );
 		echo $template_override; // This should get passed into the od_template_output_buffer filter.
 
-		ob_end_flush();
+		ob_end_flush(); // Close the output buffer opened by od_buffer_output().
+		$this->assertSame( $initial_level + 1, ob_get_level() );
 
-		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer.
+		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer and close it.
+		$this->assertSame( $initial_level, ob_get_level() );
 
 		$this->assertSame( 1, $filter_count, 'Expected filter to be called once.' );
-		$this->assertSame( $filter_override, $buffer, 'Excepted return value of filter to be the resulting value for the buffer.' ); // TODO: The output buffer callback returns $filter_override, but the $buffer for some reason is actually equal to $template_override. Why?
-
-		ob_end_clean(); // Close the wrapper buffer.
+		$this->assertSame( $filter_override, $buffer, 'Excepted return value of filter to be the resulting value for the buffer.' );
 	}
 
 	/**

From f218dc833f9bb02b0001e9a765fbebb23ddd530a Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 11:06:57 -0700
Subject: [PATCH 05/12] Allow output buffer to be cleaned but not flushed

---
 .../optimization-detective/optimization.php   | 52 +++++++++++++-----
 .../tests/test-optimization.php               | 55 +++++++++++++------
 2 files changed, 78 insertions(+), 29 deletions(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index e3a60b1891..1d5172f216 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -31,21 +31,47 @@
  * @return string Unmodified value of $passthrough.
  */
 function od_buffer_output( string $passthrough ): string {
+	/*
+	 * Instead of the default PHP_OUTPUT_HANDLER_STDFLAGS (cleanable, flushable, and removable) being used for flags,
+	 * we need to omit PHP_OUTPUT_HANDLER_FLUSHABLE. If the buffer were flushable, then each time that ob_flush() is
+	 * called, it would send a fragment of the output into the output buffer callback. When buffering the entire
+	 * response as an HTML document, this would result in broken HTML processing.
+	 *
+	 * If this ends up being problematic, then PHP_OUTPUT_HANDLER_FLUSHABLE could be added to the $flags and the
+	 * output buffer callback could check if the phase is PHP_OUTPUT_HANDLER_FLUSH and abort any subsequent
+	 * processing while also emitting a _doing_it_wrong().
+	 */
+	$flags = PHP_OUTPUT_HANDLER_CLEANABLE;
+
+	// When running unit tests the output buffer must also be removable in order to obtain the buffered output.
+	if ( php_sapi_name() === 'cli' ) {
+		// TODO: Do any caching plugins need the output buffer to be removable? This is unlikely, as they would pass an output buffer callback to ob_start() instead of calling ob_get_clean() at shutdown.
+		$flags |= PHP_OUTPUT_HANDLER_REMOVABLE;
+	}
+
 	ob_start(
 		static function ( string $output, ?int $phase ): string {
-			if ( ( $phase & PHP_OUTPUT_HANDLER_FINAL ) > 0 ) {
-				/**
-				 * Filters the template output buffer prior to sending to the client.
-				 *
-				 * @since 0.1.0
-				 *
-				 * @param string $output Output buffer.
-				 * @return string Filtered output buffer.
-				 */
-				$output = (string) apply_filters( 'od_template_output_buffer', $output );
+			// When the output is being cleaned (e.g. pending template is replaced with error page), do not send it through the filter.
+			if ( ( $phase & PHP_OUTPUT_HANDLER_CLEAN ) !== 0 ) {
+				return $output;
 			}
-			return $output;
-		}
+
+			// Since ob_start() was called without PHP_OUTPUT_HANDLER_FLUSHABLE, at this point the phase should never be flush, and it should always be final.
+			assert( ( $phase & ( PHP_OUTPUT_HANDLER_FLUSH ) ) === 0 );
+			assert( ( $phase & ( PHP_OUTPUT_HANDLER_FINAL ) ) !== 0 );
+
+			/**
+			 * Filters the template output buffer prior to sending to the client.
+			 *
+			 * @since 0.1.0
+			 *
+			 * @param string $output Output buffer.
+			 * @return string Filtered output buffer.
+			 */
+			return (string) apply_filters( 'od_template_output_buffer', $output );
+		},
+		0, // Unlimited buffer size.
+		$flags
 	);
 	return $passthrough;
 }
@@ -142,7 +168,7 @@ function od_is_response_html_content_type(): bool {
  * @return string Filtered template output buffer.
  */
 function od_optimize_template_output_buffer( string $buffer ): string {
-	if ( ! od_is_response_html_content_type() ) {
+	if ( ! od_is_response_html_content_type() ) { // TODO: This should check to see if there is an HTML tag.
 		return $buffer;
 	}
 
diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php
index 4601cfc83d..14dc698695 100644
--- a/plugins/optimization-detective/tests/test-optimization.php
+++ b/plugins/optimization-detective/tests/test-optimization.php
@@ -65,7 +65,8 @@ function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
 		);
 
 		$original_ob_level = ob_get_level();
-		od_buffer_output( '' );
+		$template          = sprintf( 'page-%s.php', wp_generate_uuid4() );
+		$this->assertSame( $template, od_buffer_output( $template ), 'Expected value to be passed through.' );
 		$this->assertSame( $original_ob_level + 1, ob_get_level(), 'Expected call to ob_start().' );
 		echo $original;
 
@@ -77,48 +78,70 @@ function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
 	}
 
 	/**
-	 * Test that calling ob_clean() will discard previous buffer and never send it into the od_template_output_buffer filter.
+	 * Test that calling ob_flush() will not result in the buffer being processed and that ob_clean() will successfully prevent content from being processed.
 	 *
 	 * @covers ::od_buffer_output
 	 */
-	public function test_od_buffer_output_not_finalized(): void {
-		$original          = 'Hello My World!';
-		$template_override = 'Ciao mondo!';
-		$filter_override   = '¡Hola Mi Mundo!';
+	public function test_od_buffer_with_cleaning_and_attempted_flushing(): void {
+		$template_aborted = 'Before time began!';
+		$template_start   = 'The beginning';
+		$template_middle  = ', the middle';
+		$template_end     = ', and the end!';
 
 		// In order to test, a wrapping output buffer is required because ob_get_clean() does not invoke the output
 		// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
 		$initial_level = ob_get_level();
-		ob_start();
+		$this->assertTrue( ob_start() );
 		$this->assertSame( $initial_level + 1, ob_get_level() );
 
 		$filter_count = 0;
 		add_filter(
 			'od_template_output_buffer',
-			function ( $buffer ) use ( $template_override, $filter_override, &$filter_count ) {
-				$this->assertSame( $template_override, $buffer, 'Expected the original template output to never get passed into the buffer callback since ob_clean() was called after the original was printed.' );
+			function ( $buffer ) use ( $template_start, $template_middle, $template_end, &$filter_count ) {
 				$filter_count++;
-				return $filter_override;
+				$this->assertSame( $template_start . $template_middle . $template_end, $buffer );
+				return '<filtered>' . $buffer . '</filtered>';
 			}
 		);
 
 		od_buffer_output( '' );
 		$this->assertSame( $initial_level + 2, ob_get_level() );
-		echo $original; // This should never be passed into the od_template_output_buffer filter.
 
-		// Abort the original content printed above.
-		ob_clean(); // Note the lack of flush here and the lack of ending the buffer.
+		echo $template_aborted;
+		$this->assertTrue( ob_clean() ); // By cleaning, the above should never be seen by the filter.
+
+		// This is the start of what will end up getting filtered.
+		echo $template_start;
+
+		// Attempt to flush the output, which will fail because the output buffer was opened without the flushable flag.
+		$this->assertFalse( ob_flush() );
+
+		// This will also be sent into the filter.
+		echo $template_middle;
+		$this->assertFalse( ob_flush() );
+		$this->assertSame( $initial_level + 2, ob_get_level() );
+
+		// Start a nested output buffer which will also end up getting sent into the filter.
+		$this->assertTrue( ob_start() );
+		echo $template_end;
+		$this->assertSame( $initial_level + 3, ob_get_level() );
+		$this->assertTrue( ob_flush() );
+		$this->assertTrue( ob_end_flush() );
 		$this->assertSame( $initial_level + 2, ob_get_level() );
-		echo $template_override; // This should get passed into the od_template_output_buffer filter.
 
-		ob_end_flush(); // Close the output buffer opened by od_buffer_output().
+		// Close the output buffer opened by od_buffer_output(). This only works in the unit test because the removable flag was passed.
+		$this->assertTrue( ob_end_flush() );
 		$this->assertSame( $initial_level + 1, ob_get_level() );
 
 		$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer and close it.
 		$this->assertSame( $initial_level, ob_get_level() );
 
 		$this->assertSame( 1, $filter_count, 'Expected filter to be called once.' );
-		$this->assertSame( $filter_override, $buffer, 'Excepted return value of filter to be the resulting value for the buffer.' );
+		$this->assertSame(
+			'<filtered>' . $template_start . $template_middle . $template_end . '</filtered>',
+			$buffer,
+			'Excepted return value of filter to be the resulting value for the buffer.'
+		);
 	}
 
 	/**

From 0b4f2cd6a871ab30df01152b277d27dd6803d289 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 12:08:05 -0700
Subject: [PATCH 06/12] Allow output buffer to be removable

---
 plugins/optimization-detective/optimization.php | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index 1d5172f216..17eca60bdd 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -40,14 +40,13 @@ function od_buffer_output( string $passthrough ): string {
 	 * If this ends up being problematic, then PHP_OUTPUT_HANDLER_FLUSHABLE could be added to the $flags and the
 	 * output buffer callback could check if the phase is PHP_OUTPUT_HANDLER_FLUSH and abort any subsequent
 	 * processing while also emitting a _doing_it_wrong().
+	 *
+	 * The output buffer needs to be removable because WordPress calls wp_ob_end_flush_all() and then calls
+	 * wp_cache_close(). If the buffers are not all flushed before wp_cache_close() is closed, then some output buffer
+	 * handlers (e.g. for caching plugins) may fail to be able to store the page output in the object cache.
+	 * See <https://github.com/WordPress/performance/pull/1317#issuecomment-2271955356>.
 	 */
-	$flags = PHP_OUTPUT_HANDLER_CLEANABLE;
-
-	// When running unit tests the output buffer must also be removable in order to obtain the buffered output.
-	if ( php_sapi_name() === 'cli' ) {
-		// TODO: Do any caching plugins need the output buffer to be removable? This is unlikely, as they would pass an output buffer callback to ob_start() instead of calling ob_get_clean() at shutdown.
-		$flags |= PHP_OUTPUT_HANDLER_REMOVABLE;
-	}
+	$flags = PHP_OUTPUT_HANDLER_CLEANABLE | PHP_OUTPUT_HANDLER_REMOVABLE;
 
 	ob_start(
 		static function ( string $output, ?int $phase ): string {

From 2c6ec0c40fbf951b957a5714acd4fb9ee72ecf58 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 12:15:00 -0700
Subject: [PATCH 07/12] Use bitwise XOR on the standard flags to remove the
 flushable flag

---
 plugins/optimization-detective/optimization.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index 17eca60bdd..a4d12cfd45 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -46,7 +46,7 @@ function od_buffer_output( string $passthrough ): string {
 	 * handlers (e.g. for caching plugins) may fail to be able to store the page output in the object cache.
 	 * See <https://github.com/WordPress/performance/pull/1317#issuecomment-2271955356>.
 	 */
-	$flags = PHP_OUTPUT_HANDLER_CLEANABLE | PHP_OUTPUT_HANDLER_REMOVABLE;
+	$flags = PHP_OUTPUT_HANDLER_STDFLAGS ^ PHP_OUTPUT_HANDLER_FLUSHABLE;
 
 	ob_start(
 		static function ( string $output, ?int $phase ): string {

From 35085e86be4b3a7a882d964c6fd38e62a3610ca4 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 13:16:32 -0700
Subject: [PATCH 08/12] Ensure only HTML documents are processed by
 Optimization Detective

---
 .../optimization-detective/optimization.php   | 21 +++++++++---
 ...n-response-without-proper-content-type.php | 12 +++++++
 .../tests/test-cases/json-response.php        |  8 +++++
 ...s-response-without-proper-content-type.php | 34 +++++++++++++++++++
 4 files changed, 71 insertions(+), 4 deletions(-)
 create mode 100644 plugins/optimization-detective/tests/test-cases/json-response-without-proper-content-type.php
 create mode 100644 plugins/optimization-detective/tests/test-cases/json-response.php
 create mode 100644 plugins/optimization-detective/tests/test-cases/rss-response-without-proper-content-type.php

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index a4d12cfd45..139d3f860f 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -167,7 +167,21 @@ function od_is_response_html_content_type(): bool {
  * @return string Filtered template output buffer.
  */
 function od_optimize_template_output_buffer( string $buffer ): string {
-	if ( ! od_is_response_html_content_type() ) { // TODO: This should check to see if there is an HTML tag.
+	// If the content-type is not HTML or the output does not start with '<', then abort since the buffer is definitely not HTML.
+	if (
+		! od_is_response_html_content_type() ||
+		! str_starts_with( ltrim( $buffer ), '<' )
+	) {
+		return $buffer;
+	}
+
+	// If the initial tag is not an open HTML tag, then abort since the buffer is not a complete HTML document.
+	$processor = new OD_HTML_Tag_Processor( $buffer );
+	if ( ! (
+		$processor->next_tag() &&
+		! $processor->is_tag_closer() &&
+		'HTML' === $processor->get_tag()
+	) ) {
 		return $buffer;
 	}
 
@@ -196,11 +210,10 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 	do_action( 'od_register_tag_visitors', $tag_visitor_registry );
 
 	$link_collection      = new OD_Link_Collection();
-	$processor            = new OD_HTML_Tag_Processor( $buffer );
 	$tag_visitor_context  = new OD_Tag_Visitor_Context( $processor, $group_collection, $link_collection );
 	$current_tag_bookmark = 'optimization_detective_current_tag';
 	$visitors             = iterator_to_array( $tag_visitor_registry );
-	while ( $processor->next_open_tag() ) {
+	do {
 		$did_visit = false;
 		$processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false?
 
@@ -220,7 +233,7 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 		if ( $did_visit && $needs_detection ) {
 			$processor->set_meta_attribute( 'xpath', $processor->get_xpath() );
 		}
-	}
+	} while ( $processor->next_open_tag() );
 
 	// Send any preload links in a Link response header and in a LINK tag injected at the end of the HEAD.
 	if ( count( $link_collection ) > 0 ) {
diff --git a/plugins/optimization-detective/tests/test-cases/json-response-without-proper-content-type.php b/plugins/optimization-detective/tests/test-cases/json-response-without-proper-content-type.php
new file mode 100644
index 0000000000..76e9e1fe3a
--- /dev/null
+++ b/plugins/optimization-detective/tests/test-cases/json-response-without-proper-content-type.php
@@ -0,0 +1,12 @@
+<?php
+return array(
+	'set_up'   => static function (): void {
+		/*
+		 * This is intentionally not 'application/json'. This is to test whether od_optimize_template_output_buffer()
+		 * is checking whether the output starts with '<' (after whitespace is trimmed).
+		 */
+		ini_set( 'default_mimetype', 'text/html' ); // phpcs:ignore WordPress.PHP.IniSet.Risky
+	},
+	'buffer'   => ' {"doc": "<html lang="en"><body><img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" /></body></html>"}',
+	'expected' => ' {"doc": "<html lang="en"><body><img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" /></body></html>"}',
+);
diff --git a/plugins/optimization-detective/tests/test-cases/json-response.php b/plugins/optimization-detective/tests/test-cases/json-response.php
new file mode 100644
index 0000000000..a6494ff594
--- /dev/null
+++ b/plugins/optimization-detective/tests/test-cases/json-response.php
@@ -0,0 +1,8 @@
+<?php
+return array(
+	'set_up'   => static function (): void {
+		ini_set( 'default_mimetype', 'application/json' ); // phpcs:ignore WordPress.PHP.IniSet.Risky
+	},
+	'buffer'   => ' {"doc": "<html lang="en"><body><img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" /></body></html>"}',
+	'expected' => ' {"doc": "<html lang="en"><body><img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" /></body></html>"}',
+);
diff --git a/plugins/optimization-detective/tests/test-cases/rss-response-without-proper-content-type.php b/plugins/optimization-detective/tests/test-cases/rss-response-without-proper-content-type.php
new file mode 100644
index 0000000000..c4e267f46d
--- /dev/null
+++ b/plugins/optimization-detective/tests/test-cases/rss-response-without-proper-content-type.php
@@ -0,0 +1,34 @@
+<?php
+return array(
+	'set_up'   => static function (): void {
+		// This is intentionally not application/rss+xml as it is testing whether the first tag is HTML.
+		ini_set( 'default_mimetype', 'text/html' ); // phpcs:ignore WordPress.PHP.IniSet.Risky
+	},
+	// Also omitting the XML processing instruction.
+	'buffer'   => '
+		<rss version="2.0">
+			<channel>
+				<title>Example Blog</title>
+				<link>https://www.example.com</link>
+				<description>
+					<img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" />
+					A blog about technology, design, and culture.
+				</description>
+				<language>en-us</language>
+			</channel>
+		</rss>
+	',
+	'expected' => '
+		<rss version="2.0">
+			<channel>
+				<title>Example Blog</title>
+				<link>https://www.example.com</link>
+				<description>
+					<img src="https://www.example.com/logo.jpg" alt="Example Blog Logo" />
+					A blog about technology, design, and culture.
+				</description>
+				<language>en-us</language>
+			</channel>
+		</rss>
+	',
+);

From 364fa43d64440099deb6e023e93e89020d4baab0 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 13:21:13 -0700
Subject: [PATCH 09/12] Remove assert() checks

---
 plugins/optimization-detective/optimization.php | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
index a4d12cfd45..22bf259603 100644
--- a/plugins/optimization-detective/optimization.php
+++ b/plugins/optimization-detective/optimization.php
@@ -55,10 +55,6 @@ static function ( string $output, ?int $phase ): string {
 				return $output;
 			}
 
-			// Since ob_start() was called without PHP_OUTPUT_HANDLER_FLUSHABLE, at this point the phase should never be flush, and it should always be final.
-			assert( ( $phase & ( PHP_OUTPUT_HANDLER_FLUSH ) ) === 0 );
-			assert( ( $phase & ( PHP_OUTPUT_HANDLER_FINAL ) ) !== 0 );
-
 			/**
 			 * Filters the template output buffer prior to sending to the client.
 			 *

From 1b62c1af3a7844377fcc8e3e9d758dd8eda9af9b Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 14:48:55 -0700
Subject: [PATCH 10/12] Avoid sending Server-Timing header when buffer is being
 cleaned

---
 .../includes/server-timing/class-perflab-server-timing.php | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
index 56a6536a3a..a5c05c2f49 100644
--- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
+++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
@@ -268,8 +268,11 @@ public function on_template_include( $passthrough = null ) {
 	 */
 	public function start_output_buffer(): void {
 		ob_start(
-			function ( $output ) {
-				$this->send_header();
+			function ( string $output, ?int $phase ): string {
+				// Only send the header when the buffer is not being cleaned.
+				if ( ( $phase & PHP_OUTPUT_HANDLER_CLEAN ) === 0 ) {
+					$this->send_header();
+				}
 				return $output;
 			}
 		);

From 26e859db06cb4254510869610644641e60c796ea Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 14:51:06 -0700
Subject: [PATCH 11/12] Fix passing arg name to _doing_it_wrong()

---
 .../includes/server-timing/class-perflab-server-timing.php    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
index a5c05c2f49..a6608df8a3 100644
--- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
+++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
@@ -88,7 +88,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: PHP parameter name */
-				sprintf( esc_html__( 'The %s argument is required and must be a callable.', 'performance-lab' ), esc_attr( $args['measure_callback'] ) ),
+				sprintf( esc_html__( 'The %s argument is required and must be a callable.', 'performance-lab' ), 'measure_callback' ),
 				''
 			);
 			return;
@@ -97,7 +97,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: PHP parameter name */
-				sprintf( esc_html__( 'The %s argument is required and must be a string.', 'performance-lab' ), esc_attr( $args['access_cap'] ) ),
+				sprintf( esc_html__( 'The %s argument is required and must be a string.', 'performance-lab' ), 'access_cap' ),
 				''
 			);
 			return;

From 21ebb9744efdbfabbc022e37deb8128ed5bd5ac9 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 6 Aug 2024 14:56:41 -0700
Subject: [PATCH 12/12] Improve escaping output by wrapping entire string in
 esc_html()

---
 .../server-timing/class-perflab-server-timing.php         | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
index a6608df8a3..3eed6ad703 100644
--- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
+++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
@@ -61,7 +61,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: metric slug */
-				sprintf( esc_html__( 'A metric with the slug %s is already registered.', 'performance-lab' ), esc_attr( $metric_slug ) ),
+				esc_html( sprintf( __( 'A metric with the slug %s is already registered.', 'performance-lab' ), $metric_slug ) ),
 				''
 			);
 			return;
@@ -71,7 +71,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: WordPress action name */
-				sprintf( esc_html__( 'The method must be called before or during the %s action.', 'performance-lab' ), 'perflab_server_timing_send_header' ),
+				esc_html( sprintf( __( 'The method must be called before or during the %s action.', 'performance-lab' ), 'perflab_server_timing_send_header' ) ),
 				''
 			);
 			return;
@@ -88,7 +88,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: PHP parameter name */
-				sprintf( esc_html__( 'The %s argument is required and must be a callable.', 'performance-lab' ), 'measure_callback' ),
+				esc_html( sprintf( __( 'The %s argument is required and must be a callable.', 'performance-lab' ), 'measure_callback' ) ),
 				''
 			);
 			return;
@@ -97,7 +97,7 @@ public function register_metric( string $metric_slug, array $args ): void {
 			_doing_it_wrong(
 				__METHOD__,
 				/* translators: %s: PHP parameter name */
-				sprintf( esc_html__( 'The %s argument is required and must be a string.', 'performance-lab' ), 'access_cap' ),
+				esc_html( sprintf( __( 'The %s argument is required and must be a string.', 'performance-lab' ), 'access_cap' ) ),
 				''
 			);
 			return;