Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

[Ted Zlatanov <tzz@lifelogs.com>]

On Tue, 05 Feb 2013 08:15:48 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> +# build reverse token map
>> +my %rmap;
>> +foreach my $k (keys %{$options{tmap}}) {
>> +	push @{$rmap{$options{tmap}->{$k}}}, $k;
>> +}

JCH> Mental note: "$rmap{foo} -eq 'bar'" means that what Git calls 'bar'
JCH> is found as 'foo' in the netrc/authinfo file.  Keys in %rmap are
JCH> what we expect to read from the netrc/authinfo file.

I'll document that better in PATCHv4.

JCH> So you grabbed one line of input, split them into token pairs, and
JCH> built %tokens = ('key Git may want to see' => 'value read from file')
JCH> mapping.

This will be fixed with PATCHv4 to do multiple lines (as defined by the
netrc manpage, etc.).

>> +	# for "host X port Y" where Y is an integer (captured by
>> +	# $num_port above), set the host to "X:Y"
>> +	$tokens{host} = join(':', $tokens{host}, $num_port)
>> +		if defined $tokens{host} && defined $num_port;

JCH> What happens when 'host' does not exist?  netrc/authinfo should be a
JCH> stream of SP/HT/LF delimited tokens and 'machine' token (or
JCH> 'default') begins a new entry, so it would mean the input file is
JCH> corrupt if we do not have $tokens{host} when we get here, I think.

Yes.  I'll make the host/machine token required, which will avoid this
issue.

JCH> Oh, another thing. 'default' is like 'machine' followed by any
JCH> machine name, so the above while loop that reads two tokens
JCH> pair-wise needs to be aware that 'default' is not followed by a
JCH> value.  I think the loop will fail to parse this:

JCH>         default       login anonymous    password me@home
JCH>         machine k.org login me           password mysecret

I'd prefer to ignore "default" because it should not be used for the Git
credential helpers (its only use case is for anonymous services AFAIK).
So I'll add a case to ignore it in PATCHv4, if that's OK.

JCH> Hmph, aren't you checking what you read a bit too early?  This is a
JCH> valid input:

JCH>         default       
JCH>                 login anonymous
JCH>                 password me@home
JCH>         machine k.org
JCH>                 login me
JCH>                 password mysecret

JCH> but does this loop gives mysecret back to me when asked for
JCH> host=k.org and user=me? 

To be fixed in PATCHv4, which will require the host/machine, use it as
the primary key, and only examine entries with it.

JCH> I would probably structure this part like this: [...]

I will do it like that, thank you for the suggestion.

I'll also add a simple testing Makefile for my own use, and you can
consider adding tests to the general framework later.

Ted