diff --git a/packages/mysql-parser/src/class-wp-mysql-lexer.php b/packages/mysql-parser/src/class-wp-mysql-lexer.php index d73f88b63..bb82537c1 100644 --- a/packages/mysql-parser/src/class-wp-mysql-lexer.php +++ b/packages/mysql-parser/src/class-wp-mysql-lexer.php @@ -1297,20 +1297,32 @@ private function read_line_comment(): int { private function read_mysql_comment(): int { // @TODO: Consider supporting optimizer hints (/*+ ... */) or document // that they are not supported. - // @TODO: Implement six-digit version number support (from MySQL 8.4). // MySQL-specific comment in one of the following forms: - // 1. /*! ... */ - The content is treated as SQL. - // 2. /*!12345 ... */ - The content is treated as SQL when "MySQL version >= 12345". + // 1. /*! ... */ - The content is treated as SQL. + // 2. /*!12345 ... */ - The content is treated as SQL when "MySQL version >= 12345". + // 3. /*!123456 ... */ - As of MySQL 8.4, a six-digit version (MMmmrr). $this->bytes_already_read += 3; // Consume the '/*!'. - // Check if the next 5 characters are digits. - $digit_count = strspn( $this->sql, self::DIGIT_MASK, $this->bytes_already_read, 5 ); - $is_version_comment = 5 === $digit_count; - - // For version comments, extract the version number. - $version = $is_version_comment - ? (int) substr( $this->sql, $this->bytes_already_read, $digit_count ) + /* + * Extract the version number, mirroring MySQL's own strict rule: the first + * five characters must be digits. If a sixth digit follows and is itself + * followed by whitespace, it is a six-digit version (MMmmrr, as of MySQL + * 8.4); otherwise the version is the first five digits and any extra digit + * stays comment content. + */ + $version_length = 0; + if ( 5 === strspn( $this->sql, self::DIGIT_MASK, $this->bytes_already_read, 5 ) ) { + $version_length = 5; + if ( + 1 === strspn( $this->sql, self::DIGIT_MASK, $this->bytes_already_read + 5, 1 ) + && 1 === strspn( $this->sql, self::WHITESPACE_MASK, $this->bytes_already_read + 6, 1 ) + ) { + $version_length = 6; + } + } + $version = $version_length > 0 + ? (int) substr( $this->sql, $this->bytes_already_read, $version_length ) : 0; if ( $this->mysql_version < $version ) { @@ -1319,7 +1331,7 @@ private function read_mysql_comment(): int { return self::COMMENT; } else { // Version satisfied or not specified. Treat the content as SQL code. - $this->bytes_already_read += $digit_count; // Skip the version number. + $this->bytes_already_read += $version_length; // Skip the version number. $this->in_mysql_comment = true; return self::MYSQL_COMMENT_START; } @@ -1354,11 +1366,16 @@ private function resolve_keyword_type( int $type, string $word ): int { // Function keywords (declared with SYM_FN in MySQL's lex.h) are keywords // only when directly followed by an opening parenthesis. if ( isset( self::FUNCTIONS[ $word ] ) ) { - // Skip any whitespace character if the SQL mode says they should be ignored. + // Under SQL_MODE_IGNORE_SPACE, whitespace may sit between the keyword and + // the "(", so peek past it WITHOUT consuming it. Those bytes belong to the + // next lexeme, not to this token: consuming them would stretch the token's + // byte range (corrupting an identifier's value, or padding the keyword), + // because produce() derives the length from bytes_already_read. + $peek = $this->bytes_already_read; if ( $this->is_sql_mode_active( self::SQL_MODE_IGNORE_SPACE ) ) { - $this->bytes_already_read += strspn( $this->sql, self::WHITESPACE_MASK, $this->bytes_already_read ); + $peek += strspn( $this->sql, self::WHITESPACE_MASK, $peek ); } - if ( '(' !== ( $this->sql[ $this->bytes_already_read ] ?? null ) ) { + if ( '(' !== ( $this->sql[ $peek ] ?? null ) ) { return self::IDENTIFIER; } } diff --git a/packages/mysql-parser/src/class-wp-mysql-token.php b/packages/mysql-parser/src/class-wp-mysql-token.php index db4b6e666..f2fe11067 100644 --- a/packages/mysql-parser/src/class-wp-mysql-token.php +++ b/packages/mysql-parser/src/class-wp-mysql-token.php @@ -169,8 +169,14 @@ public function get_value(): string { * A backslash with any other character represents the character itself. * That is, \x evaluates to x, \\ evaluates to \, and \🙂 evaluates to 🙂. */ + // Use the "s" (DOTALL) modifier, never "u" (UTF-8): "u" makes PCRE + // validate the whole subject as UTF-8 and return null on the first + // invalid byte, which would crash on the legitimate non-UTF-8 bytes a + // MySQL literal may carry (binary or other-charset payloads). A byte-wise + // strip is binary-safe and identical for valid UTF-8, since no UTF-8 + // continuation byte is a backslash. $preg_quoted_backslash = preg_quote( $backslash ); - $value = preg_replace( "/$preg_quoted_backslash(.)/u", '$1', $value ); + $value = preg_replace( "/$preg_quoted_backslash(.)/s", '$1', $value ); } return $value; } diff --git a/packages/mysql-parser/tests/WP_MySQL_Lexer_Tests.php b/packages/mysql-parser/tests/WP_MySQL_Lexer_Tests.php index 67ad9ef36..9e43c8e79 100644 --- a/packages/mysql-parser/tests/WP_MySQL_Lexer_Tests.php +++ b/packages/mysql-parser/tests/WP_MySQL_Lexer_Tests.php @@ -85,6 +85,49 @@ public function test_emits_bison_terminals_with_end_markers(): void { ); } + public function test_ignore_space_does_not_absorb_whitespace_into_function_identifiers(): void { + // COUNT is a function keyword (SYM_FN). Under IGNORE_SPACE, "COUNT" that is + // not followed by "(" is a plain identifier, and its byte range must exclude + // the trailing whitespace that the mode skips while peeking for "(". + foreach ( array( 'SELECT COUNT FROM t', "SELECT COUNT\t\n FROM t" ) as $sql ) { + $tokens = ( new WP_MySQL_Lexer( $sql, 80400, array( 'IGNORE_SPACE' ) ) )->remaining_tokens(); + $this->assertSame( 'IDENTIFIER', $tokens[1]->get_name(), $sql ); + $this->assertSame( 'COUNT', $tokens[1]->get_value(), $sql ); + $this->assertSame( 5, $tokens[1]->length, $sql ); + } + + // When "(" does follow across whitespace, COUNT stays a function keyword and + // its byte range still excludes the whitespace. + $tokens = ( new WP_MySQL_Lexer( 'SELECT COUNT (1)', 80400, array( 'IGNORE_SPACE' ) ) )->remaining_tokens(); + $this->assertSame( 5, $tokens[1]->length ); + $this->assertNotSame( 'IDENTIFIER', $tokens[1]->get_name() ); + $this->assertSame( 'OPEN_PAR_SYMBOL', $tokens[2]->get_name() ); + } + + public function test_version_comments_gate_the_body_by_version(): void { + // Five-digit version: the body is SQL only when the server version satisfies it. + $this->assertSame( + array( 'SELECT', 'INT_NUMBER', 'INT_NUMBER', 'END_OF_INPUT', 'END_MARKER' ), + self::token_names( 'SELECT /*!50000 1 */ 2' ) + ); + $this->assertSame( + array( 'SELECT', 'INT_NUMBER', 'END_OF_INPUT', 'END_MARKER' ), + self::token_names( 'SELECT /*!99999 1 */ 2' ) + ); + + // Six-digit MMmmrr version (MySQL 8.4): a sixth digit followed by whitespace + // belongs to the version, not the body — so 080400 is consumed whole (no stray + // digit), and 100000 gates above 8.4.0 rather than as 10000. + $this->assertSame( + array( 'SELECT', 'INT_NUMBER', 'INT_NUMBER', 'END_OF_INPUT', 'END_MARKER' ), + self::token_names( 'SELECT /*!080400 1 */ 2' ) + ); + $this->assertSame( + array( 'SELECT', 'INT_NUMBER', 'END_OF_INPUT', 'END_MARKER' ), + self::token_names( 'SELECT /*!100000 1 */ 2' ) + ); + } + public function test_at_name_splits_into_at_and_ident(): void { $tokens = ( new WP_MySQL_Lexer( 'SELECT @var1' ) )->remaining_tokens(); $this->assertSame( 'AT_SIGN_SYMBOL', $tokens[1]->get_name() ); diff --git a/packages/mysql-parser/tests/WP_MySQL_Server_Suite_Parser_Tests.php b/packages/mysql-parser/tests/WP_MySQL_Server_Suite_Parser_Tests.php index 703b5fe80..261bb8838 100644 --- a/packages/mysql-parser/tests/WP_MySQL_Server_Suite_Parser_Tests.php +++ b/packages/mysql-parser/tests/WP_MySQL_Server_Suite_Parser_Tests.php @@ -20,7 +20,7 @@ class WP_MySQL_Server_Suite_Parser_Tests extends TestCase { * legitimately changes. */ const EXPECTED_QUERIES = 69158; - const EXPECTED_FAILURES = 171; + const EXPECTED_FAILURES = 168; public function test_corpus_acceptance_rate(): void { $parser = WP_MySQL_Parser_Factory::create_parser(); diff --git a/packages/mysql-parser/tests/WP_MySQL_Token_Tests.php b/packages/mysql-parser/tests/WP_MySQL_Token_Tests.php index 7e5cb07aa..bc3e9c148 100644 --- a/packages/mysql-parser/tests/WP_MySQL_Token_Tests.php +++ b/packages/mysql-parser/tests/WP_MySQL_Token_Tests.php @@ -34,6 +34,30 @@ public function test_get_value_honors_no_backslash_escapes_mode(): void { $this->assertSame( 'a\\nb', $token->get_value() ); } + public function test_get_value_preserves_non_utf8_bytes_in_string_literals(): void { + // A quoted literal may legitimately carry non-UTF-8 bytes (binary or + // other-charset payloads). Value extraction must return them unchanged + // rather than crash on a UTF-8 validation failure. + $raw = chr( 0xFF ) . chr( 0xFE ); + $this->assertSame( + $raw, + self::first_token( "SELECT '$raw'", 'SINGLE_QUOTED_TEXT' )->get_value() + ); + + // A backslash escape preceding a non-UTF-8 byte strips the backslash and + // keeps the raw byte. + $this->assertSame( + chr( 0xE9 ), + self::first_token( "SELECT '\\" . chr( 0xE9 ) . "'", 'SINGLE_QUOTED_TEXT' )->get_value() + ); + + // Valid multibyte UTF-8 still round-trips. + $this->assertSame( + 'café 🙂', + self::first_token( "SELECT 'café 🙂'", 'SINGLE_QUOTED_TEXT' )->get_value() + ); + } + public function test_get_value_unquotes_backtick_identifiers(): void { $this->assertSame( 'col name', self::first_token( 'SELECT `col name` FROM t', 'BACK_TICK_QUOTED_ID' )->get_value() ); } diff --git a/packages/mysql-parser/tests/data/corpus-failures.csv b/packages/mysql-parser/tests/data/corpus-failures.csv index bb4b62830..40190eeed 100644 --- a/packages/mysql-parser/tests/data/corpus-failures.csv +++ b/packages/mysql-parser/tests/data/corpus-failures.csv @@ -653,10 +653,6 @@ UNLOCK TABLES" "SELECT 'a%' NOT LIKE 'a!%' ESCAPE '!', 'a%' NOT LIKE 'a!' || '%' ESCAPE '!'" "SELECT 'a%' NOT LIKE 'a!%' ESCAPE '$', 'a%' NOT LIKE 'a!' || '%' ESCAPE '$'" "SELECT * from ""full""" -"SELECT 1 /*!080100 +1*/ AS should_return_2" -"SELECT 1 /*!080100 -+1*/ AS should_return_2" -"SELECT 1 /*!080100 +1*/ AS should_return_2" "SELECT 1 /*!99999 /* */ */" "SET sql_mode = 'NO_ENGINE_SUBSTITUTION'; create procedure test_resignal()