On Tue, Apr 24, 2012 at 10:40 PM, Jeff King <peff@xxxxxxxx> 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));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html