Password Prompt Fix#185
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the password prompt logic to treat URI-encoded <password> placeholders as empty passwords, preventing a series of dots from appearing in the input box.
- Decode and inspect the connection string’s password; if it equals the literal
<password>, use an empty value in the prompt. - Retain existing value otherwise.
- Ensures user isn’t shown a masked placeholder when a placeholder password is supplied.
Comments suppressed due to low confidence (2)
src/commands/newConnection/PromptPasswordStep.ts:21
- This inline conditional is quite complex. Consider extracting the decoded password check into a well-named variable or helper function for readability and testability.
value: decodeURIComponent(new ConnectionString(context.connectionString!).password?.trim() || '') === '<password>' ? '' : context.password,
src/commands/newConnection/PromptPasswordStep.ts:21
- Add a unit test for scenarios where the connection string password is
%3Cpassword%3E, ensuring the prompt value is empty rather than masked.
value: decodeURIComponent(new ConnectionString(context.connectionString!).password?.trim() || '') === '<password>' ? '' : context.password,
| // Parse the connection string | ||
| const parsedCS = new ConnectionString(params.connectionString!); | ||
| if (parsedCS.password && decodeURIComponent(parsedCS.password.trim()) === '<password>') { | ||
| parsedCS.password = ''; |
There was a problem hiding this comment.
In line 161, the parsed connection string (parsedCS) is stored in a storage item, and I suppose this allows the connection to be used after reopening the VSCode window. I'm unsure if setting the password as an empty string would affect the current experience. Could you check whether your change might cause the connection to become invalid after reopening VSCode please?
tnaum-ms
left a comment
There was a problem hiding this comment.
@nishthaAhujaa thanks for the contribution. I get the idea, but I’d kindly ask you to reconsider this change.
<password> is unusual, but still a valid value. Using <password> it as a signal feels risky, since someone might actually use it as their password. The current way of leaving out the password field from the connection string already gives the right behavior.
A better long-term fix would be to add a show/hide toggle to the input UI. We both know folks on extension teams, so we could reach out, ask for support, upvotes, or ideas. This issue would be a good starting point: microsoft/vscode#221397
Thanks again. I’d love to see this move forward in a more solid direction.
|
This issue has been resolved by:
|
When the connection string uses username:, we want the password prompt to come. Hence, treating as empty password