Thread (3 messages) 3 messages, 3 authors, 2025-11-14

Re: [PATCH] Revert "osxkeychain: state to skip unnecessary store operations"

From: brian m. carlson <hidden>
Date: 2025-11-13 23:30:56

On 2025-11-12 at 07:01:21, Koji Nakamaru via GitGitGadget wrote:
From: Koji Nakamaru <redacted>

This reverts commit e1ab45b2dab51f94db9548666dfd7af626d2aa7e.

That commit was trying to skip to store a credential returned by
"git-credential-osxkeychain get" by setting
"state[]=osxkeychain:seen=1". However, this state[] is kept even if a
credential returned by "git-credential-osxkeychain get" is invalid and
another subsequent helper's "get" returns a valid credential. Another
subsequent helper (such as [1]) may expect git-credential-osxkeychain to
store the valid credential so that "store" cannot be skipped by just
checking "state[]=osxkeychain:seen=1".
I believe the intended approach here is that if we do a get and the
credential is invalid, we return the same state[] header to erase, but
we should not send it to subsequent gets for a new credential.  However,
we do need to send it to subsequent gets (which will not have an
intervening erase) if this is a multistage request because otherwise
multistage requests will not be able to keep state, which NTLM and
Kerberos require.  Does that make sense?

My guess is that the problem here is that we reuse the credential
structure without resetting it somewhere in the HTTP code rather than a
problem in this particular helper.  That is probably my fault, but in my
defence I would not say that the structure of the HTTP code is very easy
to follow.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help