* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-05 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vpq0equo9.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 14:09:58 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> open $io, "-|", qw(gpg --decrypt), $ARGV[0]
OK, the below will be in PATCHv6 (I'll wait on mailing it until after
you've reviewed the rest of PATCHv5). Thanks for checking... I must
have had a typo or a missing comma or something, I could swear I tried
exactly what you show.
Ted
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
index 8298564..5f91630 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -230,13 +230,9 @@ sub load_netrc {
my $io;
if ($gpgmode) {
- # typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
- my $f = $file;
- $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
- # GPG doesn't work well with 2- or 3-argument open
- my $cmd = "gpg --decrypt $f";
- log_verbose("Using GPG to open $file: [$cmd]");
- open $io, "$cmd|";
+ my @cmd = (qw(gpg --decrypt), $file);
+ log_verbose("Using GPG to open $file: [@cmd]");
+ open $io, "-|", @cmd;
}
else {
log_verbose("Opening $file...");
^ permalink raw reply related
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Junio C Hamano @ 2013-02-05 23:10 UTC (permalink / raw)
To: Jeff King; +Cc: Michal Nazarewicz, git, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <20130130074306.GA17868@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote:
>
>> Either way it still encourages a plaintext password to be on disk,
>> which may not be what we want, even though it may be slight if not
>> really much of an improvement. Again the Help-for-users has this
>> amusing bit:
>
> I do not mind a .netrc or .authinfo parser, because while those formats
> do have security problems, they are standard files that may already be
> in use. So as long as we are not encouraging their use, I do not see a
> problem in supporting them (and we already do the same with curl's netrc
> support).
>
> But it would probably make sense for send-email to support the existing
> git-credential subsystem, so that it can take advantage of secure
> system-specific storage. And that is where we should be pointing new
> users. I think contrib/mw-to-git even has credential support written in
> perl, so it would just need to be factored out to Git.pm.
I see a lot of rerolls on the credential helper front, but is there
anybody working on hooking send-email to the credential framework?
^ permalink raw reply
* Re: Rebased git-subtree changes
From: Junio C Hamano @ 2013-02-05 23:18 UTC (permalink / raw)
To: David A. Greene; +Cc: git
In-Reply-To: <1360064219-28789-1-git-send-email-greened@obbligato.org>
This looks to be of mixed quality. The earlier ones look fairly
finished, while the later ones not so much.
I am tempted to take up to 06/13 and advance them to 'next', without
the rest.
^ permalink raw reply
* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-05 23:58 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <7vip66qu0u.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Otherwise, looks almost ready to me.
For now, I've queued this as a minimum fix-up on top of your patch
and pushed the result out. It is an equivalent of the previous
review comments in a patch form. Please review and incorporate in
your reroll as appropriate.
I haven't looked at the part that interacts with the credential
subsystem itself, though.
contrib/credential/netrc/git-credential-netrc | 35 ++++++++++++---------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
index 8298564..30e05fb 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -74,6 +74,10 @@ Options:
first matching entry found is returned via the credential
helper protocol (see below).
+ When no -f option is given, .authinfo.gpg, .netrc.gpg,
+ .authinfo, and .netrc files in your home directory are used
+ in this order.
+
-k|--insecure : ignore bad file ownership or permissions
-d|--debug : turn on debugging (developer info)
@@ -206,8 +210,7 @@ foreach my $file (@$files) {
unless (scalar @entries) {
if ($!) {
log_verbose("Unable to open $file: $!");
- }
- else {
+ } else {
log_verbose("No netrc entries found in $file");
}
@@ -230,15 +233,10 @@ sub load_netrc {
my $io;
if ($gpgmode) {
- # typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
- my $f = $file;
- $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
- # GPG doesn't work well with 2- or 3-argument open
- my $cmd = "gpg --decrypt $f";
- log_verbose("Using GPG to open $file: [$cmd]");
- open $io, "$cmd|";
- }
- else {
+ my @cmd = (qw(gpg --decrypt), $file)
+ log_verbose("Using GPG to open $file: [@cmd]");
+ open $io, "-|", @cmd;
+ } else {
log_verbose("Opening $file...");
open $io, '<', $file;
}
@@ -257,6 +255,9 @@ sub load_netrc {
my %entry;
my $num_port;
+ if (!defined $nentry->{machine}) {
+ next;
+ }
if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
$num_port = $nentry->{port};
delete $nentry->{port};
@@ -302,8 +303,7 @@ sub net_netrc_loader {
while (@tok) {
if ($tok[0] eq "default") {
shift(@tok);
- undef $mach; # ignore 'default' lines
-
+ $mach = { machine => undef }
next TOKEN;
}
@@ -313,8 +313,7 @@ sub net_netrc_loader {
my $host = shift @tok;
$mach = { machine => $host };
push @entries, $mach;
- }
- elsif (exists $options{tmap}->{$tok}) {
+ } elsif (exists $options{tmap}->{$tok}) {
unless ($mach) {
log_debug("Skipping token $tok because no machine was given");
next TOKEN;
@@ -329,8 +328,7 @@ sub net_netrc_loader {
# Following line added by rmerrell to remove '/' escape char in .netrc
$value =~ s/\/\\/\\/g;
$mach->{$tok} = $value;
- }
- elsif ($tok eq "macdef") { # we ignore macros
+ } elsif ($tok eq "macdef") { # we ignore macros
next TOKEN unless $mach;
my $value = shift @tok;
$macdef = 1;
@@ -380,8 +378,7 @@ sub find_netrc_entry {
unless ($query->{$check} eq $entry->{$check}) {
next ENTRY;
}
- }
- else {
+ } else {
log_debug("OK: any value satisfies check $check");
}
}
--
1.8.1.2.641.g0b90ac4
^ permalink raw reply related
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Jiang Xin @ 2013-02-06 0:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git List
In-Reply-To: <7vd2weu1sq.fsf@alter.siamese.dyndns.org>
2013/2/6 Junio C Hamano <gitster@pobox.com>:
> I somehow suspect that this is going in a direction that makes this
> piece of code much less maintainable.
>
> Look at the entire function and see how many places you do fprintf
> on strings that are marked with _(). short_name and long_name are
> not likely to be translated, but everything else is, especially
> multiple places that show _(opts->help) neither of these patches
> touch.
>
> I wonder if it makes more sense to add a helper function that
> returns the number of column positions (not bytes) with a signature
> similar to fprintf() and use that throughout the function instead.
I agree, a helper named 'utf8_fprintf' in utf8.c is better.
I will send a patch latter.
--
Jiang Xin
^ permalink raw reply
* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-06 0:34 UTC (permalink / raw)
To: git
In-Reply-To: <7vip66qu0u.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 14:24:01 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> + $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
JCH> Yuck. If you really have to quote, it is often far simpler to take
JCH> advantage of the fact that quoting rule for shell is much simpler
JCH> inside '', i.e.
JCH> sub sq {
JCH> my ($string) = @_;
JCH> $string =~ s|'|'\\''|g;
JCH> return "'$string'";
JCH> }
Oh, that's nice. Thanks. We don't need it anymore, but I'm sad to see
it go unused.
JCH> I think it is saner to do something like this instead here:
JCH> $mach = { machine => undef }
JCH> Otherwise your log_debug() will be filled by the tokens used for the
JCH> default entry, and also this "undef $mach" here will break your
JCH> macdef skipping logic if the default entry has a macdef, I think.
JCH> You can ignore an entry with undefined "machine" in the loop at the
JCH> end of load_netrc.
Cool, I merged your changes into PATCHv6. I'll keep in mind about
merging the trailing else braces, too. I forgot that setting for
cperl-mode (`cperl-merge-trailing-else . t').
Thanks
Ted
^ permalink raw reply
* Re: importing two different trees into a fresh git repo
From: Constantine A. Murenin @ 2013-02-06 0:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7va9riqtro.fsf@alter.siamese.dyndns.org>
On 5 February 2013 14:29, Junio C Hamano <gitster@pobox.com> wrote:
> "Constantine A. Murenin" <mureninc@gmail.com> writes:
>
>> I have two distinct trees that were not managed by any RCS, and I'd
>> like to import them into a single repository into two separate orphan
>> branches, then make sense of what's in there, merge, and unify into
>> 'master'.
>>
>> (To give some context, it's /etc/nginx config files from nginx/1.0.12
>> on Debian 6 and nginx/1.2.2 on OpenBSD 5.2.)
>
> As these come from two totally separate sources, I'd find it more
> natural to do two repositories, deb-nginx-conf and obsd-nginx-conf,
> each with one commit and then pull one into the other (or pull both
> to master-nginx-conf if you really wanted to), to me.
Yeah, I guess it might be more of a git-style to have two/three
separate repositories here. (The sources are just a couple of files,
so I think my specific example still calls for merely two orphan
branches.)
Still, is it really expected that you can't create an orphan branch in
an empty repository? On the outside, this sounds like a rather benign
bug.
C.
^ permalink raw reply
* [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-06 0:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vtxpqnwiv.fsf@alter.siamese.dyndns.org>
Add Git credential helper that can parse netrc/authinfo files.
This credential helper supports multiple files, returning the first one
that matches. It checks file permissions and owner. For *.gpg files,
it will run GPG to decrypt the file.
Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv5:
- reroll from tz/credential-authinfo:
* brace fixes
* list attempted default files in help doc
* 3-arg open() for the GPG pipe
* instead of skipping 'default' tokens, store them as machine => undef
contrib/credential/netrc/Makefile | 12 +
contrib/credential/netrc/git-credential-netrc | 421 +++++++++++++++++++++++++
2 files changed, 433 insertions(+), 0 deletions(-)
create mode 100644 contrib/credential/netrc/Makefile
create mode 100755 contrib/credential/netrc/git-credential-netrc
diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+ @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+ @echo "=> Silent invocation... nothing should show up here with a missing file"
+ @echo "bad data" | ./git-credential-netrc -f A get
+ @echo "=> Back to noisy: -v and -d used below, missing file"
+ echo "bad data" | ./git-credential-netrc -f A -d -v get
+ @echo "=> Look for any entry in the default file set"
+ echo "" | ./git-credential-netrc -d -v get
+ @echo "=> Look for github.com in the default file set"
+ echo "host=google.com" | ./git-credential-netrc -d -v get
+ @echo "=> Look for a nonexistent machine in the default file set"
+ echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..30e05fb
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,421 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+ help => 0,
+ debug => 0,
+ verbose => 0,
+ insecure => 0,
+ file => [],
+
+ # identical token maps, e.g. host -> host, will be inserted later
+ tmap => {
+ port => 'protocol',
+ machine => 'host',
+ path => 'path',
+ login => 'username',
+ user => 'username',
+ password => 'password',
+ }
+ );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+ $options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file. Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+ push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+ "help|h",
+ "debug|d",
+ "insecure|k",
+ "verbose|v",
+ "file|f=s@",
+ );
+
+if ($options{help}) {
+ my $shortname = basename($0);
+ $shortname =~ s/git-credential-//;
+
+ print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com. License: BSD.
+
+Options:
+
+ -f|--file AUTHFILE : specify netrc-style files. Files with the .gpg extension
+ will be decrypted by GPG before parsing. Multiple -f
+ arguments are OK. They are processed in order, and the
+ first matching entry found is returned via the credential
+ helper protocol (see below).
+
+ When no -f option is given, .authinfo.gpg, .netrc.gpg,
+ .authinfo, and .netrc files in your home directory are used
+ in this order.
+
+ -k|--insecure : ignore bad file ownership or permissions
+
+ -d|--debug : turn on debugging (developer info)
+
+ -v|--verbose : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+ git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+ git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+ git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper. It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+ The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+ The remote hostname for a network credential. (host=X)
+
+ 'path':
+ The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+ The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+host=github.com
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+machine github.com port https login tzz
+
+OR
+
+machine github.com protocol https login tzz
+
+OR... etc. acceptable tokens as listed above. Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol". Any redundant entry tokens (part of the original query) are
+skipped.
+
+Again, note that only the first matching entry from all the AUTHFILEs, processed
+in the sequence given on the command line, is used.
+
+Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+ exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files) {
+ my @candidates = qw[
+ ~/.authinfo.gpg
+ ~/.netrc.gpg
+ ~/.authinfo
+ ~/.netrc
+ ];
+
+ $files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files) {
+ my $gpgmode = $file =~ m/\.gpg$/;
+ unless (-r $file) {
+ log_verbose("Unable to read $file; skipping it");
+ next FILE;
+ }
+
+ # the following check is copied from Net::Netrc, for non-GPG files
+ # OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+ unless ($gpgmode || $options{insecure} ||
+ $^O eq 'os2'
+ || $^O eq 'MSWin32'
+ || $^O eq 'MacOS'
+ || $^O =~ /^cygwin/) {
+ my @stat = stat($file);
+
+ if (@stat) {
+ if ($stat[2] & 077) {
+ log_verbose("Insecure $file (mode=%04o); skipping it",
+ $stat[2] & 07777);
+ next FILE;
+ }
+
+ if ($stat[4] != $<) {
+ log_verbose("Not owner of $file; skipping it");
+ next FILE;
+ }
+ }
+ }
+
+ my @entries = load_netrc($file, $gpgmode);
+
+ unless (scalar @entries) {
+ if ($!) {
+ log_verbose("Unable to open $file: $!");
+ } else {
+ log_verbose("No netrc entries found in $file");
+ }
+
+ next FILE;
+ }
+
+ my $entry = find_netrc_entry($query, @entries);
+ if ($entry) {
+ print_credential_data($entry, $query);
+ # we're done!
+ last FILE;
+ }
+}
+
+exit 0;
+
+sub load_netrc {
+ my $file = shift @_;
+ my $gpgmode = shift @_;
+
+ my $io;
+ if ($gpgmode) {
+ my @cmd = (qw(gpg --decrypt), $file)
+ log_verbose("Using GPG to open $file: [@cmd]");
+ open $io, "-|", @cmd;
+ } else {
+ log_verbose("Opening $file...");
+ open $io, '<', $file;
+ }
+
+ # nothing to do if the open failed (we log the error later)
+ return unless $io;
+
+ # Net::Netrc does this, but the functionality is merged with the file
+ # detection logic, so we have to extract just the part we need
+ my @netrc_entries = net_netrc_loader($io);
+
+ # these entries will use the credential helper protocol token names
+ my @entries;
+
+ foreach my $nentry (@netrc_entries) {
+ my %entry;
+ my $num_port;
+
+ if (!defined $nentry->{machine}) {
+ next;
+ }
+ if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+ $num_port = $nentry->{port};
+ delete $nentry->{port};
+ }
+
+ # create the new entry for the credential helper protocol
+ $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+ # for "host X port Y" where Y is an integer (captured by
+ # $num_port above), set the host to "X:Y"
+ if (defined $entry{host} && defined $num_port) {
+ $entry{host} = join(':', $entry{host}, $num_port);
+ }
+
+ push @entries, \%entry;
+ }
+
+ return @entries;
+}
+
+sub net_netrc_loader {
+ my $fh = shift @_;
+ my @entries;
+ my ($mach, $macdef, $tok, @tok);
+
+ LINE:
+ while (<$fh>) {
+ undef $macdef if /\A\n\Z/;
+
+ if ($macdef) {
+ next LINE;
+ }
+
+ s/^\s*//;
+ chomp;
+
+ while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+ (my $tok = $+) =~ s/\\(.)/$1/g;
+ push(@tok, $tok);
+ }
+
+ TOKEN:
+ while (@tok) {
+ if ($tok[0] eq "default") {
+ shift(@tok);
+ $mach = { machine => undef }
+ next TOKEN;
+ }
+
+ $tok = shift(@tok);
+
+ if ($tok eq "machine") {
+ my $host = shift @tok;
+ $mach = { machine => $host };
+ push @entries, $mach;
+ } elsif (exists $options{tmap}->{$tok}) {
+ unless ($mach) {
+ log_debug("Skipping token $tok because no machine was given");
+ next TOKEN;
+ }
+
+ my $value = shift @tok;
+ unless (defined $value) {
+ log_debug("Token $tok had no value, skipping it.");
+ next TOKEN;
+ }
+
+ # Following line added by rmerrell to remove '/' escape char in .netrc
+ $value =~ s/\/\\/\\/g;
+ $mach->{$tok} = $value;
+ } elsif ($tok eq "macdef") { # we ignore macros
+ next TOKEN unless $mach;
+ my $value = shift @tok;
+ $macdef = 1;
+ }
+ }
+ }
+
+ return @entries;
+}
+
+sub read_credential_data_from_stdin {
+ # the query: start with every token with no value
+ my %q = map { $_ => undef } values(%{$options{tmap}});
+
+ while (<STDIN>) {
+ next unless m/^([^=]+)=(.+)/;
+
+ my ($token, $value) = ($1, $2);
+ die "Unknown search token $token" unless exists $q{$token};
+ $q{$token} = $value;
+ log_debug("We were given search token $token and value $value");
+ }
+
+ foreach (sort keys %q) {
+ log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+ }
+
+ return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+ my $query = shift @_;
+
+ ENTRY:
+ foreach my $entry (@_)
+ {
+ my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+ foreach my $check (sort keys %$query) {
+ if (defined $query->{$check}) {
+ log_debug("compare %s [%s] to [%s] (entry: %s)",
+ $check,
+ $entry->{$check},
+ $query->{$check},
+ $entry_text);
+ unless ($query->{$check} eq $entry->{$check}) {
+ next ENTRY;
+ }
+ } else {
+ log_debug("OK: any value satisfies check $check");
+ }
+ }
+
+ return $entry;
+ }
+
+ # nothing was found
+ return;
+}
+
+sub print_credential_data {
+ my $entry = shift @_;
+ my $query = shift @_;
+
+ log_debug("entry has passed all the search checks");
+ TOKEN:
+ foreach my $git_token (sort keys %$entry) {
+ log_debug("looking for useful token $git_token");
+ # don't print unknown (to the credential helper protocol) tokens
+ next TOKEN unless exists $query->{$git_token};
+
+ # don't print things asked in the query (the entry matches them)
+ next TOKEN if defined $query->{$git_token};
+
+ log_debug("FOUND: $git_token=$entry->{$git_token}");
+ printf "%s=%s\n", $git_token, $entry->{$git_token};
+ }
+}
+sub log_verbose {
+ return unless $options{verbose};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
+
+sub log_debug {
+ return unless $options{debug};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
--
1.7.9.rc2
^ permalink raw reply related
* [PATCH v3] Add utf8_fprintf helper which returns correct columns
From: Jiang Xin @ 2013-02-06 1:16 UTC (permalink / raw)
To: Junio C Hamano, Nguyễn Thái Ngọc Duy; +Cc: Git List, Jiang Xin
In-Reply-To: <7vd2weu1sq.fsf@alter.siamese.dyndns.org>
Since command usages can be translated, they may not align well especially
when they are translated to CJK. A wrapper utf8_fprintf can help to return
the correct columns required.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 5 +++--
utf8.c | 20 ++++++++++++++++++++
utf8.h | 1 +
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98..a6ce9e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
+#include "utf8.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
s = literal ? "[%s]" : "[<%s>]";
else
s = literal ? " %s" : " <%s>";
- return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
+ return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
}
#define USAGE_OPTS_WIDTH 24
@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
if (opts->long_name)
pos += fprintf(outfile, "--%s", opts->long_name);
if (opts->type == OPTION_NUMBER)
- pos += fprintf(outfile, "-NUM");
+ pos += utf8_fprintf(outfile, _("-NUM"));
if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
!(opts->flags & PARSE_OPT_NOARG))
diff --git a/utf8.c b/utf8.c
index a4ee6..52dbd 100644
--- a/utf8.c
+++ b/utf8.c
@@ -430,6 +430,26 @@ int same_encoding(const char *src, const char *dst)
}
/*
+ * Wrapper for fprintf and returns the total number of columns required
+ * for the printed string, assuming that the string is utf8.
+ */
+int utf8_fprintf(FILE *stream, const char *format, ...)
+{
+ struct strbuf buf = STRBUF_INIT;
+ va_list arg;
+ int columns;
+
+ va_start (arg, format);
+ strbuf_vaddf(&buf, format, arg);
+ va_end (arg);
+
+ fputs(buf.buf, stream);
+ columns = utf8_strwidth(buf.buf);
+ strbuf_release(&buf);
+ return columns;
+}
+
+/*
* Given a buffer and its encoding, return it re-encoded
* with iconv. If the conversion fails, returns NULL.
*/
diff --git a/utf8.h b/utf8.h
index a2142..501b2 100644
--- a/utf8.h
+++ b/utf8.h
@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);
int is_utf8(const char *text);
int is_encoding_utf8(const char *name);
int same_encoding(const char *, const char *);
+int utf8_fprintf(FILE *, const char *, ...);
void strbuf_add_wrapped_text(struct strbuf *buf,
const char *text, int indent, int indent2, int width);
--
1.8.1.1.370.g47b6ee8.dirty
^ permalink raw reply
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Junio C Hamano @ 2013-02-06 2:44 UTC (permalink / raw)
To: Jiang Xin; +Cc: Duy Nguyen, Git List
In-Reply-To: <CANYiYbF1cS=K9M0cwE5V0pUJMPEYGiJOjJwg5KQScCf8pjyTqw@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> 2013/2/6 Junio C Hamano <gitster@pobox.com>:
>> I somehow suspect that this is going in a direction that makes this
>> piece of code much less maintainable.
>>
>> Look at the entire function and see how many places you do fprintf
>> on strings that are marked with _(). short_name and long_name are
>> not likely to be translated, but everything else is, especially
>> multiple places that show _(opts->help) neither of these patches
>> touch.
>>
>> I wonder if it makes more sense to add a helper function that
>> returns the number of column positions (not bytes) with a signature
>> similar to fprintf() and use that throughout the function instead.
>
> I agree, a helper named 'utf8_fprintf' in utf8.c is better.
> I will send a patch latter.
Yeah, the idea of a helper function I agree with; I am not thrilled
with the name utf8_fprintf() though. People use the return value of
fprintf() for error detection (negative return value means an error)
most of the time (even though non-negative value gives the number of
bytes shown), but the primary use of the return value from the
utf8_fprintf() function will be to get the display width, and the
name does not quite capture that.
^ permalink raw reply
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Jiang Xin @ 2013-02-06 3:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git List
In-Reply-To: <7vpq0enoui.fsf@alter.siamese.dyndns.org>
2013/2/6 Junio C Hamano <gitster@pobox.com>:
> Jiang Xin <worldhello.net@gmail.com> writes:
>> I agree, a helper named 'utf8_fprintf' in utf8.c is better.
>> I will send a patch latter.
>
> Yeah, the idea of a helper function I agree with; I am not thrilled
> with the name utf8_fprintf() though. People use the return value of
> fprintf() for error detection (negative return value means an error)
> most of the time (even though non-negative value gives the number of
> bytes shown), but the primary use of the return value from the
> utf8_fprintf() function will be to get the display width, and the
> name does not quite capture that.
>
How about this since [PATCH v3]:
diff --git a/utf8.c b/utf8.c
index 52dbd..b893a 100644
--- a/utf8.c
+++ b/utf8.c
@@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
strbuf_vaddf(&buf, format, arg);
va_end (arg);
- fputs(buf.buf, stream);
- columns = utf8_strwidth(buf.buf);
+ columns = fputs(buf.buf, stream);
+ /* If no error occurs, and really write something (columns > 0),
+ * calculate really columns width with utf8_strwidth. */
+ if (columns > 0)
+ columns = utf8_strwidth(buf.buf);
strbuf_release(&buf);
return columns;
}
--
蒋鑫
北京群英汇信息技术有限公司
邮件: worldhello.net@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 010-51262007, 18601196889
^ permalink raw reply
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Junio C Hamano @ 2013-02-06 4:35 UTC (permalink / raw)
To: Jiang Xin; +Cc: Duy Nguyen, Git List
In-Reply-To: <CANYiYbF8DCPxqGQ2AFFXpSm0nO+wFDg=qrn9C8uoZO6fj__NHA@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> 2013/2/6 Junio C Hamano <gitster@pobox.com>:
>> Jiang Xin <worldhello.net@gmail.com> writes:
>>> I agree, a helper named 'utf8_fprintf' in utf8.c is better.
>>> I will send a patch latter.
>>
>> Yeah, the idea of a helper function I agree with; I am not thrilled
>> with the name utf8_fprintf() though. People use the return value of
>> fprintf() for error detection (negative return value means an error)
>> most of the time (even though non-negative value gives the number of
>> bytes shown), but the primary use of the return value from the
>> utf8_fprintf() function will be to get the display width, and the
>> name does not quite capture that.
>>
>
> How about this since [PATCH v3]:
>
> diff --git a/utf8.c b/utf8.c
> index 52dbd..b893a 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
> strbuf_vaddf(&buf, format, arg);
> va_end (arg);
>
> - fputs(buf.buf, stream);
> - columns = utf8_strwidth(buf.buf);
> + columns = fputs(buf.buf, stream);
> + /* If no error occurs, and really write something (columns > 0),
> + * calculate really columns width with utf8_strwidth. */
> + if (columns > 0)
> + columns = utf8_strwidth(buf.buf);
> strbuf_release(&buf);
> return columns;
> }
Yeah, the error checking is necessary; it would make your intention
more clear to say:
if (0 <= columns)
columns = utf8_strwidth(buf.buf);
though, as buf.buf _may_ be an empty string, and with the "if"
statement you are saying "we return the width only when output did
not result in an error".
The above bugfix does not address my original concern about
the name, though.
^ permalink raw reply
* Adding Missing Tags to a Repository
From: Neil @ 2013-02-06 4:45 UTC (permalink / raw)
To: git
Hi everyone,
A while back I did a svn-to-git migration for my team. Our subversion
repository had about 30K+ commits, 100+ branches, 2K+ tags, all made
over a 20+ year period. I was doing the migration using git-svn, and
my big problem was the tags. git-svn seemed to want to traverse the
entire history of each tag, which was taking a long time. Because time
and resources were limited, I ended up just migrating the branches and
trunk, with the idea that I would handle the tags at a later date. My
original plan to do that was to crawl the subversion log, find where
the tags were made, and apply a git tag to the commit that was the
source of the tag. This was a bad idea.
I've found that over the years, people have made tags that are only
subdirectories of the source tree, made tags off of other tags, and
committed to tags. The latter is the biggest problem, since those
commits don't seem to be stored in the git repository because they
never appeared in the branches/trunk.
So, I'm wondering what my options are to bring back this history. One
idea is to somehow resume the git-svn download, but changing it to
also scan tags (it sounds like it should be possible, but I haven't
tried it yet). Or maybe there's some other tool that will more quickly
clone the repository including tags, and then I can somehow splice the
tags back into the repository we're already using?
Any ideas or suggestions?
Thanks!
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-06 8:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Michal Nazarewicz, git, Krzysztof Mazur,
Michal Nazarewicz
In-Reply-To: <7v6226pdb7.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I see a lot of rerolls on the credential helper front, but is there
> anybody working on hooking send-email to the credential framework?
Not answering the question, but git-remote-mediawiki supports the
credential framework. It is written in perl, and the credential support
is rather cleanly written and doesn't have dependencies on the wiki
part, so the way to go for send-email is probably to libify the
credential support in git-remote-mediawiki, and to use it in send-email.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: importing two different trees into a fresh git repo
From: Jeff King @ 2013-02-06 9:07 UTC (permalink / raw)
To: Constantine A. Murenin; +Cc: git
In-Reply-To: <CAPKkNb6+ojb+uvBW+AkhGrhjR85LrJEbmR0KmvaKYb2Cj5Aa4g@mail.gmail.com>
On Tue, Feb 05, 2013 at 01:46:09PM -0800, Constantine A. Murenin wrote:
> I've encountered two problems so far:
>
> 0. After initialising the repository, I was unable to `git checkout
> --orphan Debian-6.0.4-nginx-1.0.12` -- presumably it doesn't work when
> the repo is empty? This sounds like a bug or an artefact of
> implementation. I presume this can be worked around by committing
> into master instead, and then doing `git checkout -b
> Debian-6.0.4-nginx-1.0.12`, and then force-fixing the master somehow
> later on.
What version of git are you using? Using both "-b" and "--orphan" from a
non-existing branch used to be broken, but was fixed by abe1998 (git
checkout -b: allow switching out of an unborn branch, 2012-01-30), which
first appeared in git v1.7.9.2.
> 1. After making a mistake on my first commit (my first commit into
> OpenBSD-5.2-nginx-1.2.2 orphan branch ended up including a directory
> from master by mistake), I am now unable to rebase and "fixup" the
> changes -- `git rebase --interactive HEAD~2` doesn't work, which, from
> one perspective, makes perfect sense (indeed there's no prior
> revision), but, from another, it's not immediately obvious how to
> quickly work around it.
You cannot ask to rebase onto HEAD~2 because it does not exist (I'm assuming
from your description that HEAD~1 is the root of your repository). But
you can use the "--root" flag to ask git to rebase all the way down to
the roots, like:
git rebase -i --root
However, note that older versions of git do not support using "--root"
with "-i". The first usable version is v1.7.12.
-Peff
^ permalink raw reply
* Re: git-svn problems with white-space in tag names
From: Erik Faye-Lund @ 2013-02-06 9:26 UTC (permalink / raw)
To: Hans-Juergen Euler; +Cc: git
In-Reply-To: <CAK3CF+46a2LsrZ0ef5Bc2XfwCVv1LVk6YbMasa6e5kx-Dp3JTg@mail.gmail.com>
On Sun, Jan 27, 2013 at 3:12 PM, Hans-Juergen Euler <waas.nett@gmail.com> wrote:
> This seems to be a problem of the windows version. At least with its
> complete severity. Installed git on Ubuntu in a virtual machine was
> able to clone the subversion repos past the tag with the white-space
> at the end. I am not sure but apparently this tag has not been
> converted.
>
> The git repos I could copy from Ubuntu to windows. So far no problems
> seen in this copy on windows.
>
The cause of the problem is that the tagname isn't a valid filename on
Windows, which git requires it to be.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Erik Faye-Lund @ 2013-02-06 9:34 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: ramsay, git, j6t
In-Reply-To: <201301271557.08994.tboegi@web.de>
On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> t0070 and t1301 fail when running the test suite under cygwin.
> Skip the failing tests by unsetting POSIXPERM.
>
But is this the real reason? I thought Cygwin implemented POSIX permissions...?
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Michael Haggerty @ 2013-02-06 10:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce
In-Reply-To: <7v8v72u0vw.fsf@alter.siamese.dyndns.org>
On 02/05/2013 06:27 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> I would again like to express my discomfort about this feature, which is
>> already listed as "will merge to next".
>
> Do not take "will merge to next" too literally. One major purpose
> of marking a topic as such is exactly to solicit comments like this
> ;-)
I take "will merge to next" pretty seriously, because I know how hard it
is to get *my* patch series to this state :-)
>> * I didn't see a response to Peff's convincing arguments that this
>> should be a client-side feature rather than a server-side feature [1].
>
> Uncluttering is not about a choice client should make. "delayed
> advertisement" is an orthogonal issue and requires a larger protocol
> update (it needs to make "git fetch" speak first instead of the
> current protocol in which "upload-pack" speaks first).
There seem to be a few issues mixed up in this topic. It is hard to
reason about your patch series without understanding which scenarios and
problems it is meant to address. First the problems that we might like
to solve:
Clutter: The typical user is subjected to much unneeded clutter in the
form of references that he/she will likely never use.
Bandwidth: Interactions with the remote repo (clone, fetch, etc) are
slowed down by the large volume of unnecessary data.
Provenance: Users mistakenly think that content originates with the
repository owner whereas it in fact came from some other
(perhaps untrusted) source.
Now, what are some use-case scenarios in which these problems arise? As
I understand it, there are a few:
Scenario 1: Some providers junk up their users' repositories with
content that is not created by the repository's owner and that the owner
doesn't want to appear to vouch for (e.g., GitHub pull requests). These
references might sometimes be useful to fetch, singly or in bulk.
Scenario 2: Some systems junk up their users' repositories with
additional references that are not interesting to most pullers (e.g.,
Gerrit activity markers) though they don't add questionable content.
Scenario 3: Some repository owners might *themselves* want to push
references to their repository but hide them from most users (e.g.,
Junio's topic branches) or make them completely hidden from the rest of
the world (e.g., proprietary vs. open-source branches).
In most of these cases, it would be desirable for at least some users to
be able to fetch and/or push hidden content.
A first weakness of your proposal is that even though the hidden refs
are (optionally) fetchable, there is *no* way to discover them remotely
or to bulk-download them; they would have to be retrieved one by one
using out-of-band information. And if I understand correctly, there
would be no way to push hidden references remotely (whether manually or
from some automated process). Such processes would have to be local to
the machine holding the repository.
A second weakness of your proposal is that the repository owner would
*anyway* need local access to the repo server or the help of the
provider to implement reference hiding (since hidden references cannot
be configured remotely). Who will choose what references to hide? Most
likely each provider will pick a one-size-fits-all configuration and
apply it to all of the repos that they manage. All users would be at
the mercy of their provider to make wise choices and would not be able
to override the choice via their client.
A third weakness of your hidden references proposal is that it is
schizophrenic: some references are hidden and undiscoverable, but their
content can nevertheless be made fetchable if the user happens to know
the SHA1. This is more complicated to understand and reason about than
the rule "exactly the content that is referred to by published
references is fetchable".
What would be a better way? Providers could expose multiple views of
the same repository; for example, one view with just the uncluttered
content, and a second view that includes *all* fetchable references.
Accessing the repository via the first view would give all of the
benefits provided by your hidden reference proposal. Accessing it via
the second view would allow the hidden references to be fetched (even in
bulk) using purely git tools. The documentation for the second view
could explain that it contains un-vetted content.
But your proposal does not admit two-tiered access to a single
repository. You only support one hidden reference configuration that is
applied to all remote access [1]. See below for more ideas about
implementing multiple views.
>> * I didn't see a response to my worries that this feature could be
>> abused [3].
>
> You can choose not to advertise allow-tip-sha1-in-want capability; I
> do not think it is making things worse than the status quo.
Yes, if the feature is turned off then it is not worse than the status
quo. But what if the feature is turned on?
Actually, I'm still not clear about how these hidden references are
supposed to be created. I know that you would forbid updating or
deleting hidden references via the remote protocol, but would you allow
them to be created? If so, then it seems that any pusher can create
dark content. Or can they only be created via a separate, local channel
to the repository? In this case, it seems rather limiting that any
process that wants to create hidden references has to be local.
>> * Why should a repository have exactly one setting for what refs should
>> be hidden? Wouldn't it make more sense to allow multiple "views" to be
>> defined?:
>
> You are welcome to extend to have different views, but how would
> your clients express which view they would want?
There are several possibilities:
1. Assuming the cooperation of the provider, the provider could offer
two separate URLs: one for the uncluttered view and one for the
cluttered view. The client would choose the view by choosing which URL
to clone from. On the provider side, both of these URLs could refer to
the same Git repository but, for example, set an environment variable
GIT_VIEW differently depending on which URL was used. This approach
would solve clutter, bandwidth and provenance but require cooperation
from the provider.
2a. Assuming no cooperation from the provider, the git client could have
options like "git fetch --view=uncluttered URL". This would receive all
references from the server but discard any that are not included in the
client's "uncluttered" view definition. This would solve clutter.
2b. Again assuming no cooperation from the provider, the user could
clone all references from the remote repo, but define a local
"uncluttered" view that hides the extra references on the local side.
The view could be selected by setting a local environment variable
GIT_VIEW or via configuration option "git config view.default
uncluttered". This would solve clutter in a more flexible way because
the clutter would still be available locally for those occasions when
the user wants to see it.
Please note that none of the above options require a new remote protocol.
If/when a new protocol is implemented, then the client could tell the
server what view it wants and the server would only advertise those refs
to the client:
3a. The client could tell the host what reference namespaces it wants to
fetch. Its choice would only be used for the single transaction and
would not be recorded on the server side.
3b. The client could pick a server-defined view by name. The server
would look up the name in its own configuration to translate it into a
subset of references. The views that a particular server supports would
be documented in the same place that the URL is documented and might
also be queryable by the client. There should probably be some standard
views like "default" and "full" that every server would be expected to
implement. Please note that this method can fall back to 2a when
communicating with a server that does not support the new protocol.
>> * Why should this feature only be available remotely?
>
> The whole point is to give the server side a choice to show selected
> refs, so that it can use hidden portion for its own use. These refs
> should not be hidden from local operations like "gc".
Certainly they shouldn't be hidden from "gc", but it would be useful to
be able to hide references from user-facing commands like "log --all",
"log --decorate", "gitk", "grep --all" etc. For example, here are some
more scenarios where clutter is annoying:
Scenario 4: I occasionally share with colleague Foo, so I want to
configure his repo as a remote for mine and fetch his latest work:
git remote add foo $URL
git fetch foo
But now every time I do a "gitk --all" or "git log --decorate", the
output is cluttered with all of his references (most of which are just
old versions of references from the upstream repository that we both
use). I would like to be able to hide his references most of the time
but turn them back on when I need them.
Scenario 5: Our upstream repository has gazillions of release tags under
"refs/tags/releases/...", sometimes including customer-specific
releases. In my daily life these are just clutter. (This scenario is
made worse by the fact that AFAIK there is no way to tell Git to fetch
some tags but not others others.) But sometimes I need to track down a
bug in a particular release and need to access that release tag. So it
would be nice to be able to hide and unhide them locally.
> I appreciate the comments, but I do not think any point you raised
> in this message is very much relevant as objections.
Tl;dr summary:
* Hidden refs don't give a way to offer two-tiered remote access to a
repository (e.g., one uncluttered view and one full view), so
* local access to the repository would (apparently) be required to
put *anything* in the hidden namespaces.
* they don't help in any scenario where you *sometimes*
want to bulk fetch the hidden refs, and even make it awkward to
fetch single hidden refs.
* Hidden refs introduce a confusing schizophrenia between "advertised"
and "not advertised but nonetheless fetchable".
* Hidden refs require the cooperation of the provider to configure and
will therefore be unusable by many repository owners.
* Some small improvements (e.g. allowing *multiple* views to be
defined) would provide much more benefit for about the same effort,
and would be a better base for building other features in the future
(e.g., local views).
Thanks for listening.
Michael
[1] Theoretically one could support multiple views of a single
repository by using something like "GIT_CONFIG=view_1_config git
upload-pack ..." or "git -c transfer.hiderefs=... git upload-pack ...",
but this would be awkward.
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Duy Nguyen @ 2013-02-06 10:34 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jonathan Nieder, Junio C Hamano, git, Jeff King, Shawn Pearce
In-Reply-To: <5110DF1D.8010505@alum.mit.edu>
On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> Hiderefs creates a "dark" corner of a remote git repo that can hold
> arbitrary content that is impossible for anybody to discover but
> nevertheless possible for anybody to download (if they know the name of
> a hidden reference). In earlier versions of the patch series I believe
> that it was possible to push to a hidden reference hierarchy, which made
> it possible to upload dark content. The new version appears (from the
> code) to prohibit adding references in a hidden hierarchy, which would
> close the main loophole that I was worried about. But the documentation
> and the unit tests only explicitly say that updates and deletes are
> prohibited; nothing is said about adding references (unless "update" is
> understood to include "add"). I think the true behavior should be
> clarified and tested.
>
> I was worried that somehow this "dark" content could be used for
> malicious purposes; for example, pushing compromised code then
> convincing somebody to download it by SHA1 with the implicit argument
> "it's safe since it comes directly from the project's official
> repository". If it is indeed impossible to populate the dark namespace
> remotely then I can't think of a way to exploit it.
Or you can think hiderefs is the first step to addressing the initial
ref advertisment problem. The series says hidden refs are to be
fetched out of band, but that's not the only way. A new extension can
be added to the protocol later to let the client explore this dark
space. It's only truly dark for old clients. We could even shed some
light to old clients by sending a dummy ref with some loud name like
PLEASE_UPDATE_TO_LATEST_GIT_TO_FETCH_REMAINING_REFS (new clients
silently drop this ref)
--
Duy
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Michael Schubert @ 2013-02-06 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, git, Jeff King
In-Reply-To: <7v38xhf1i3.fsf@alter.siamese.dyndns.org>
On 01/31/2013 11:09 PM, Junio C Hamano wrote:
>
> -static int http_request_reauth(const char *url, void *result, int target,
> +static int http_request_reauth(const char *url,
> + struct strbuf *type,
> + void *result, int target,
> int options)
> {
> - int ret = http_request(url, result, target, options);
> + int ret = http_request(url, type, result, target, options);
> if (ret != HTTP_REAUTH)
> return ret;
> - return http_request(url, result, target, options);
> + return http_request(url, type, result, target, options);
> }
This needs something like
diff --git a/http.c b/http.c
index d868d8b..da43be3 100644
--- a/http.c
+++ b/http.c
@@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
+ if (type)
+ strbuf_reset(type);
return http_request(url, type, result, target, options);
}
on top. Otherwise we get
"text/plainapplication/x-git-receive-pack-advertisement"
when doing HTTP auth.
Thanks.
> -int http_get_strbuf(const char *url, struct strbuf *result, int options)
> +int http_get_strbuf(const char *url,
> + struct strbuf *type,
> + struct strbuf *result, int options)
> {
> - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
> + return http_request_reauth(url, type, result,
> + HTTP_REQUEST_STRBUF, options);
> }
>
> /*
> @@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char *filename, int options)
> goto cleanup;
> }
>
> - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
> + ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, options);
> fclose(result);
>
> if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
> @@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
> int ret = -1;
>
> url = quote_ref_url(base, ref->name);
> - if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> + if (http_get_strbuf(url, NULL, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> strbuf_rtrim(&buffer);
> if (buffer.len == 40)
> ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> @@ -997,7 +1010,7 @@ int http_get_info_packs(const char *base_url, struct packed_git **packs_head)
> strbuf_addstr(&buf, "objects/info/packs");
> url = strbuf_detach(&buf, NULL);
>
> - ret = http_get_strbuf(url, &buf, HTTP_NO_CACHE);
> + ret = http_get_strbuf(url, NULL, &buf, HTTP_NO_CACHE);
> if (ret != HTTP_OK)
> goto cleanup;
>
> diff --git a/http.h b/http.h
> index 0a80d30..25d1931 100644
> --- a/http.h
> +++ b/http.h
> @@ -132,7 +132,7 @@ extern char *get_remote_object_url(const char *url, const char *hex,
> *
> * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
> */
> -int http_get_strbuf(const char *url, struct strbuf *result, int options);
> +int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options);
>
> /*
> * Prints an error message using error() containing url and curl_errorstr,
> diff --git a/remote-curl.c b/remote-curl.c
> index 9a8b123..e6f3b63 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -92,6 +92,8 @@ static void free_discovery(struct discovery *d)
>
> static struct discovery* discover_refs(const char *service)
> {
> + struct strbuf exp = STRBUF_INIT;
> + struct strbuf type = STRBUF_INIT;
> struct strbuf buffer = STRBUF_INIT;
> struct discovery *last = last_discovery;
> char *refs_url;
> @@ -113,7 +115,7 @@ static struct discovery* discover_refs(const char *service)
> }
> refs_url = strbuf_detach(&buffer, NULL);
>
> - http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
> + http_ret = http_get_strbuf(refs_url, &type, &buffer, HTTP_NO_CACHE);
> switch (http_ret) {
> case HTTP_OK:
> break;
> @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char *service)
> last->buf = last->buf_alloc;
>
> if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> - /* smart HTTP response; validate that the service
> + /*
> + * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - struct strbuf exp = STRBUF_INIT;
> -
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (strbuf_cmp(&exp, &type))
> + die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
> strbuf_setlen(&buffer, buffer.len - 1);
>
> + strbuf_reset(&exp);
> strbuf_addf(&exp, "# service=%s", service);
> if (strbuf_cmp(&exp, &buffer))
> die("invalid server response; got '%s'", buffer.buf);
> @@ -160,6 +165,8 @@ static struct discovery* discover_refs(const char *service)
> }
>
> free(refs_url);
> + strbuf_release(&exp);
> + strbuf_release(&type);
> strbuf_release(&buffer);
> last_discovery = last;
> return last;
^ permalink raw reply related
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Jeff King @ 2013-02-06 10:39 UTC (permalink / raw)
To: Michael Schubert; +Cc: Junio C Hamano, Shawn Pearce, git
In-Reply-To: <51122F69.9060704@elegosoft.com>
On Wed, Feb 06, 2013 at 11:24:41AM +0100, Michael Schubert wrote:
> On 01/31/2013 11:09 PM, Junio C Hamano wrote:
>
> >
> > -static int http_request_reauth(const char *url, void *result, int target,
> > +static int http_request_reauth(const char *url,
> > + struct strbuf *type,
> > + void *result, int target,
> > int options)
> > {
> > - int ret = http_request(url, result, target, options);
> > + int ret = http_request(url, type, result, target, options);
> > if (ret != HTTP_REAUTH)
> > return ret;
> > - return http_request(url, result, target, options);
> > + return http_request(url, type, result, target, options);
> > }
>
> This needs something like
>
> diff --git a/http.c b/http.c
> index d868d8b..da43be3 100644
> --- a/http.c
> +++ b/http.c
> @@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
> int ret = http_request(url, type, result, target, options);
> if (ret != HTTP_REAUTH)
> return ret;
> + if (type)
> + strbuf_reset(type);
> return http_request(url, type, result, target, options);
> }
>
> on top. Otherwise we get
>
> "text/plainapplication/x-git-receive-pack-advertisement"
>
> when doing HTTP auth.
Good catch. It probably makes sense to put it in http_request, so that
we also protect against any existing cruft from the callers of
http_get_*, like:
-- >8 --
Subject: [PATCH] http_request: reset "type" strbuf before adding
Callers may pass us a strbuf which we use to record the
content-type of the response. However, we simply appended to
it rather than overwriting its contents, meaning that cruft
in the strbuf gave us a bogus type. E.g., the multiple
requests triggered by http_request could yield a type like
"text/plainapplication/x-git-receive-pack-advertisement".
Reported-by: Michael Schubert <mschub@elegosoft.com>
Signed-off-by: Jeff King <peff@peff.net>
---
Is it worth having a strbuf_set* family of functions to match the
strbuf_add*? We seem to have these sorts of errors with strbuf from time
to time, and I wonder if that would make it easier (and more readable)
to do the right thing.
http.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/http.c b/http.c
index d868d8b..d9d1aad 100644
--- a/http.c
+++ b/http.c
@@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf *type,
if (type) {
char *t;
+ strbuf_reset(type);
curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t);
if (t)
strbuf_addstr(type, t);
--
1.8.1.2.11.g1a2f572
^ permalink raw reply related
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Duy Nguyen @ 2013-02-06 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jiang Xin, Git List
In-Reply-To: <7vip66njpj.fsf@alter.siamese.dyndns.org>
On Wed, Feb 6, 2013 at 11:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> How about this since [PATCH v3]:
>>
>> diff --git a/utf8.c b/utf8.c
>> index 52dbd..b893a 100644
>> --- a/utf8.c
>> +++ b/utf8.c
>> @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
>> strbuf_vaddf(&buf, format, arg);
>> va_end (arg);
>>
>> - fputs(buf.buf, stream);
>> - columns = utf8_strwidth(buf.buf);
>> + columns = fputs(buf.buf, stream);
>> + /* If no error occurs, and really write something (columns > 0),
>> + * calculate really columns width with utf8_strwidth. */
>> + if (columns > 0)
>> + columns = utf8_strwidth(buf.buf);
>> strbuf_release(&buf);
>> return columns;
>> }
>
> The above bugfix does not address my original concern about
> the name, though.
How about utf8_fwprintf? wprintf() deals with wide characters and
returns the number of wide characters, I think the name fits. And we
could just drop utf8_ and use the existing name, because we don't use
wchar_t* anyway.
--
Duy
^ permalink raw reply
* Re: Adding Missing Tags to a Repository
From: Michael J Gruber @ 2013-02-06 11:18 UTC (permalink / raw)
To: Neil; +Cc: git
In-Reply-To: <CANC5J9F5Pnp08KTem-fdcs_4DcmoN+OgqCHR0=r0y--U8=fdog@mail.gmail.com>
Neil venit, vidit, dixit 06.02.2013 05:45:
> Hi everyone,
>
> A while back I did a svn-to-git migration for my team. Our subversion
> repository had about 30K+ commits, 100+ branches, 2K+ tags, all made
> over a 20+ year period. I was doing the migration using git-svn, and
> my big problem was the tags. git-svn seemed to want to traverse the
> entire history of each tag, which was taking a long time. Because time
> and resources were limited, I ended up just migrating the branches and
> trunk, with the idea that I would handle the tags at a later date. My
> original plan to do that was to crawl the subversion log, find where
> the tags were made, and apply a git tag to the commit that was the
> source of the tag. This was a bad idea.
>
> I've found that over the years, people have made tags that are only
> subdirectories of the source tree, made tags off of other tags, and
> committed to tags. The latter is the biggest problem, since those
> commits don't seem to be stored in the git repository because they
> never appeared in the branches/trunk.
I think it's fair to say that svn encourages the abuse of tags ;)
I've cleaned up other people's mis-tags myself, and it's no fun. If the
tree layout is messed up then git-svn will often create a new root
commit, so that you have to stitch the history yourself.
In the case of "correct" tags, you're often better of converting the tag
creating commit into an annotated tag, provided that the tree is unchanged.
Tags with tree changes are really branches, usually maintenance branches
after a proper tag.
> So, I'm wondering what my options are to bring back this history. One
> idea is to somehow resume the git-svn download, but changing it to
> also scan tags (it sounds like it should be possible, but I haven't
> tried it yet). Or maybe there's some other tool that will more quickly
> clone the repository including tags, and then I can somehow splice the
> tags back into the repository we're already using?
>
> Any ideas or suggestions?
>
> Thanks!
>
If you add a git-svn refspec to your config and rerun git-svn fetch it
might rescan tags, although you may have to poke around to make it
rescan from revision 1.
Michael
^ permalink raw reply
* Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies
From: Jeff King @ 2013-02-06 11:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <7vwqumvk76.fsf@alter.siamese.dyndns.org>
On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:
> > In the earlier review, I mentioned making this per-service, but I see
> > that is not the case here. Do you have an argument against doing so?
>
> Perhaps then I misunderstood your intention. By reminding me of the
> receive-pack side, I thought you were hinting to unify these two
> into one, which I did. There is no argument against it.
What I meant was that there should be transfer.hiderefs, and an
individual {receive,uploadpack}.hiderefs, similar to the way we have
transfer.unpacklimit. That makes the easy case (hiding the refs
completely) easy, but leaves the flexibility for more.
Like this:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a8248d9..131c163 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -59,7 +59,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
static int receive_pack_config(const char *var, const char *value, void *cb)
{
- int status = parse_hide_refs_config(var, value, cb);
+ int status = parse_hide_refs_config(var, value, "receive");
if (status)
return status;
diff --git a/refs.c b/refs.c
index e3574ca..9bfea58 100644
--- a/refs.c
+++ b/refs.c
@@ -2560,9 +2560,13 @@ int parse_hide_refs_config(const char *var, const char *value, void *unused)
static struct string_list *hide_refs;
-int parse_hide_refs_config(const char *var, const char *value, void *unused)
+int parse_hide_refs_config(const char *var, const char *value, void *vsection)
{
- if (!strcmp("transfer.hiderefs", var)) {
+ const char *section = vsection;
+
+ if (!strcmp("transfer.hiderefs", var) ||
+ (!prefixcmp(var, section) &&
+ !strcmp(var + strlen(section), ".hiderefs"))) {
char *ref;
int len;
diff --git a/upload-pack.c b/upload-pack.c
index 37977e2..c0390af 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -794,7 +794,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
{
if (!strcmp("uploadpack.allowtipsha1inwant", var))
allow_tip_sha1_in_want = git_config_bool(var, value);
- return parse_hide_refs_config(var, value, unused);
+ return parse_hide_refs_config(var, value, "uploadpack");
}
int main(int argc, char **argv)
As an aside, I wonder if there is any point to the void pointer
parameter of parse_hide_refs_config. It is not used as a git_config
callback anywhere.
> > And I
> > have not seen complaints about the current system.
>
> Immediately after I added github to the set of places I push into,
> which I think is long before you joined GitHub, I noticed that _my_
> repository gets contaminated by second rate commits called pull
> requests, and I may even have complained, but most likely I didn't,
> as I could easily tell that, even though I know it is _not_ the only
> way, nor even the best way [*1*], to implement the GitHub's pull
> request workflow, I perfectly well understood that it would be the
> most expedient way for GitHub folks to implement this feature.
>
> I think you should take lack of complaints with a huge grain of
> salt. It does not suggest much.
Sure, I do not pretend that nobody cares. But it is certainly not a
pressing issue, or there would probably be more outcry. And we must also
weigh it against the silent majority that are perfectly happy with the
status quo, that lets them fetch refs/pull/* as any other ref.
In your case, I really think the problem is less that you have a problem
with PR refs in the repository, and more that you do not care about the
pull request feature at all. To you it is useless noise, both in the
repo and on the web. Your arguments about provenance could apply equally
well to PRs accessible via the web interface.
I think the refs/ clutter is only an issue if you want to do mirroring,
and then you have an obvious conflict: did the fetcher want to mirror
everything, including refs/pull, or do they consider that to be clutter?
Only the client knows, which is why I think refspecs are the right place
to deal with clutter (the fact that we cannot say "everything except
refs/pull/*" is a weakness in our refspecs).
> *1* From the ownership point of view, objects that are only
> reachable from these refs/pull/* refs do *not* belong to the
> requestee, until the requestee chooses to accept the changes.
>
> A malicious requestor can fork your repository, add an objectionable
> blob to it, and throw a pull request at you. GitHub shows that the
> blob now belongs to your repository, so the requestor turns around
> and file a DMCA takedown complaint against your repository. A
> clueful judge would then agree with the complaint after running a
> "clone --mirror" and seeing the blob in your repository. Oops?
I don't think this is a problem in practice. DMCA notices do not go to
the repository owner; they go to GitHub. And as far as I know, our
support staff deals with them on a case by case basis (and knows what a
pull request is, and who is responsible for the content in question). It
is not like they see a report of something in refs/pull and lock down
the parent repository; they can see where the request came from and deal
with it appropriately.
But again, such a notice could just as easily come from the list of open
PRs against your repo in the web interface.
> A funny thing is that you cannot "push :refs/pull/1/head" to remove
> it anymore (I think in the early days, I took them out by doing this
> a few times, but I may be misremembering),
We block updates to them explicitly in a hook; it looks like that went
in around mid-2011.
> The e-mail sent to you to let you know about outstanding pull
> requests and the web UI could just point at that forked repository,
> not your own (you also could choose to leave the outging pull
> requests in the requestor's repository, but that is only OK if you
> do not worry about (1) a requestor sending a pull request, then
> updating the branch the pull request talks about later, to trick you
> with bait-and-switch, or (2) a requestor sending a pull request,
> thinks he is done with the topic and removes the repository).
Yes, point (2) is the main reason they are not simply attached to the
sender's repository.
-Peff
^ permalink raw reply related
* Why is ident_is_sufficient different on Windows?
From: Max Horn @ 2013-02-06 13:06 UTC (permalink / raw)
To: git
Hi there,
while trying to understand which parts of the author & committer identity are mandatory (name, email, or both), I ended up in ident.c, looking at ident_is_sufficient(), and to my surprise discovered that this seems to differ between Windows (were both are mandatory) and everyone else:
static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
#else
return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
#endif
}
According to git blame, this was introduced here:
commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
Author: Junio C Hamano <gitster@pobox.com>
Date: Sun Jan 17 13:54:28 2010 -0800
user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere
The commit message sounds as if this was only a refactoring, but the patch to me look as if it changes behaviour, too. Of course this could very well be false, say due to code elsewhere that already caused Windows to behave differently; I wouldn't know.
Still, I wonder: Why does this difference exist?
Cheers,
Max
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox