Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bracketed-paste mode messing up password input in client reconnect #48528

Merged
merged 2 commits into from
Apr 8, 2023

Conversation

al13n321
Copy link
Member

@al13n321 al13n321 commented Apr 7, 2023

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix clickhouse-client not accepting copy-pasted password in interactive mode when reconnecting (because of bracketed-paste mode).

Fixes the following scenario:

  1. Connect to a cloud instance: clickhouse client --host <hostname> --secure --port <port> --user default
  2. It asks: Password for user (default):. Paste it in. It connects: :).
  3. Wait 10 minutes.
  4. Do a query: SELECT 1
  5. It asks for password again, paste it in again.
Exception on client:
Code: 36. DB::Exception: Parameters 'default_database', 'user' and 'password' must not contain ASCII control characters. (BAD_ARGUMENTS)

The problem was "bracketed-paste mode": clickhouse client instructed the terminal to surround copy-pasted text with control sequences (like this: \x1b[200~thepassword\x1b[201~), then password input didn't recognize them and got confused.

This PR fixes it by enabling the bracketed-paste mode only for the duration of query input, keeping it disabled at all other times. An alternative would be to make the password input recognize and skip the escape sequences (ascii control characters are not allowed in passwords already anyway, so this wouldn't limit the set of usable passwords). I'm not sure which option is better.

@@ -2239,7 +2236,17 @@ void ClientBase::runInteractive()

do
{
/// Enable bracketed-paste-mode so that we are able to paste multiline queries as a whole.
lr.enableBracketedPaste();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

I'd wrap it in a code block

{
...
}

and use SCOPE_EXIT.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, maybe it can throw.

@alexey-milovidov alexey-milovidov self-assigned this Apr 7, 2023
@alexey-milovidov alexey-milovidov merged commit c77acac into master Apr 8, 2023
139 checks passed
@alexey-milovidov alexey-milovidov deleted the bracketed branch April 8, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants