From 227dfeb524e091b832331a8ab632df01b31227f7 Mon Sep 17 00:00:00 2001 From: Przemek Szalko Date: Wed, 30 Sep 2026 13:31:06 +0200 Subject: [PATCH] Added parser SQL syntax check for invalid VARCHAR column definition for 5.11.x. #issue-671 --- src/Components/AlterOperation.php | 7 + src/Components/DataType.php | 33 ++ tests/Components/DataTypeTest.php | 185 +++++++++++ tests/Parser/AlterStatementTest.php | 1 + ...AlterTableAddColumnInvalidVarcharLength.in | 1 + ...lterTableAddColumnInvalidVarcharLength.out | 301 ++++++++++++++++++ 6 files changed, 528 insertions(+) create mode 100644 tests/Components/DataTypeTest.php create mode 100644 tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.in create mode 100644 tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.out diff --git a/src/Components/AlterOperation.php b/src/Components/AlterOperation.php index a8124a29d..1fbbcd159 100644 --- a/src/Components/AlterOperation.php +++ b/src/Components/AlterOperation.php @@ -337,6 +337,7 @@ public static function parse(Parser $parser, TokensList $list, array $options = * @var int */ $brackets = 0; + $isFirstUnknownToken = true; /** * The state of the parser. @@ -434,6 +435,12 @@ public static function parse(Parser $parser, TokensList $list, array $options = $state = 2; } elseif ($state === 2) { + if ($isFirstUnknownToken && $ret->options->has('ADD') === true && $token->keyword === 'VARCHAR') { + // Validate the column type while preserving the original tokens used to build ALTER. + DataType::parse($parser, clone $list); + } + + $isFirstUnknownToken = false; if (is_string($token->value) || is_int($token->value)) { $arrayKey = $token->value; } else { diff --git a/src/Components/DataType.php b/src/Components/DataType.php index 4f4b521b8..58824a6e7 100644 --- a/src/Components/DataType.php +++ b/src/Components/DataType.php @@ -10,6 +10,7 @@ use PhpMyAdmin\SqlParser\TokensList; use function implode; +use function preg_match; use function strtolower; use function strtoupper; use function trim; @@ -133,7 +134,39 @@ public static function parse(Parser $parser, TokensList $list, array $options = $state = 1; } elseif ($state === 1) { if (($token->type === Token::TYPE_OPERATOR) && ($token->value === '(')) { + $parametersStart = $list->idx + 1; $parameters = ArrayObj::parse($parser, $list); + if ($ret->name === 'VARCHAR') { + $hasLength = false; + for ($idx = $parametersStart; $idx < $list->idx; ++$idx) { + $parameter = $list->tokens[$idx]; + if ( + $parameter->type === Token::TYPE_WHITESPACE || $parameter->type === Token::TYPE_COMMENT + ) { + continue; + } + + // Check the original lexeme: conversion can turn 20.0 or '20' into 20. + if ( + $hasLength || $parameter->type !== Token::TYPE_NUMBER + || preg_match('/^[0-9]+$/D', $parameter->token) !== 1 + ) { + $parser->error('VARCHAR length must be a single nonnegative integer.', $parameter); + $hasLength = true; + break; + } + + $hasLength = true; + } + + if (! $hasLength) { + $parser->error( + 'VARCHAR length must be a single nonnegative integer.', + $list->tokens[$list->idx] ?? null + ); + } + } + ++$list->idx; $ret->parameters = ($ret->name === 'ENUM') || ($ret->name === 'SET') ? $parameters->raw : $parameters->values; diff --git a/tests/Components/DataTypeTest.php b/tests/Components/DataTypeTest.php new file mode 100644 index 000000000..ee74515c4 --- /dev/null +++ b/tests/Components/DataTypeTest.php @@ -0,0 +1,185 @@ +getTokensList($declaration . ' COLLATE utf8mb4_bin NULL')); + self::assertSame([], $parser->errors); + self::assertNotNull($type); + self::assertSame('VARCHAR', $type->name); + self::assertSame([$length], $type->parameters); + self::assertSame('utf8mb4_bin', $type->options->has('COLLATE', true)); + + $parser = new Parser('CREATE TABLE temp_users (phone ' . $declaration . ' COLLATE utf8mb4_bin NULL);'); + self::assertSame([], $parser->errors); + self::assertInstanceOf(CreateStatement::class, $parser->statements[0]); + self::assertIsArray($parser->statements[0]->fields); + $field = $parser->statements[0]->fields[0]; + self::assertNotNull($field->type); + self::assertSame([$length], $field->type->parameters); + self::assertNotNull($field->options); + self::assertTrue($field->options->has('NULL')); + + $parser = new Parser('ALTER TABLE temp_users ADD COLUMN phone ' . $declaration . ' NULL;'); + self::assertSame([], $parser->errors); + self::assertInstanceOf(AlterStatement::class, $parser->statements[0]); + self::assertStringContainsString($length, $parser->statements[0]->build()); + self::assertStringContainsString('NULL', $parser->statements[0]->build()); + } + + /** @return array */ + public static function validLengthProvider(): array + { + return [ + 'ordinary' => ['VARCHAR(20)', '20'], + 'zero' => ['VARCHAR(0)', '0'], + 'leading zeros' => ['VARCHAR(020)', '20'], + 'mixed case and whitespace' => ["vArChAr \n ( 20\t )", '20'], + 'comments' => ['varchar /* type */ ( /* before */ 20 /* after */ )', '20'], + 'line comments' => ["varchar( -- before\n 20 # after\n )", '20'], + ]; + } + + /** + * @dataProvider invalidLengthProvider + */ + #[DataProvider('invalidLengthProvider')] + public function testInvalidLength(string $length, string $invalidToken): void + { + $parser = new Parser(); + DataType::parse($parser, $this->getTokensList('VARCHAR(' . $length . ') NULL')); + self::assertCount(1, $parser->errors); + self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage()); + self::assertInstanceOf(ParserException::class, $parser->errors[0]); + self::assertNotNull($parser->errors[0]->token); + self::assertSame($invalidToken, $parser->errors[0]->token->token); + + foreach (self::columnQueries('VARCHAR(' . $length . ')') as $query) { + $parser = new Parser($query . ' SELECT 1;'); + self::assertCount(1, $parser->errors); + self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage()); + self::assertInstanceOf(ParserException::class, $parser->errors[0]); + self::assertNotNull($parser->errors[0]->token); + self::assertSame($invalidToken, $parser->errors[0]->token->token); + self::assertCount(2, $parser->statements); + self::assertInstanceOf(SelectStatement::class, $parser->statements[1]); + + try { + new Parser($query, true); + self::fail('Strict mode must reject an invalid VARCHAR length.'); + } catch (ParserException $exception) { + self::assertSame(self::LENGTH_ERROR, $exception->getMessage()); + } + } + } + + /** @return array */ + public static function invalidLengthProvider(): array + { + return [ + 'identifier' => ['X', 'X'], + 'quoted string' => ["'20'", "'20'"], + 'double quoted string' => ['"20"', '"20"'], + 'quoted identifier' => ['`20`', '`20`'], + 'fraction' => ['20.0', '20.0'], + 'negative' => ['-20', '-20'], + 'positive sign' => ['+20', '+20'], + 'scientific' => ['2e1', '2e1'], + 'hexadecimal' => ['0x14', '0x14'], + 'binary' => ['0b10100', '0b10100'], + 'expression' => ['1+19', '+19'], + 'nested parentheses' => ['(20)', '('], + 'empty' => ['', ')'], + 'only comments' => [' /* empty */ ', ')'], + 'extra parameter' => ['20,30', ','], + 'trailing comma' => ['20,', ','], + 'separate numbers' => ['20 /* gap */ 30', '30'], + ]; + } + + /** @return list */ + private static function columnQueries(string $declaration): array + { + return [ + 'CREATE TABLE temp_users (phone ' . $declaration . ' NULL);', + 'ALTER TABLE temp_users ADD COLUMN phone ' . $declaration . ' NULL;', + 'ALTER TABLE temp_users ADD phone ' . $declaration . ' NULL;', + ]; + } + + /** + * @param list $parameters + * + * @dataProvider otherTypeProvider + */ + #[DataProvider('otherTypeProvider')] + public function testOtherTypes(string $declaration, array $parameters): void + { + $parser = new Parser(); + $type = DataType::parse($parser, $this->getTokensList($declaration)); + self::assertSame([], $parser->errors); + self::assertNotNull($type); + self::assertSame($parameters, $type->parameters); + } + + /** @return array}> */ + public static function otherTypeProvider(): array + { + return [ + 'enum' => ["ENUM('a','b')", ["'a'", "'b'"]], + 'set' => ["SET('a','b')", ["'a'", "'b'"]], + 'decimal' => ['DECIMAL(10,2)', ['10', '2']], + 'lengthless varchar' => ['VARCHAR', []], + ]; + } + + public function testLengthlessRoutine(): void + { + $parser = new Parser('CREATE PROCEDURE p(IN phone VARCHAR) SELECT phone;'); + self::assertSame([], $parser->errors); + } + + public function testAlterContinuation(): void + { + $parser = new Parser( + 'ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL, ADD COLUMN name VARCHAR(20) NOT NULL;' + ); + self::assertCount(1, $parser->errors); + self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage()); + self::assertInstanceOf(AlterStatement::class, $parser->statements[0]); + self::assertNotNull($parser->statements[0]->altered); + self::assertCount(2, $parser->statements[0]->altered); + self::assertStringContainsString('VARCHAR(20) NOT NULL', $parser->statements[0]->build()); + } + + public function testAlterStringParameters(): void + { + $parser = new Parser("ALTER TABLE temp_users ADD COLUMN e ENUM('a','b'), ADD COLUMN s SET('a','b');"); + self::assertSame([], $parser->errors); + self::assertInstanceOf(AlterStatement::class, $parser->statements[0]); + self::assertNotNull($parser->statements[0]->altered); + self::assertCount(2, $parser->statements[0]->altered); + self::assertStringContainsString("ENUM('a','b')", $parser->statements[0]->build()); + } +} diff --git a/tests/Parser/AlterStatementTest.php b/tests/Parser/AlterStatementTest.php index be628c516..aaa2d0fa2 100644 --- a/tests/Parser/AlterStatementTest.php +++ b/tests/Parser/AlterStatementTest.php @@ -50,6 +50,7 @@ public static function alterProvider(): array ['parser/parseAlterTablePartitionByRange2'], ['parser/parseAlterTableCoalescePartition'], ['parser/parseAlterTableAddColumnWithCheck'], + ['parser/parseAlterTableAddColumnInvalidVarcharLength'], ['parser/parseAlterTableAddSpatialIndex1'], ['parser/parseAlterTableAddUniqueKey1'], ['parser/parseAlterTableAddUniqueKey2'], diff --git a/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.in b/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.in new file mode 100644 index 000000000..217c77aee --- /dev/null +++ b/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.in @@ -0,0 +1 @@ +ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL; \ No newline at end of file diff --git a/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.out b/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.out new file mode 100644 index 000000000..af3d69523 --- /dev/null +++ b/tests/data/parser/parseAlterTableAddColumnInvalidVarcharLength.out @@ -0,0 +1,301 @@ +{ + "query": "ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL;", + "lexer": { + "@type": "PhpMyAdmin\\SqlParser\\Lexer", + "str": "ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL;", + "len": 56, + "last": 56, + "list": { + "@type": "PhpMyAdmin\\SqlParser\\TokensList", + "tokens": [ + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "ALTER", + "value": "ALTER", + "keyword": "ALTER", + "type": 1, + "flags": 3, + "position": 0 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 5 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "TABLE", + "value": "TABLE", + "keyword": "TABLE", + "type": 1, + "flags": 3, + "position": 6 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 11 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "temp_users", + "value": "temp_users", + "keyword": null, + "type": 0, + "flags": 0, + "position": 12 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 22 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "ADD", + "value": "ADD", + "keyword": "ADD", + "type": 1, + "flags": 3, + "position": 23 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 26 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "COLUMN", + "value": "COLUMN", + "keyword": "COLUMN", + "type": 1, + "flags": 3, + "position": 27 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 33 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "phone", + "value": "phone", + "keyword": null, + "type": 0, + "flags": 0, + "position": 34 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 39 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "VARCHAR", + "value": "VARCHAR", + "keyword": "VARCHAR", + "type": 1, + "flags": 11, + "position": 40 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "(", + "value": "(", + "keyword": null, + "type": 2, + "flags": 16, + "position": 47 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "X", + "value": "X", + "keyword": "X", + "type": 1, + "flags": 33, + "position": 48 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": ")", + "value": ")", + "keyword": null, + "type": 2, + "flags": 16, + "position": 49 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": " ", + "value": " ", + "keyword": null, + "type": 3, + "flags": 0, + "position": 50 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": "NULL", + "value": "NULL", + "keyword": "NULL", + "type": 1, + "flags": 3, + "position": 51 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": ";", + "value": ";", + "keyword": null, + "type": 9, + "flags": 0, + "position": 55 + }, + { + "@type": "PhpMyAdmin\\SqlParser\\Token", + "token": null, + "value": null, + "keyword": null, + "type": 9, + "flags": 0, + "position": null + } + ], + "count": 20, + "idx": 20 + }, + "delimiter": ";", + "delimiterLen": 1, + "strict": false, + "errors": [] + }, + "parser": { + "@type": "PhpMyAdmin\\SqlParser\\Parser", + "list": { + "@type": "@1" + }, + "statements": [ + { + "@type": "PhpMyAdmin\\SqlParser\\Statements\\AlterStatement", + "table": { + "@type": "PhpMyAdmin\\SqlParser\\Components\\Expression", + "database": null, + "table": "temp_users", + "column": null, + "expr": "temp_users", + "alias": null, + "function": null, + "subquery": null + }, + "altered": [ + { + "@type": "PhpMyAdmin\\SqlParser\\Components\\AlterOperation", + "ROUTINE_OPTIONS": { + "COMMENT": [ + 1, + "var" + ], + "LANGUAGE SQL": 2, + "CONTAINS SQL": 3, + "NO SQL": 3, + "READS SQL DATA": 3, + "MODIFIES SQL DATA": 3, + "SQL SECURITY": 4, + "DEFINER": 5, + "INVOKER": 5 + }, + "options": { + "@type": "PhpMyAdmin\\SqlParser\\Components\\OptionsArray", + "options": { + "1": "ADD", + "2": "COLUMN" + } + }, + "field": { + "@type": "PhpMyAdmin\\SqlParser\\Components\\Expression", + "database": null, + "table": null, + "column": "phone", + "expr": "phone", + "alias": null, + "function": null, + "subquery": null + }, + "partitions": null, + "unknown": [ + { + "@type": "@14" + }, + { + "@type": "@15" + }, + { + "@type": "@16" + }, + { + "@type": "@17" + }, + { + "@type": "@18" + }, + { + "@type": "@19" + } + ] + } + ], + "options": { + "@type": "PhpMyAdmin\\SqlParser\\Components\\OptionsArray", + "options": { + "3": "TABLE" + } + }, + "first": 0, + "last": 18 + } + ], + "brackets": 0, + "strict": false, + "errors": [] + }, + "errors": { + "lexer": [], + "parser": [ + [ + "VARCHAR length must be a single nonnegative integer.", + { + "@type": "@16" + }, + 0 + ] + ] + } +} \ No newline at end of file