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

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Nov 13, 2025, 23:30 UTC
Message-ID: <aRZqLp__WdA4hbuD@fruit.crustytoothpaste.net>
In-Reply-To: <pull.1998.git.1762930881599.gitgitgadget@gmail.com>

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

Show 12 quoted lines

> From: Koji Nakamaru <koji.nakamaru@gree.net>
> 
> 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