Fix PostgreSQL product creation with special characters in DB name#4830
Conversation
Introduces is_valid_postgresql_db_name, a permissive validator that rejects database names that would break a CREATE DATABASE statement or corrupt a connection string (quotes, semicolons, whitespace, null and control characters, or names longer than PostgreSQL's 63-byte identifier limit). The validator is intentionally permissive: names that are legal only as quoted identifiers (e.g. 'test-product', '1team', 'user') are accepted because CodeChecker will quote the identifier when issuing CREATE DATABASE. See the companion commit wiring the validator into addProduct() and fixing the CREATE DATABASE statement. Unit-tested in test_request_routing.py.
The PostgreSQL path of _create_database() interpolated the user-supplied
database name directly into a CREATE DATABASE statement via an f-string.
This caused syntax errors for any name that is not a legal unquoted
PostgreSQL identifier - in particular names containing a dash
(e.g. 'test-product') or starting with a digit (e.g. '1team') - both
reported by users via the GUI's product creation dialog.
SQLAlchemy does not auto-quote identifiers in free-form text() clauses,
so the fix has two parts:
* Quote the identifier explicitly using the dialect's
IdentifierPreparer before embedding it in the statement. This
produces a properly double-quoted name such as CREATE DATABASE
"test-product", which PostgreSQL accepts.
* Validate the database name in addProduct() using the new
is_valid_postgresql_db_name() helper, so that inputs containing
quotes, semicolons, whitespace, control characters, or that
exceed PostgreSQL's 63-byte identifier limit are rejected with a
clear error message before any SQL is issued, rather than crashing
later with an opaque driver error.
bruntib
left a comment
There was a problem hiding this comment.
Thanks for the fix, it's a really nice implementation.
I have only a minor comment before it can be merged.
| return True | ||
|
|
||
|
|
||
| def is_valid_postgresql_db_name(db_name): |
There was a problem hiding this comment.
Please, replace this function to codechecker_common.util module. This routing.py is for other purposes.
Per Ericsson#4830 review feedback: is_valid_postgresql_db_name belongs with the other generic helpers in codechecker_common.util, not in the web server's routing module. The function and its tests are unchanged; only the import paths in product_server.py and test_request_routing.py are adjusted.
|
Done, moved is_valid_postgresql_db_name to codechecker_common.util and updated the imports in product_server.py and test_request_routing.py. |
Per Ericsson#4830 review feedback: is_valid_postgresql_db_name belongs with the other generic helpers in codechecker_common.util, not in the web server's routing module. The function and its tests are unchanged; only the import paths in product_server.py and test_request_routing.py are adjusted.
62ee30d to
b43bdd0
Compare
| ('DummyProduct', '6.0', 'FoobarService')) | ||
|
|
||
|
|
||
| class PostgresqlDbNameValidationTest(unittest.TestCase): |
There was a problem hiding this comment.
Sorry, I've just noticed that this test is also among the tests of the routing module. Could you place this to a new test file? Thank you!
Per Ericsson#4830 review feedback: is_valid_postgresql_db_name belongs with the other generic helpers in codechecker_common.util, not in the web server's routing module. The function and its tests are unchanged; only the import paths in product_server.py and test_request_routing.py are adjusted.
b43bdd0 to
b105629
Compare
|
Moved the test class to a new |
Closes #4805
Fixes
CREATE DATABASEfailing when the product database name contains a dash or starts with a digit (e.g.test-product,1team,42my-database). The old code interpolated the name into an f-string, andtext()doesn't quote identifiers — so names that aren't legal unquoted PostgreSQL identifiers produced a syntax error.Two commits:
routing: add PostgreSQL database name validator— newis_valid_postgresql_db_name()helper, permissively accepting anything safe to use as a quoted identifier, with 6 unit tests.product: fix CREATE DATABASE failure on special characters— quote the identifier via SQLAlchemy'sIdentifierPreparerin_create_database, and wire the new validator intoaddProductso bad input fails early with a clear message.Verified manually via the GUI:
test-product,1team,42my-database, and normal names all create successfully;bad"nameis rejected client-side with a readable error. All 31 existing unit tests still pass.