From: Koji Nakamaru Date: Fri, 14 Nov 2025 03:37:00 GMT Subject: Re: [PATCH] Revert "osxkeychain: state to skip unnecessary store operations" Message-ID: In-Reply-To: On Fri, Nov 14, 2025 at 8:30 AM brian m. carlson wrote: > > On 2025-11-12 at 07:01:21, Koji Nakamaru via GitGitGadget wrote: > > From: Koji Nakamaru > > > > 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. Thanks for the explanation. I misunderstood how state[] was intended to work. The current behavior seems rather natural now that I understand it. It might be useful if we could optionally specify that it be discarded under predefined conditions. Also, as you and others previously discussed in [1], this topic is delicate and interesting. [1]: https://lore.kernel.org/git/20240510200114.GC1954863@coredump.intra.peff.net/ -- Koji Nakamaru