lucasfang opened a new pull request, #264: URL: https://github.com/apache/paimon-cpp/pull/264
### Purpose Linked issue: close #xxx `FileSystemCatalog` builds every path it touches from the database name and from the components parsed out of the table name, and it did so without checking that those names are usable as a single path component. This PR adds that validation. The check lives in `CatalogUtils` and is applied in `FileSystemCatalog::NewDatabasePath` and `FileSystemCatalog::NewDataTablePath`. Those two functions are the only place where catalog paths are built, so all entry points (create, drop, rename, get, `List*`, `*Exists`) are covered at once and future entry points inherit the check. A name is rejected when it is empty or whitespace-only, is exactly `.` or `..`, contains `/` or `\`, or contains control characters. Validation is purely lexical, so it behaves identically for `local`, `oss://`, `hdfs://` and any other `FileSystem`, needs no extra IO and has no symlink TOCTOU semantics. Names that merely contain a dot stay valid (`my.db`, `a..b`), as do non-ascii names. The rule set, the error wording and the scope follow the equivalent change in apache/paimon-rust (`validate_identifier_name`, PR #334): only the filesystem catalog is affected and the helpers stay in the internal `CatalogUtils` rather than in a public header. `RestCatalog` is intentionally left unchanged, because `ResourcePaths` url-encodes every URL segment. One C++-specific difference: `NewDataTablePath` uses the parsed table name rather than the raw object string, so `CatalogUtils::CheckValidTableName` validates each parsed component (data table name, branch name, system table name). `FileSystemCatalog::ListSnapshots` gets the same check on its `branch` argument, since that value flows into `BranchManager::BranchPath`. Behavior change to be aware of: such names used to be accepted silently and now return `Status::Invalid` before any file system access happens. ### Tests - `FileSystemCatalogTest.TestIdentifierNameValidationRules`: table-driven coverage of the rejected forms (`""`, `" "`, `"."`, `".."`, a name with a slash, a name with a backslash, a name with `\n`, a name with `\0`), checked through both `CreateDatabase` and `CreateTable` against the expected error message. The same test asserts that `my.db`, `a..b` and `数据` remain creatable as databases and that `orders` and `订单` remain creatable as tables, so the rules are not over-tightened. - `FileSystemCatalogTest.TestRejectInvalidNames`: asserts that a rejected name is refused by every catalog entry point (`CreateDatabase`, `DatabaseExists`, `ListTables`, `DropDatabase`, `CreateTable`, `TableExists`, `GetTableLocation`, `GetTable`, `DropTable`, `RenameTable`, `ListSnapshots`, plus the branch component of a table name), that nothing is created or deleted on those paths, that `GetDatabaseLocation` returns an empty string, and that a legitimate table in the same warehouse is untouched. - Full local run: `paimon-core-test --gtest_filter='FileSystemCatalogTest.*'` passes 26/26, and the whole `unittest` target passes 28/28 test binaries, since name validation now sits on every catalog code path. ### API and Format No public API signature, ABI or storage format change. `include/paimon/catalog/catalog.h` only gets a doc comment update: `GetDatabaseLocation` returns `std::string` and has no error channel, so it now returns an empty string for a name that does not form a valid location, which matches the existing `RestCatalog::GetDatabaseLocation` convention for unknown databases. The private static `FileSystemCatalog::NewDatabasePath` changes from `std::string` to `Result<std::string>`, which is internal only. The new `CatalogUtils::CheckValidDatabaseName`, `CheckValidTableName` and `CheckValidBranchName` live in `src/paimon/core/catalog/catalog_utils.h` and are not exported as public API. ### Documentation No documentation change needed: this is a robustness fix rather than a new feature, and the only user-visible contract update is the `GetDatabaseLocation` doc comment described above. ### Generative AI tooling Generated-by: Qoder -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
