diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 442a7a6226..e47d7813e2 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -31,8 +31,30 @@ * @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(). + * + * 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 . + */ + $flags = PHP_OUTPUT_HANDLER_STDFLAGS ^ PHP_OUTPUT_HANDLER_FLUSHABLE; + ob_start( - static function ( string $output ): string { + static function ( string $output, ?int $phase ): string { + // 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; + } + /** * Filters the template output buffer prior to sending to the client. * @@ -42,7 +64,9 @@ static function ( string $output ): string { * @return string Filtered output buffer. */ return (string) apply_filters( 'od_template_output_buffer', $output ); - } + }, + 0, // Unlimited buffer size. + $flags ); return $passthrough; } @@ -139,7 +163,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() ) { + // 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; } @@ -168,11 +206,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? @@ -192,7 +229,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 @@ + 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": "Example Blog Logo"}', + 'expected' => ' {"doc": "Example Blog Logo"}', +); 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 @@ + static function (): void { + ini_set( 'default_mimetype', 'application/json' ); // phpcs:ignore WordPress.PHP.IniSet.Risky + }, + 'buffer' => ' {"doc": "Example Blog Logo"}', + 'expected' => ' {"doc": "Example Blog Logo"}', +); 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 @@ + 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' => ' + + + Example Blog + https://www.example.com + + Example Blog Logo + A blog about technology, design, and culture. + + en-us + + + ', + 'expected' => ' + + + Example Blog + https://www.example.com + + Example Blog Logo + A blog about technology, design, and culture. + + en-us + + + ', +); diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php index ff62951a5c..14dc698695 100644 --- a/plugins/optimization-detective/tests/test-optimization.php +++ b/plugins/optimization-detective/tests/test-optimization.php @@ -54,16 +54,19 @@ public function test_od_buffer_output(): void { // buffer callback. See . 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; } ); $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; @@ -71,6 +74,74 @@ 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 ); + } + + /** + * 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_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 . + $initial_level = ob_get_level(); + $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_start, $template_middle, $template_end, &$filter_count ) { + $filter_count++; + $this->assertSame( $template_start . $template_middle . $template_end, $buffer ); + return '' . $buffer . ''; + } + ); + + od_buffer_output( '' ); + $this->assertSame( $initial_level + 2, ob_get_level() ); + + 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() ); + + // 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( + '' . $template_start . $template_middle . $template_end . '', + $buffer, + 'Excepted return value of filter to be the resulting value for the buffer.' + ); } /** 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..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' ), esc_attr( $args['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' ), esc_attr( $args['access_cap'] ) ), + esc_html( sprintf( __( 'The %s argument is required and must be a string.', 'performance-lab' ), 'access_cap' ) ), '' ); return; @@ -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; } );