diff --git a/concat-utils.php b/concat-utils.php index ecb55ba..b8be619 100644 --- a/concat-utils.php +++ b/concat-utils.php @@ -3,6 +3,9 @@ class WPCOM_Concat_Utils { // Maximum group size, anything over that will be split into multiple groups protected static int $concat_max = 150; + // UTF-8 byte order mark. + const UTF8_BOM = "\xEF\xBB\xBF"; + public static function get_concat_max() { return self::$concat_max; } @@ -44,6 +47,49 @@ public static function realpath( $url, $site_url ) { return realpath( ABSPATH . $url_path ); } + /** + * Tells if a buffer starts with a UTF-8 byte order mark. + * + * @param string $buf File contents. + * @return bool + */ + public static function has_utf8_bom( $buf ) { + return 0 === strncmp( $buf, self::UTF8_BOM, strlen( self::UTF8_BOM ) ); + } + + /** + * Removes UTF-8 byte order marks from the start of a buffer. + * + * A BOM gives the encoding only at offset 0. In a bundle, the BOM of a later + * file becomes a U+FEFF character in the middle of the stream. CSS accepts + * that character in a selector. The browser then adds it to the next + * selector, and ignores that rule. + * + * The function removes every BOM at the start, not only the first one, so the + * buffer it returns has none. A second BOM is not an encoding signal. In any + * file but the first it becomes the same stray character. + * + * It keeps a UTF-16 or UTF-32 BOM. Those bytes are not valid in a UTF-8 bundle. + * + * @see https://github.com/Automattic/nginx-http-concat/issues/49 + * + * @param string $buf File contents. + * @return string The buffer without a BOM at the start. + */ + public static function strip_utf8_bom( $buf ) { + $bom_len = strlen( self::UTF8_BOM ); + $len = strlen( $buf ); + $offset = 0; + + // Count the BOMs in one pass. A file of only BOMs then needs one copy. + while ( $offset + $bom_len <= $len + && 0 === substr_compare( $buf, self::UTF8_BOM, $offset, $bom_len ) ) { + $offset += $bom_len; + } + + return 0 === $offset ? $buf : substr( $buf, $offset ); + } + public static function relative_path_replace( $buf, $dirpath ) { // url(relative/path/to/file) -> url(/absolute/and/not/relative/path/to/file) $buf = preg_replace( diff --git a/ngx-http-concat.php b/ngx-http-concat.php index ec0522d..385b144 100644 --- a/ngx-http-concat.php +++ b/ngx-http-concat.php @@ -143,6 +143,7 @@ function concat_get_path( $uri ) { $last_modified = 0; $pre_output = ''; $output = ''; +$had_utf8_bom = false; $css_minify = new tubalmartin\CssMin\Minifier; @@ -175,6 +176,13 @@ function concat_get_path( $uri ) { if ( false === $buf ) concat_http_status_exit( 500 ); + // Remove the BOM before the code below reads $buf. The @charset test is at + // offset 0, and a BOM also breaks that test. + if ( WPCOM_Concat_Utils::has_utf8_bom( $buf ) ) { + $buf = WPCOM_Concat_Utils::strip_utf8_bom( $buf ); + $had_utf8_bom = true; + } + if ( 'text/css' == $mime_type ) { $dirpath = $subdir_path_prefix . dirname( $uri ); @@ -240,8 +248,17 @@ function ( $match ) use ( $dirpath ) { $output .= "$buf"; } +// A BOM showed that a file is UTF-8, and the loop above removed it. Give that +// encoding in the header to keep the declaration. The Content-Type charset has a +// higher priority than a BOM and than an @charset rule. Set the charset only +// after a BOM, because the encoding of other bundles is not known. +$content_type = $mime_type; +if ( $had_utf8_bom ) { + $content_type .= '; charset=UTF-8'; +} + header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s', $last_modified ) . ' GMT' ); header( 'Content-Length: ' . ( strlen( $pre_output ) + strlen( $output ) ) ); -header( "Content-Type: $mime_type" ); +header( "Content-Type: $content_type" ); echo $pre_output . $output; diff --git a/tests/test-concat-utils.php b/tests/test-concat-utils.php index 693ccd9..1eadf9e 100755 --- a/tests/test-concat-utils.php +++ b/tests/test-concat-utils.php @@ -206,3 +206,144 @@ function test__function( $test_string, $expected, $dirpath = '/' ) { $this->assertSame( $expected, $actual ); } } + +class WPCOM_Concat_Utils__Strip_Utf8_Bom__TestCase extends WP_UnitTestCase { + const BOM = "\xEF\xBB\xBF"; + + function get_test_data() { + return array( + // Nothing to strip. + 'empty_string' => array( + '', + '', + false, + ), + 'no_bom' => array( + '*{margin:0;padding:0}', + '*{margin:0;padding:0}', + false, + ), + // The buffer is shorter than a BOM. The code must not read too far. + 'single_byte' => array( + 'a', + 'a', + false, + ), + 'two_bytes' => array( + 'ab', + 'ab', + false, + ), + // An incomplete BOM is not a BOM. + 'partial_bom' => array( + "\xEF\xBB", + "\xEF\xBB", + false, + ), + 'partial_bom_then_text' => array( + "\xEF\xBBz", + "\xEF\xBBz", + false, + ), + // The reported bug. A BOM is before the first rule of a file. + // https://github.com/Automattic/nginx-http-concat/issues/49 + 'bom_then_rule' => array( + self::BOM . '*{margin:0;padding:0}', + '*{margin:0;padding:0}', + true, + ), + // The @charset test is at offset 0. A BOM breaks that test. + 'bom_then_charset' => array( + self::BOM . '@charset "UTF-8";', + '@charset "UTF-8";', + true, + ), + 'bom_only' => array( + self::BOM, + '', + true, + ), + // Only a BOM at offset 0 is an encoding signal. The rest must go too. + 'repeated_boms' => array( + self::BOM . self::BOM . self::BOM . '.a{color:red}', + '.a{color:red}', + true, + ), + // A BOM gives the encoding only at offset 0. In other positions it is content. + 'bom_mid_stream_is_left_alone' => array( + '.a{color:red}' . self::BOM . '.b{color:blue}', + '.a{color:red}' . self::BOM . '.b{color:blue}', + false, + ), + 'bom_at_end_is_left_alone' => array( + '.a{color:red}' . self::BOM, + '.a{color:red}' . self::BOM, + false, + ), + // UTF-16 and UTF-32 bytes are not valid in the bundle. Keep the BOM, + // because UTF-8 is the wrong encoding for that content. + 'utf16_le_bom_untouched' => array( + "\xFF\xFE.a{color:red}", + "\xFF\xFE.a{color:red}", + false, + ), + 'utf16_be_bom_untouched' => array( + "\xFE\xFF.a{color:red}", + "\xFE\xFF.a{color:red}", + false, + ), + // The first character has more than one byte, but it is not a BOM. + 'leading_non_bom_multibyte_char' => array( + "\xE2\x86\x92.a{color:red}", + "\xE2\x86\x92.a{color:red}", + false, + ), + ); + } + + /** + * @dataProvider get_test_data + */ + function test__strip_utf8_bom( $test_string, $expected, $expected_has_bom ) { + $this->assertSame( $expected, WPCOM_Concat_Utils::strip_utf8_bom( $test_string ) ); + } + + /** + * @dataProvider get_test_data + */ + function test__has_utf8_bom( $test_string, $expected, $expected_has_bom ) { + $this->assertSame( $expected_has_bom, WPCOM_Concat_Utils::has_utf8_bom( $test_string ) ); + } + + /** + * A safe buffer has no BOM. Therefore a second call must make no change. + * + * @dataProvider get_test_data + */ + function test__strip_utf8_bom_is_idempotent( $test_string, $expected, $expected_has_bom ) { + $once = WPCOM_Concat_Utils::strip_utf8_bom( $test_string ); + + $this->assertSame( $once, WPCOM_Concat_Utils::strip_utf8_bom( $once ) ); + $this->assertFalse( WPCOM_Concat_Utils::has_utf8_bom( $once ) ); + } + + /** + * The stream must have no BOM after offset 0. The position of the file + * with the BOM must not change this result. + */ + function test__concatenating_stripped_buffers_leaves_no_stray_bom() { + $files = array( + 'body{color:red}', + self::BOM . '*{margin:0;padding:0}', + '.c{color:blue}', + ); + + $output = ''; + foreach ( $files as $buf ) { + $output .= WPCOM_Concat_Utils::strip_utf8_bom( $buf ); + } + + $this->assertSame( 'body{color:red}*{margin:0;padding:0}.c{color:blue}', $output ); + $this->assertStringNotContainsString( self::BOM, $output ); + } +}