Fix MySQL lexer edge cases

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;
 			}
 		}

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;
 	}

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() );

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();

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() );
 	}

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()