diff --git a/changelog.md b/changelog.md index 8a79c2d90..d0cd2205d 100644 --- a/changelog.md +++ b/changelog.md @@ -14,6 +14,7 @@ Bug Fixes * Improve query cancellation on control-c. * Improve refresh of some format strings in the toolbar. * Improve keyring storage, requiring re-entering most keyring passwords. +* Improve sentinel value for `--password` without argument. Internal diff --git a/mycli/main.py b/mycli/main.py index 9bf6ecac6..885807fa0 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -102,6 +102,7 @@ DEFAULT_HEIGHT = 25 MIN_COMPLETION_TRIGGER = 1 MAX_MULTILINE_BATCH_STATEMENT = 5000 +EMPTY_PASSWORD_FLAG_SENTINEL = -1 @Condition @@ -133,6 +134,23 @@ def complete_while_typing_filter() -> bool: return not bool(re.search(r'[\s!-/:-@\[-^\{-~]', last_word)) +class IntOrStringClickParamType(click.ParamType): + name = 'string' # display as STRING in helpdoc + + def convert(self, value, param, ctx): + if isinstance(value, int): + return value + elif isinstance(value, str): + return value + elif value is None: + return value + else: + self.fail('Not a valid password string', param, ctx) + + +INT_OR_STRING_CLICK_TYPE = IntOrStringClickParamType() + + class MyCli: default_prompt = "\\t \\u@\\h:\\d> " default_prompt_splitln = "\\u@\\h\\n(\\t):\\d>" @@ -563,7 +581,7 @@ def connect( self, database: str | None = "", user: str | None = "", - passwd: str | None = None, + passwd: str | int | None = None, host: str | None = "", port: str | int | None = "", socket: str | None = "", @@ -622,7 +640,7 @@ def connect( or guess_socket_location() ) - passwd = passwd if isinstance(passwd, str) else cnf["password"] + passwd = passwd if isinstance(passwd, (str, int)) else cnf["password"] # default_character_set doesn't check in self.config_without_package_defaults, because the # option already existed before the my.cnf deprecation. For the same reason, @@ -699,10 +717,13 @@ def connect( keyring_retrieved_cleanly = True # prompt for password if requested by user - if passwd == "MYCLI_ASK_PASSWORD": + if passwd == EMPTY_PASSWORD_FLAG_SENTINEL: passwd = click.prompt(f"Enter password for {user}", hide_input=True, show_default=False, default='', type=str, err=True) keyring_retrieved_cleanly = False + # should not fail, but will help the typechecker + assert not isinstance(passwd, int) + connection_info: dict[Any, Any] = { "database": database, "user": user, @@ -1886,9 +1907,9 @@ def get_last_query(self) -> str | None: "--password", "password", is_flag=False, - flag_value="MYCLI_ASK_PASSWORD", - type=str, - help="Prompt for (or enter in cleartext) password to connect to the database.", + flag_value=EMPTY_PASSWORD_FLAG_SENTINEL, + type=INT_OR_STRING_CLICK_TYPE, + help="Prompt for (or pass in cleartext) the password to connect to the database.", ) @click.option("--ssh-user", help="User name to connect to ssh server.") @click.option("--ssh-host", help="Host name to connect to ssh server.") @@ -1986,7 +2007,7 @@ def cli( host: str | None, port: int | None, socket: str | None, - password: str | None, + password: str | int | None, dbname: str | None, verbose: bool, prompt: str | None, @@ -2067,7 +2088,7 @@ def get_password_from_file(password_file: str | None) -> str | None: # if the password value looks like a DSN, treat it as such and # prompt for password - if database is None and password is not None and "://" in password: + if database is None and isinstance(password, str) and "://" in password: # check if the scheme is valid. We do not actually have any logic for these, but # it will most usefully catch the case where we erroneously catch someone's # password, and give them an easy error message to follow / report @@ -2076,7 +2097,7 @@ def get_password_from_file(password_file: str | None) -> str | None: click.secho(f"Error: Unknown connection scheme provided for DSN URI ({scheme}://)", err=True, fg="red") sys.exit(1) database = password - password = "MYCLI_ASK_PASSWORD" + password = EMPTY_PASSWORD_FLAG_SENTINEL # if the password is not specified try to set it using the password_file option if password is None and password_file: @@ -2174,10 +2195,12 @@ def get_password_from_file(password_file: str | None) -> str | None: dsn_uri = None # Treat the database argument as a DSN alias only if it matches a configured alias + # todo why is port tested but not socket? + truthy_password = password not in (None, EMPTY_PASSWORD_FLAG_SENTINEL) if ( database and "://" not in database - and not any([user, password, host, port, login_path]) + and not any([user, truthy_password, host, port, login_path]) and database in mycli.config.get("alias_dsn", {}) ): dsn_alias, database = database, "" @@ -2208,6 +2231,7 @@ def get_password_from_file(password_file: str | None) -> str | None: database = uri.path[1:] # ignore the leading fwd slash if not user and uri.username is not None: user = unquote(uri.username) + # todo: rationalize the behavior of empty-string passwords here if not password and uri.password is not None: password = unquote(uri.password) if not host: diff --git a/test/test_main.py b/test/test_main.py index bcfce05ef..f135fe927 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -13,7 +13,7 @@ from click.testing import CliRunner from pymysql.err import OperationalError -from mycli.main import MyCli, cli, thanks_picker +from mycli.main import EMPTY_PASSWORD_FLAG_SENTINEL, MyCli, cli, thanks_picker from mycli.packages.parseutils import is_valid_connection_scheme import mycli.packages.special from mycli.packages.special.main import COMMANDS as SPECIAL_COMMANDS @@ -1032,6 +1032,64 @@ def run_query(self, query, new_line=True): assert MockMyCli.connect_args['character_set'] == 'utf8mb3' +def test_password_flag_uses_sentinel(monkeypatch): + class Formatter: + format_name = None + + class Logger: + def debug(self, *args, **args_dict): + pass + + def warning(self, *args, **args_dict): + pass + + class MockMyCli: + config = { + 'main': {}, + 'alias_dsn': {}, + 'connection': { + 'default_keepalive_ticks': 0, + }, + } + + def __init__(self, **_args): + self.logger = Logger() + self.destructive_warning = False + self.main_formatter = Formatter() + self.redirect_formatter = Formatter() + self.ssl_mode = 'auto' + self.my_cnf = {'client': {}, 'mysqld': {}} + self.default_keepalive_ticks = 0 + + def connect(self, **args): + MockMyCli.connect_args = args + + def run_query(self, query, new_line=True): + pass + + import mycli.main + + monkeypatch.setattr(mycli.main, 'MyCli', MockMyCli) + runner = CliRunner() + + result = runner.invoke( + mycli.main.cli, + args=[ + '--user', + 'user', + '--host', + 'localhost', + '--port', + '3306', + '--database', + 'database', + '--password', + ], + ) + assert result.exit_code == 0, result.output + ' ' + str(result.exception) + assert MockMyCli.connect_args['passwd'] == EMPTY_PASSWORD_FLAG_SENTINEL + + def test_ssh_config(monkeypatch): # Setup classes to mock mycli.main.MyCli class Formatter: