From: Jeff King <peff@peff.net>
Date: Tue, 24 Apr 2012 20:40:52 GMT
Subject: Re: [PATCH v2] contrib: add win32 credential-helper
Message-ID: <20120424204052.GC21904@sigill.intra.peff.net>
In-Reply-To: <1334861122-3144-1-git-send-email-kusmabite@gmail.com>

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

> Here's an updated version of my Windows credential-helper.
> 
> The most important difference is that it doesn't suck as much as
> it used to ;)
> 
> Basically, I'm now using the attribute-system to store the meta-data
> for the credentials.
> 
> This passes the test for me, and seems to work OK.

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

> +int main(int argc, char *argv[])
> +{
> [...]
> +	if (!protocol || !host)
> +		return 0;

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

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***

*** Please avoid top-posting. ***

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en