Skip to content

Fix getting and setting credentials from Windows #7

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.

Sign up for GitHub

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

Closed
hawkeye116477 wants to merge 2 commits into from
Closed

Fix getting and setting credentials from Windows #7

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
629681c
Fix getting credential from Windows
hawkeye116477 Aug 8, 2026
9161744
[Windows] Fix calculating credential blob size
hawkeye116477 Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions SIL.PasswordStore/Provider/WindowsProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ public void SetPassword(string service, string user, string password)

var passwordLength = string.IsNullOrEmpty(password)
? 0
: (uint)Encoding.UTF8.GetBytes(password).Length + 1;
: (uint)Encoding.Unicode.GetByteCount(password);
var credential = new Credential {
Flags = 0,
Type = CredType.Generic,
TargetName = GetTargetName(service, user),
Comment = null,
CredentialBlobSize = passwordLength * 2,
CredentialBlobSize = passwordLength,
CredentialBlob = Marshal.StringToCoTaskMemUni(password),
Persist = CredPersist.LocalMachine,
AttributeCount = 0
Expand All @@ -58,7 +58,7 @@ public void SetPassword(string service, string user, string password)
var credential = credentialHandle.GetCredential();
return credential?.CredentialBlobSize == 0
? null
: Marshal.PtrToStringUni(credential?.CredentialBlob ?? IntPtr.Zero);
: Marshal.PtrToStringUni(credential?.CredentialBlob ?? IntPtr.Zero, (int)(credential?.CredentialBlobSize ?? 0) / 2).TrimEnd('\0');

@ermshiperete ermshiperete Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please add a unit test that demonstrates the need for this change?

Also, why divide by 2, and trim the null character at the end?

@hawkeye116477 hawkeye116477 Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added trimming null character, cuz Sil when setting password for some reason always added it, while that shouldn't happen (#8 fixes that case).
Dividing by 2 is needed, cuz we need amount of characters to read and each character takes up 2 bytes.

I done that change, cuz had problem with getting some values by Sil set from python's keyring.
So earlier implementation in some cases would require adding null character at the end to read correctly, which I think shouldn't be done, at least didn't saw that at other implementations.

}

var error = Marshal.GetLastWin32Error();
Expand Down