From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Fri, 22 Jun 2012 16:29:34 GMT
Subject: Re: [PATCH v2] contrib: add win32 credential-helper
Message-ID: <CABPQNSZLVuDKi0k0fhW+WyvCG_ShDETt_0i+BnmDUUE_su9Suw@mail.gmail.com>
In-Reply-To: <20120424204052.GC21904@sigill.intra.peff.net>

On Tue, Apr 24, 2012 at 10:40 PM, Jeff King <peff@peff.net> wrote:
> 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.

Right. I copied this logic from the OSX helper.

> 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).

Yeah, I agree. This on top should do it:

diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index e63575f..cbaec5f 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -232,16 +232,20 @@ static void store_credential(void)
 	cred.CredentialBlobSize = auth_buf_size;
 	cred.CredentialBlob = auth_buf;
 	cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
-	cred.AttributeCount = 2;
+	cred.AttributeCount = 1;
 	cred.Attributes = attrs;
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;

 	write_attr(attrs, L"git_protocol", protocol);
-	write_attr(attrs + 1, L"git_host", host);
+
+	if (host) {
+		write_attr(attrs + cred.AttributeCount, L"git_host", host);
+		cred.AttributeCount++;
+	}

 	if (path) {
-		write_attr(attrs + 2, L"git_path", path);
+		write_attr(attrs + cred.AttributeCount, L"git_path", path);
 		cred.AttributeCount++;
 	}

@@ -322,7 +326,7 @@ int main(int argc, char *argv[])

 	load_cred_funcs();

-	if (!protocol || !host)
+	if (!protocol || !(host || path))
 		return 0;

 	/* prepare 'target', the unique key for the credential */
@@ -333,7 +337,8 @@ int main(int argc, char *argv[])
 		strncat(target_buf, username, sizeof(target_buf));
 		strncat(target_buf, "@", sizeof(target_buf));
 	}
-	strncat(target_buf, host, sizeof(target_buf));
+	if (host)
+		strncat(target_buf, host, sizeof(target_buf));
 	if (path) {
 		strncat(target_buf, "/", sizeof(target_buf));
 		strncat(target_buf, path, sizeof(target_buf));

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

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, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en