Allow tilde (~) in passwords#498
Allow tilde (~) in passwords#498slabko merged 1 commit intoClickHouse:masterfrom slabko:fix-tilde-in-passwords
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the restriction on tilde (~) characters in passwords by updating the Base64 encoding logic and adds integration tests covering passwords with various special characters.
- Adjusts Base64 encoder configuration to prevent line breaks and use standard encoding.
- Adds an end-to-end test (
authentication_it.cpp) that creates users with passwords containing a wide range of special characters (including~) and verifies authentication.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| driver/connection.cpp | Updated buildCredentialsString to disable line breaks and use standard Base64 encoding. |
| driver/test/authentication_it.cpp | New integration test that exercises password encoding/decoding for special characters. |
Files not reviewed (1)
- driver/test/CMakeLists.txt: Language not supported
driver/connection.cpp
Outdated
| base64_encoder << username << ":" << password; | ||
| base64_encoder.close(); | ||
| base64_encoder.close(); // close to flush before reading the value | ||
| std::string ret = user_password_base64.str(); |
There was a problem hiding this comment.
The local variable ret is assigned but never used. Consider returning ret directly or removing this variable to avoid the redundant call to str().
| std::string ret = user_password_base64.str(); |
There was a problem hiding this comment.
Pull Request Overview
This PR enables passwords containing tildes by switching to standard Base64 encoding without line breaks and adds an end-to-end test suite for special-character passwords.
- Switched
buildCredentialsString()to use default Base64 encoder and disabled line wrapping - Introduced
AuthenticationTest::PasswordEncodingto create users with various special-character passwords (including~) - Generated sliding-window passwords up to 32 bytes to cover all printable ASCII
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| driver/connection.cpp | Removed URL-safe Base64 flag and set line length to 0 when building credentials |
| driver/test/authentication_it.cpp | Added integration tests for passwords with special characters, including tilde |
Files not reviewed (1)
- driver/test/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (1)
driver/test/authentication_it.cpp:90
- [nitpick] The test name
PasswordEncodingmay be misleading since it verifies authentication with special-character passwords. Consider renaming it to something likePasswordAuthenticationorSpecialCharacterPasswordAuth.
TEST_F(AuthenticationTest, PasswordEncoding)
| std::string Connection::buildCredentialsString() const { | ||
| std::ostringstream user_password_base64; | ||
| Poco::Base64Encoder base64_encoder(user_password_base64, Poco::BASE64_URL_ENCODING); | ||
| Poco::Base64Encoder base64_encoder(user_password_base64); |
There was a problem hiding this comment.
Removing the URL-safe Base64 flag changes the output alphabet; consumers expecting URL-safe encoding may break. Consider documenting this compatibility change or providing an option for URL-safe encoding.
There was a problem hiding this comment.
This is exactly what we want here -- to have a normal, not a URL-safe, encoding
driver/test/authentication_it.cpp
Outdated
| } | ||
|
|
||
| // Cleanup works because all failures are non-fatal: | ||
| // EXCEPT is used instead of ASSERT, and ADD_FAILURE instead of FAIL after these |
There was a problem hiding this comment.
Typo in comment: "EXCEPT" should be "EXPECT" to match the Google Test macro.
| // EXCEPT is used instead of ASSERT, and ADD_FAILURE instead of FAIL after these | |
| // EXPECT is used instead of ASSERT, and ADD_FAILURE instead of FAIL after these |
The driver previously failed when a password contained a tilde (~). See: https://clickhouse.com/docs/knowledgebase/ODBC-authentication-failed-error-using-PowerBI-CH-connector This change removes that limitation and adds tests for passwords with various special characters.
The driver previously failed when a password contained a tilde (~).
See: https://clickhouse.com/docs/knowledgebase/ODBC-authentication-failed-error-using-PowerBI-CH-connector
This change removes that limitation and adds tests for passwords with various special characters.