git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Erik Faye-Lund <kusmabite@gmail.com>
Cc: git@vger.kernel.org, msysgit@googlegroups.com
Subject: Re: [PATCH v2] contrib: add win32 credential-helper
Date: Tue, 24 Apr 2012 16:40:52 -0400	[thread overview]
Message-ID: <20120424204052.GC21904@sigill.intra.peff.net> (raw)
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

  parent reply	other threads:[~2012-04-24 20:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-19 18:45 [PATCH v2] contrib: add win32 credential-helper Erik Faye-Lund
2012-04-22 18:07 ` Jeff King
2012-04-23 16:05   ` Erik Faye-Lund
2012-04-24 20:21     ` Jeff King
2012-04-24 20:40 ` Jeff King [this message]
2012-06-22 16:29   ` Erik Faye-Lund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120424204052.GC21904@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=kusmabite@gmail.com \
    --cc=msysgit@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).