From 65cac8e945b760d16b83663ec1152a6a1049b499 Mon Sep 17 00:00:00 2001 From: WenyXu Date: Mon, 27 Nov 2023 11:27:38 +0000 Subject: [PATCH] fix: fix name verifying --- src/common/meta/src/key.rs | 2 +- src/common/procedure/Cargo.toml | 3 +- src/operator/src/statement/ddl.rs | 52 ++++++++++++++++++- .../common/alter/rename_table.result | 4 ++ .../standalone/common/alter/rename_table.sql | 2 + .../standalone/common/create/create.result | 4 ++ .../cases/standalone/common/create/create.sql | 2 + .../common/create/create_database.result | 4 ++ .../common/create/create_database.sql | 2 + 9 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/common/meta/src/key.rs b/src/common/meta/src/key.rs index c66ead5b8002..39a942d5f3bd 100644 --- a/src/common/meta/src/key.rs +++ b/src/common/meta/src/key.rs @@ -89,7 +89,7 @@ use crate::DatanodeId; pub const REMOVED_PREFIX: &str = "__removed"; -const NAME_PATTERN: &str = r"[a-zA-Z_:-][a-zA-Z0-9_:\-\.]*"; +pub const NAME_PATTERN: &str = r"[a-zA-Z_:-][a-zA-Z0-9_:\-\.]*"; const DATANODE_TABLE_KEY_PREFIX: &str = "__dn_table"; const TABLE_REGION_KEY_PREFIX: &str = "__table_region"; diff --git a/src/common/procedure/Cargo.toml b/src/common/procedure/Cargo.toml index b029c52ce7bd..ece649434b88 100644 --- a/src/common/procedure/Cargo.toml +++ b/src/common/procedure/Cargo.toml @@ -4,9 +4,8 @@ version.workspace = true edition.workspace = true license.workspace = true - [features] -testing=[] +testing = [] [dependencies] async-stream.workspace = true diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 7051a9c626a8..36cb4d93ab1b 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -24,6 +24,7 @@ use common_catalog::format_full_table_name; use common_meta::cache_invalidator::Context; use common_meta::ddl::ExecutorContext; use common_meta::key::schema_name::{SchemaNameKey, SchemaNameValue}; +use common_meta::key::NAME_PATTERN; use common_meta::rpc::ddl::{DdlTask, SubmitDdlTaskRequest, SubmitDdlTaskResponse}; use common_meta::rpc::router::{Partition, Partition as MetaPartition}; use common_meta::table_name::TableName; @@ -31,7 +32,9 @@ use common_query::Output; use common_telemetry::{info, tracing}; use datatypes::prelude::ConcreteDataType; use datatypes::schema::RawSchema; +use lazy_static::lazy_static; use partition::partition::{PartitionBound, PartitionDef}; +use regex::Regex; use session::context::QueryContextRef; use snafu::{ensure, OptionExt, ResultExt}; use sql::ast::Value as SqlValue; @@ -41,7 +44,7 @@ use sql::statements::sql_value_to_value; use sql::MAXVALUE; use table::dist_table::DistTable; use table::metadata::{self, RawTableInfo, RawTableMeta, TableId, TableInfo, TableType}; -use table::requests::{AlterTableRequest, TableOptions}; +use table::requests::{AlterKind, AlterTableRequest, TableOptions}; use table::TableRef; use super::StatementExecutor; @@ -53,6 +56,10 @@ use crate::error::{ }; use crate::expr_factory; +lazy_static! { + static ref NAME_PATTERN_REG: Regex = Regex::new(&format!("^{NAME_PATTERN}$")).unwrap(); +} + impl StatementExecutor { pub fn catalog_manager(&self) -> CatalogManagerRef { self.catalog_manager.clone() @@ -122,6 +129,13 @@ impl StatementExecutor { }; } + ensure!( + NAME_PATTERN_REG.is_match(&create_table.table_name), + error::UnexpectedSnafu { + violated: format!("Invalid table name: {}", create_table.table_name) + } + ); + let table_name = TableName::new( &create_table.catalog_name, &create_table.schema_name, @@ -213,7 +227,20 @@ impl StatementExecutor { let request: AlterTableRequest = common_grpc_expr::alter_expr_to_request(table_id, expr) .context(AlterExprToRequestSnafu)?; - let AlterTableRequest { table_name, .. } = &request; + let AlterTableRequest { + table_name, + alter_kind, + .. + } = &request; + + if let AlterKind::RenameTable { new_table_name } = alter_kind { + ensure!( + NAME_PATTERN_REG.is_match(new_table_name), + error::UnexpectedSnafu { + violated: format!("Invalid table name: {}", new_table_name) + } + ); + } let _ = table_info .meta @@ -359,6 +386,20 @@ impl StatementExecutor { database: &str, create_if_not_exists: bool, ) -> Result { + ensure!( + NAME_PATTERN_REG.is_match(catalog), + error::UnexpectedSnafu { + violated: format!("Invalid catalog name: {}", catalog) + } + ); + + ensure!( + NAME_PATTERN_REG.is_match(database), + error::UnexpectedSnafu { + violated: format!("Invalid database name: {}", database) + } + ); + // TODO(weny): considers executing it in the procedures. let schema_key = SchemaNameKey::new(catalog, database); let exists = self @@ -587,6 +628,13 @@ mod test { use super::*; use crate::expr_factory; + #[test] + fn test_name_is_match() { + assert!(!NAME_PATTERN_REG.is_match("/adaf")); + assert!(!NAME_PATTERN_REG.is_match("🈲")); + assert!(NAME_PATTERN_REG.is_match("hello")); + } + #[test] fn test_validate_partition_columns() { let create_table = CreateTableExpr { diff --git a/tests/cases/standalone/common/alter/rename_table.result b/tests/cases/standalone/common/alter/rename_table.result index bffd78ad94ab..40982666dc05 100644 --- a/tests/cases/standalone/common/alter/rename_table.result +++ b/tests/cases/standalone/common/alter/rename_table.result @@ -25,6 +25,10 @@ SELECT * from t; | | 1970-01-01T00:00:00.004 | +---+-------------------------+ +ALTER TABLE t RENAME 'Hi👋'; + +Error: 1002(Unexpected), Unexpected, violated: Invalid table name: Hi👋 + ALTER TABLE t RENAME new_table; Affected Rows: 0 diff --git a/tests/cases/standalone/common/alter/rename_table.sql b/tests/cases/standalone/common/alter/rename_table.sql index 0f6ad52868a0..85c3e8b4b9ff 100644 --- a/tests/cases/standalone/common/alter/rename_table.sql +++ b/tests/cases/standalone/common/alter/rename_table.sql @@ -6,6 +6,8 @@ INSERT INTO TABLE t VALUES (1, 1), (3, 3), (NULL, 4); SELECT * from t; +ALTER TABLE t RENAME 'Hi👋'; + ALTER TABLE t RENAME new_table; DESC TABLE t; diff --git a/tests/cases/standalone/common/create/create.result b/tests/cases/standalone/common/create/create.result index 4acfca83635e..08e4b658de2b 100644 --- a/tests/cases/standalone/common/create/create.result +++ b/tests/cases/standalone/common/create/create.result @@ -50,6 +50,10 @@ CREATE TABLE test2 (i INTEGER, j TIMESTAMP TIME INDEX); Error: 4000(TableAlreadyExists), Table already exists: `greptime.public.test2` +CREATE TABLE 'N.~' (i TIMESTAMP TIME INDEX); + +Error: 1002(Unexpected), Unexpected, violated: Invalid table name: N.~ + DESC TABLE integers; +--------+----------------------+-----+------+---------+---------------+ diff --git a/tests/cases/standalone/common/create/create.sql b/tests/cases/standalone/common/create/create.sql index 315a3aad05b2..e350ef84ed46 100644 --- a/tests/cases/standalone/common/create/create.sql +++ b/tests/cases/standalone/common/create/create.sql @@ -24,6 +24,8 @@ CREATE TABLE test2 (i INTEGER, j TIMESTAMP TIME INDEX); CREATE TABLE test2 (i INTEGER, j TIMESTAMP TIME INDEX); +CREATE TABLE 'N.~' (i TIMESTAMP TIME INDEX); + DESC TABLE integers; DESC TABLE test1; diff --git a/tests/cases/standalone/common/create/create_database.result b/tests/cases/standalone/common/create/create_database.result index c684869e9d24..45c47cb00700 100644 --- a/tests/cases/standalone/common/create/create_database.result +++ b/tests/cases/standalone/common/create/create_database.result @@ -6,6 +6,10 @@ create database 'illegal-database'; Affected Rows: 1 +create database '㊙️database'; + +Error: 1002(Unexpected), Unexpected, violated: Invalid database name: ㊙️database + show databases; +--------------------+ diff --git a/tests/cases/standalone/common/create/create_database.sql b/tests/cases/standalone/common/create/create_database.sql index 90f5de9edce5..0116e2bcce1a 100644 --- a/tests/cases/standalone/common/create/create_database.sql +++ b/tests/cases/standalone/common/create/create_database.sql @@ -2,4 +2,6 @@ create database illegal-database; create database 'illegal-database'; +create database '㊙️database'; + show databases;