Skip to content

Commit f218dc8

Browse files
westonruterwestonruter
committed
Allow output buffer to be cleaned but not flushed
1 parent 78967f5 commit f218dc8

File tree

2 files changed

+78
-29
lines changed

2 files changed

+78
-29
lines changed

‎plugins/optimization-detective/optimization.php

+39-13
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,47 @@
3131
* @return string Unmodified value of $passthrough.
3232
*/
3333
function od_buffer_output( string $passthrough ): string {
34+
/*
35+
* Instead of the default PHP_OUTPUT_HANDLER_STDFLAGS (cleanable, flushable, and removable) being used for flags,
36+
* we need to omit PHP_OUTPUT_HANDLER_FLUSHABLE. If the buffer were flushable, then each time that ob_flush() is
37+
* called, it would send a fragment of the output into the output buffer callback. When buffering the entire
38+
* response as an HTML document, this would result in broken HTML processing.
39+
*
40+
* If this ends up being problematic, then PHP_OUTPUT_HANDLER_FLUSHABLE could be added to the $flags and the
41+
* output buffer callback could check if the phase is PHP_OUTPUT_HANDLER_FLUSH and abort any subsequent
42+
* processing while also emitting a _doing_it_wrong().
43+
*/
44+
$flags = PHP_OUTPUT_HANDLER_CLEANABLE;
45+
46+
// When running unit tests the output buffer must also be removable in order to obtain the buffered output.
47+
if ( php_sapi_name() === 'cli' ) {
48+
// 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.
49+
$flags |= PHP_OUTPUT_HANDLER_REMOVABLE;
50+
}
51+
3452
ob_start(
3553
static function ( string $output, ?int $phase ): string {
36-
if ( ( $phase & PHP_OUTPUT_HANDLER_FINAL ) > 0 ) {
37-
/**
38-
* Filters the template output buffer prior to sending to the client.
39-
*
40-
* @since 0.1.0
41-
*
42-
* @param string $output Output buffer.
43-
* @return string Filtered output buffer.
44-
*/
45-
$output = (string) apply_filters( 'od_template_output_buffer', $output );
54+
// When the output is being cleaned (e.g. pending template is replaced with error page), do not send it through the filter.
55+
if ( ( $phase & PHP_OUTPUT_HANDLER_CLEAN ) !== 0 ) {
56+
return $output;
4657
}
47-
return $output;
48-
}
58+
59+
// 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.
60+
assert( ( $phase & ( PHP_OUTPUT_HANDLER_FLUSH ) ) === 0 );
61+
assert( ( $phase & ( PHP_OUTPUT_HANDLER_FINAL ) ) !== 0 );
62+
63+
/**
64+
* Filters the template output buffer prior to sending to the client.
65+
*
66+
* @since 0.1.0
67+
*
68+
* @param string $output Output buffer.
69+
* @return string Filtered output buffer.
70+
*/
71+
return (string) apply_filters( 'od_template_output_buffer', $output );
72+
},
73+
0, // Unlimited buffer size.
74+
$flags
4975
);
5076
return $passthrough;
5177
}
@@ -142,7 +168,7 @@ function od_is_response_html_content_type(): bool {
142168
* @return string Filtered template output buffer.
143169
*/
144170
function od_optimize_template_output_buffer( string $buffer ): string {
145-
if ( ! od_is_response_html_content_type() ) {
171+
if ( ! od_is_response_html_content_type() ) { // TODO: This should check to see if there is an HTML tag.
146172
return $buffer;
147173
}
148174

‎plugins/optimization-detective/tests/test-optimization.php

+39-16
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
6565
);
6666

6767
$original_ob_level = ob_get_level();
68-
od_buffer_output( '' );
68+
$template = sprintf( 'page-%s.php', wp_generate_uuid4() );
69+
$this->assertSame( $template, od_buffer_output( $template ), 'Expected value to be passed through.' );
6970
$this->assertSame( $original_ob_level + 1, ob_get_level(), 'Expected call to ob_start().' );
7071
echo $original;
7172

@@ -77,48 +78,70 @@ function ( $buffer ) use ( $original, $expected, &$filter_invoked ) {
7778
}
7879

7980
/**
80-
* Test that calling ob_clean() will discard previous buffer and never send it into the od_template_output_buffer filter.
81+
* Test that calling ob_flush() will not result in the buffer being processed and that ob_clean() will successfully prevent content from being processed.
8182
*
8283
* @covers ::od_buffer_output
8384
*/
84-
public function test_od_buffer_output_not_finalized(): void {
85-
$original = 'Hello My World!';
86-
$template_override = 'Ciao mondo!';
87-
$filter_override = '¡Hola Mi Mundo!';
85+
public function test_od_buffer_with_cleaning_and_attempted_flushing(): void {
86+
$template_aborted = 'Before time began!';
87+
$template_start = 'The beginning';
88+
$template_middle = ', the middle';
89+
$template_end = ', and the end!';
8890

8991
// In order to test, a wrapping output buffer is required because ob_get_clean() does not invoke the output
9092
// buffer callback. See <https://stackoverflow.com/a/61439514/93579>.
9193
$initial_level = ob_get_level();
92-
ob_start();
94+
$this->assertTrue( ob_start() );
9395
$this->assertSame( $initial_level + 1, ob_get_level() );
9496

9597
$filter_count = 0;
9698
add_filter(
9799
'od_template_output_buffer',
98-
function ( $buffer ) use ( $template_override, $filter_override, &$filter_count ) {
99-
$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.' );
100+
function ( $buffer ) use ( $template_start, $template_middle, $template_end, &$filter_count ) {
100101
$filter_count++;
101-
return $filter_override;
102+
$this->assertSame( $template_start . $template_middle . $template_end, $buffer );
103+
return '<filtered>' . $buffer . '</filtered>';
102104
}
103105
);
104106

105107
od_buffer_output( '' );
106108
$this->assertSame( $initial_level + 2, ob_get_level() );
107-
echo $original; // This should never be passed into the od_template_output_buffer filter.
108109

109-
// Abort the original content printed above.
110-
ob_clean(); // Note the lack of flush here and the lack of ending the buffer.
110+
echo $template_aborted;
111+
$this->assertTrue( ob_clean() ); // By cleaning, the above should never be seen by the filter.
112+
113+
// This is the start of what will end up getting filtered.
114+
echo $template_start;
115+
116+
// Attempt to flush the output, which will fail because the output buffer was opened without the flushable flag.
117+
$this->assertFalse( ob_flush() );
118+
119+
// This will also be sent into the filter.
120+
echo $template_middle;
121+
$this->assertFalse( ob_flush() );
122+
$this->assertSame( $initial_level + 2, ob_get_level() );
123+
124+
// Start a nested output buffer which will also end up getting sent into the filter.
125+
$this->assertTrue( ob_start() );
126+
echo $template_end;
127+
$this->assertSame( $initial_level + 3, ob_get_level() );
128+
$this->assertTrue( ob_flush() );
129+
$this->assertTrue( ob_end_flush() );
111130
$this->assertSame( $initial_level + 2, ob_get_level() );
112-
echo $template_override; // This should get passed into the od_template_output_buffer filter.
113131

114-
ob_end_flush(); // Close the output buffer opened by od_buffer_output().
132+
// Close the output buffer opened by od_buffer_output(). This only works in the unit test because the removable flag was passed.
133+
$this->assertTrue( ob_end_flush() );
115134
$this->assertSame( $initial_level + 1, ob_get_level() );
116135

117136
$buffer = ob_get_clean(); // Get the buffer from our wrapper output buffer and close it.
118137
$this->assertSame( $initial_level, ob_get_level() );
119138

120139
$this->assertSame( 1, $filter_count, 'Expected filter to be called once.' );
121-
$this->assertSame( $filter_override, $buffer, 'Excepted return value of filter to be the resulting value for the buffer.' );
140+
$this->assertSame(
141+
'<filtered>' . $template_start . $template_middle . $template_end . '</filtered>',
142+
$buffer,
143+
'Excepted return value of filter to be the resulting value for the buffer.'
144+
);
122145
}
123146

124147
/**

0 commit comments

Comments
 (0)