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 handling of osxkeychain addtional keys (#391) #392

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

pgpx
Copy link
Contributor

@pgpx pgpx commented Nov 25, 2024

Fixes #391

Fix handling of osxkeychain addtional keys (#391)

  • git-credential-helper v2.45 < v2.47.0 can output additional garbage bytes after the username.
  • git-credential-helper >= v2.46.0 outputs additional capability[] and state keys.

git-credential-helper v2.45 < v2.47.0 output additional garbage bytes after the username.
git-credential-helper >= v2.46.0 outputs additional `capability[]` and `state` keys.
@pgpx pgpx force-pushed the fix-osxkeychain-keys branch from 608991f to b597f92 Compare November 25, 2024 13:39
@shcheklein
Copy link
Member

hi @pgpx , thanks a lot for addressing and for including tests!

please check the tests - seems mypy / linters are failing.

@shcheklein shcheklein added the bug Something isn't working label Nov 25, 2024
@pgpx
Copy link
Contributor Author

pgpx commented Nov 25, 2024

@shcheklein - oh sorry, looks ok now!

Note the _strip_garbage_bytes method is somewhat dubious since I'm unsure as to what bytes could be added by older versions of git to the username value. I've only ever seen a series of 0-bytes, and so have assumed that everything from the first 0-byte could be discarded since a 0-byte should never be part of a username. However, you could drop that call entirely if you wanted, because git v2.47.0 no longer has that problem anyway (only 2.45.0 - 2.46.0, maybe 2.46.1,2 are affected)

@shcheklein
Copy link
Member

thanks, I think that's fine - let's keep it for now

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

thanks @pgpx !

@shcheklein shcheklein merged commit 93861d0 into iterative:main Nov 25, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git-credential-osxkeychain >= 2.45.0 now adds keys/bytes that break the Credential class
2 participants