Re: [PATCH v2] contrib: add win32 credential-helper

On Thu, Apr 19, 2012 at 08:45:22PM +0200, Erik Faye-Lund wrote:

This looks much better. As usual, I can't comment on the Windows-y parts, but the credential logic looks good. One minor comment:

You could have a protocol that does not have a "host" component. Git will produce this for a cached certificate password (which will then always have a "path" field). I don't know if anybody is using it to cache certificate passwords, and I admit that it is not very well tested by me, either. So I'm not sure anybody will actually care.

The credential-store helper uses this logic:

  $ sed -n '70,79p' credential-store.c
        /*
         * Sanity check that what we are storing is actually sensible.
         * In particular, we can't make a URL without a protocol field.
         * Without either a host or pathname (depending on the scheme),
         * we have no primary key. And without a username and password,
         * we are not actually storing a credential.
         */
        if (!c->protocol || !(c->host || c->path) ||
            !c->username || !c->password)
                return;

I'm pretty sure the OS X helper does not handle certificate passwords at all. It is harder there, because I map directly to native protocol types, and I haven't checked to see whether OS X handles this type. But since you are just storing whatever protocol information git hands you, I think for you it is just a matter of letting it through (and handling the "!host" case when writing).

-Peff