Git development
 help / color / mirror / Atom feed
* Re: [PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header
From: Jakub Narebski @ 2006-09-26  7:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7izrw9la.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit
>> (although not documented in commit message) of quoting using quotemeta
>> the filename in HTTP -content_disposition header.  Uniquify output.
>>
>> Just in case filename contains end of line character.
> 
> What do you mean by "uniquify output"?

It means alsways use 'inline; filename="' . (quotemeta <whatever>) . '"',
and not sometimes qq(...), sometimes "...\"...\"", and sometimes '...'.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] Cleaned up git-daemon virtual hosting support.
From: Andreas Ericsson @ 2006-09-26  7:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jon Loeliger, git
In-Reply-To: <7vd59jw9lb.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
>> Added --host=host_or_ipaddr option suport.
> 
> I wonder if --listen= might be more appropriate name:
> 
>         "Listen a.b.c.d:port" (Apache),
>         "ListenAddress" (ssh daemon).
>         "--listen-host=" (svnserve).
> 
> I wanted to find an example or two who uses "bind" but I think
> listen is more popular.
> 

Apache has both Bind and Listen (at least 1.3 series). I've never 
understood the difference between the two, if there is one.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] gitweb: "raw" links to blobs at HEAD
From: Junio C Hamano @ 2006-09-26  6:46 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20060926053920.47709.qmail@web31802.mail.mud.yahoo.com>

This does not feel right.  Go to summary view, pick the tree
link of a non-head commit from the shortlog list, and the raw
links on the resulting page all say hb=HEAD.

Doesn't this make clicking the entry itself (blob view) and
clicking on the raw link (blob-plain view) inconsistent?  The
former goes to the blob from that revision while the latter
always goes to the HEAD.

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Junio C Hamano @ 2006-09-26  6:34 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20060926053816.54951.qmail@web31815.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> Binary and non-binary blobs:
>
> The "list" table element of tree view is identical
> to the "blob" link part of the link table element.
> I.e. clicking on "blob" is identical to clicking on
> the entry itself.

Then perhaps we can lose tree link for the same reasoning?

^ permalink raw reply

* [PATCH] gitweb: "raw" links to blobs at HEAD
From: Luben Tuikov @ 2006-09-26  5:39 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

Currently there is no HTML interface which references (or
can be externally referenced) to yield a _binary_ blob at
HEAD.  There is one for non-binary blobs, but none for
binary ones.

The reason is that "blob"/entry itself for non-binary
entries gives you the "Blob page" where you can click on
last link of the page path, but clicking on the binary
entry/"blob" gives you the raw file.

Binary blobs:
  Since "raw" == the entry itself, we can eliminate this
redundancy and set "raw" to the binary blob at HEAD. This
patch implements that.  If the user wants the binary blob at
hash/hash_base then they can click on the entry itself.

Non-binary blobs:
  With this patch "raw" yields the HEAD revision as per the
above comment for binary blobs.  If the raw output of the
hash/hash_base blob is required the user can click on the entry
(same as for binary only) and then click on "raw" (which
used to be named "plain" not so long ago) or last entry of
"page" path.

Compare the "raw" link for non-binary blobs from tree view,
to the alternative to getting it: click on entry->HEAD->last
entry of "page" path.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 1908141687-p2.txt --]
[-- Type: text/inline; name="p2.txt", Size: 610 bytes --]

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c7ab3b6..959e3f9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1625,9 +1625,9 @@ sub git_print_tree_entry {
 			              "history");
 		}
 		print " | " .
-		      $cgi->a({-href => href(action=>"blob_plain",
-		                             hash=>$t->{'hash'}, file_name=>"$basedir$t->{'name'}")},
-		              "raw");
+			$cgi->a({-href => href(action=>"blob_plain", hash_base=>"HEAD",
+					       file_name=>"$basedir$t->{'name'}")},
+				"raw");
 		print "</td>\n";
 
 	} elsif ($t->{'type'} eq "tree") {
-- 
1.4.2.1.gdbbb

^ permalink raw reply related

* [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Luben Tuikov @ 2006-09-26  5:38 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

Binary and non-binary blobs:

The "list" table element of tree view is identical
to the "blob" link part of the link table element.
I.e. clicking on "blob" is identical to clicking on
the entry itself.

Thus, eliminate "blob" from being shown -- the user
can get identical result by simply clicking on the
entry itself.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.perl |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 1207600725-p1.txt --]
[-- Type: text/inline; name="p1.txt", Size: 2304 bytes --]

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 66be619..c7ab3b6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1600,34 +1600,35 @@ sub git_print_tree_entry {
 	my %base_key = ();
 	$base_key{hash_base} = $hash_base if defined $hash_base;
 
+	# The format of a table row is: mode list link.  Where mode is
+	# the mode of the entry, list is the name of the entry, an href,
+	# and link is the action links of the entry.
+
 	print "<td class=\"mode\">" . mode_str($t->{'mode'}) . "</td>\n";
 	if ($t->{'type'} eq "blob") {
 		print "<td class=\"list\">" .
-		      $cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
-		                             file_name=>"$basedir$t->{'name'}", %base_key),
-		              -class => "list"}, esc_html($t->{'name'})) .
-		      "</td>\n" .
-		      "<td class=\"link\">" .
-		      $cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
-		                             file_name=>"$basedir$t->{'name'}", %base_key)},
-		              "blob");
+			$cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
+					       file_name=>"$basedir$t->{'name'}", %base_key),
+				 -class => "list"}, esc_html($t->{'name'})) . "</td>\n";
+		print "<td class=\"link\">";
 		if ($have_blame) {
-			print " | " .
-				$cgi->a({-href => href(action=>"blame", hash=>$t->{'hash'},
-				                       file_name=>"$basedir$t->{'name'}", %base_key)},
-				        "blame");
+			print $cgi->a({-href => href(action=>"blame", hash=>$t->{'hash'},
+						     file_name=>"$basedir$t->{'name'}", %base_key)},
+				      "blame");
 		}
 		if (defined $hash_base) {
-			print " | " .
-			      $cgi->a({-href => href(action=>"history", hash_base=>$hash_base,
+			if ($have_blame) {
+				print " | ";
+			}
+			print $cgi->a({-href => href(action=>"history", hash_base=>$hash_base,
 			                             hash=>$t->{'hash'}, file_name=>"$basedir$t->{'name'}")},
 			              "history");
 		}
 		print " | " .
 		      $cgi->a({-href => href(action=>"blob_plain",
 		                             hash=>$t->{'hash'}, file_name=>"$basedir$t->{'name'}")},
-		              "raw") .
-		      "</td>\n";
+		              "raw");
+		print "</td>\n";
 
 	} elsif ($t->{'type'} eq "tree") {
 		print "<td class=\"list\">" .
-- 
1.4.2.1.gdbbb

^ permalink raw reply related

* Re: [PATCH] Remove empty ref directories that prevent creating a ref.
From: Junio C Hamano @ 2006-09-26  5:27 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <20060926072337.39dbb9f4.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> +LF='
> +'
> +
> +if [ -d "$GIT_DIR/refs/heads/$branchname" ]
> +then
> +	OLD_IFS="$IFS"
> +	IFS="$LF"
> +	for refdir in `find "$GIT_DIR/refs/heads/$branchname" -type d | sort -r`
> +	do
> +		rmdir "$refdir" || \
> +		    die "Could not delete '$refdir'," \
> +		    "there may still be a ref there."
> +	done
> +	IFS="$OLD_IFS"
> +fi

I was sort of expecting that you would do something like:

	for refdir in `cd "$GIT_DIR" &&
		        find "refs/heads/$branchname" -type d |
		        sort -r`
	do                        
		rmdir "$GIT_DIR/$refdir" ...

wihtout mucking with IFS ;-). 

^ permalink raw reply

* Re: [PATCH] gitk: Fix nextfile() and more
From: Junio C Hamano @ 2006-09-26  5:24 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: git, Paul Mackerras
In-Reply-To: <873bag12k3.fsf@duaron.myhome.or.jp>

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> The current nextfile() jumps to last hunk, but I think this is not
> intention, probably, it's forgetting to add "break;". Right?  And this
> patch also adds prevfile(), it jumps to previous hunk.
>
> The following part is just my favorite the key-binds, it doesn't matter.
>
> +    bind . <Control-p> "$ctext yview scroll -1 units"
> +    bind . <Control-n> "$ctext yview scroll 1 units"
> +    bind . <Alt-v> "$ctext yview scroll -1 pages"
> +    bind . <Control-v> "$ctext yview scroll 1 pages"
> +    bindkey P prevfile
> +    bindkey N nextfile
>
> What do you think of this?

. As a patch e-mail, the initial Hi and trailing Thanks add
  additional work for the person who applies patch.

. ACK on missing "break" and addition of prevfile for symmetry.

. I do not have preference on the extra bindings.  I do not
  speak for Paul, but I suspect he would have liked them in a
  separate patch.

^ permalink raw reply

* [PATCH] Remove empty ref directories that prevent creating a ref.
From: Christian Couder @ 2006-09-26  5:23 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

Junio wrote:
"BTW, the second issue exists without packed ref; currently we
cannot do

    git branch foo/bar
    git branch -d foo/bar
    git branch foo"

This patch also add Linus and Junio test cases.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---

This fixed patch also applies to master, that's why it doesn't
contain test cases using "git pack-refs". I will send a separate
patch for them latter.

 git-branch.sh     |   16 ++++++++++++++++
 t/t3200-branch.sh |   12 ++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/git-branch.sh b/git-branch.sh
index e0501ec..834888d 100755
--- a/git-branch.sh
+++ b/git-branch.sh
@@ -112,6 +112,22 @@ rev=$(git-rev-parse --verify "$head") ||
 git-check-ref-format "heads/$branchname" ||
 	die "we do not like '$branchname' as a branch name."
 
+LF='
+'
+
+if [ -d "$GIT_DIR/refs/heads/$branchname" ]
+then
+	OLD_IFS="$IFS"
+	IFS="$LF"
+	for refdir in `find "$GIT_DIR/refs/heads/$branchname" -type d | sort -r`
+	do
+		rmdir "$refdir" || \
+		    die "Could not delete '$refdir'," \
+		    "there may still be a ref there."
+	done
+	IFS="$OLD_IFS"
+fi
+
 if [ -e "$GIT_DIR/refs/heads/$branchname" ]
 then
 	if test '' = "$force"
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5b04efc..6907cbc 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -61,4 +61,16 @@ test_expect_success \
 	 test -f .git/logs/refs/heads/g/h/i &&
 	 diff expect .git/logs/refs/heads/g/h/i'
 
+test_expect_success \
+    'git branch j/k should work after branch j has been deleted' \
+       'git-branch j &&
+        git-branch -d j &&
+        git-branch j/k'
+
+test_expect_success \
+    'git branch l should work after branch l/m has been deleted' \
+       'git-branch l/m &&
+        git-branch -d l/m &&
+        git-branch l'
+
 test_done
-- 
1.4.2.1.ged17-dirty

^ permalink raw reply related

* Re: [PATCH] Clarified documentation of --exclude-per-directory.
From: Junio C Hamano @ 2006-09-26  5:11 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060925155821.GC26844@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Tommi Virtanen noted on #git today that
>
>   git ls-files --exclude-per-directory
>
> doesn't appear to work as advertised by the documentation unless
> --others is also used.  According to the current source code this
> is the case as the --exclude-per-directory file isn't read unless
> we are iterating over the working directory, which only happens
> with --others.

I am puzzled by this problem description.

If we _were_ to read --exclude-per-directory file when we are
not doing --others, what better behaviour would we get out of
the command?

^ permalink raw reply

* Re: [PATCH] svnimport add support for parsing From lines for author
From: Junio C Hamano @ 2006-09-26  5:08 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <20060925110813.GA4419@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

> svnimport: add support for parsing From: lines for author

Please do _not_ repeat the summary line in the message body.  It
is on the Subject: already.

> Now that we have support for parsing Signed-off-by: for author
> information it makes sense to handle From: as well.

I take that you are referring to Sasha's change in ae35b304; I
asked for actual svn users for ACK/NACK but I did not hear any.
Can I understand that you use svnimport for real projects and
are happy with Sasha's change? --- that would be an ack that
would help me sleep better ;-).

> adds a new -F which will handle From: lines in the comments.  It
> may be used in combination with -S.

This sort of makes sense but how common is this?

I also wonder instead of piling up custom flags if it is better
to let match-and-extract pattern be specified from the command
line.

^ permalink raw reply

* Re: [PATCH] perl bindings fix compilation errors
From: Junio C Hamano @ 2006-09-26  4:54 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <4517BEBC.4000002@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

> I am somewhat unhappy that a make clean at the top level and remake was
> not sufficient to get a working tree.

Sympathetic I am (since I was bitten by it too), but I am not
sure what the right thing to do.

It is certainly incorrect to say that it is a driver error to
git-pull (or git-checkout to switch branches) without running
"make clean" first to get rid of perl/Git.c while your Makefile
still knows it is a generated file.  Our Makefile should be more
helpful.

But the question is Makefile in which branch?

Ideally, if Makefile of each and every branch head currently
alive knew that perl/Git.c is problematic and removed it, then
we would not suffer from this problem.  But it's obviously not
practical.

^ permalink raw reply

* Re: [PATCH] Introduce git-mirror, a tool for exactly mirroring another repository.
From: Junio C Hamano @ 2006-09-26  4:45 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git, Petr Baudis
In-Reply-To: <20060925044641.GB15757@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Sometimes its handy to be able to efficiently backup or mirror one
> Git repository to another Git repository by employing the native
> Git object transfer protocol.  But when mirroring or backing up a
> repository you really want:
>
>   1) Every object in the source to go to the mirror.
>   2) Every ref in the source to go to the mirror.
>   3) Any ref removed from the source to be removed from the mirror.
>   4) Automatically repack and prune the mirror when necessary.
>
> and since git-fetch doesn't do 2, 3, and 4 here's a tool that does.

Just a note.  I usually use git-push the other way for backups,
and I believe that is how Linus does it, too.

> diff --git a/git-mirror.perl b/git-mirror.perl
> new file mode 100755
> index 0000000..bff2003
> --- /dev/null
> +++ b/git-mirror.perl
> @@ -0,0 +1,111 @@
> +#!/usr/bin/env perl

Please don't.  "#!/usr/bin/env perl" is a disease.

> +# This file is licensed under the GPL v2, or a later version
> +# at the discretion of Linus.

Heh ;-).

> +use warnings;
> +use strict;
> +use Git;
> +
> +sub ls_refs ($$);

I wonder why people like line-noise prototypes.  Do you ever
call ls_refs with parameters that benefit from this?  Otherwise
I prefer not to see them.

> +my $remote = shift || 'origin';
> +my $repo = Git->repository();
> +
> +# Verify its OK to execute in this repository.
> +#
> +my $mirror_ok = $repo->config('mirror.allowed') || 0;
> +unless ($mirror_ok =~ /^(?:true|t|yes|y|1)$/i) {

This _is_ ugly.  Doesn't $repo->config() know how to drive
underlying "git-repo-config" with specific type argument?

> +# Delete any local refs which the server no longer contains.
> +#
> +foreach my $ref (keys %$local_refs) {
> +	next if $remote_refs->{$ref};
> +	print "removing $ref\n";
> +	my $log = "logs/$ref";
> +	unlink($repo->repo_path() . '/' . $ref);
> +	unlink($repo->repo_path() . '/' . $log);
> +	rmdir($repo->repo_path() . '/' . $ref) while $ref =~ s,/[^/]*$,,;
> +	rmdir($repo->repo_path() . '/' . $log) while $log =~ s,/[^/]*$,,;
> +}

If you do this upfront and then you lose connection while the
real fetch, next fetch may need to take a lot longer than needed
because it cannot rely on the refs you are losing here.  Ref
removal is rather a rare event, so we may not care too much
about it, though.

> +# Execute the fetch for any refs which differ from our own.
> +# We don't worry about trying to optimize for rewinds or
> +# exact branch copies as they are rather uncommon.

If we need to support only git-native protocols, all of this
optimization is not needed at all.  It's kind of sad that we
need to support commit walkers...

> +if (@to_fetch) {
> +	git_cmd_try {
> +		$repo->command_noisy('fetch',
> +			'--force',
> +			'--update-head-ok',
> +			$remote, sort @to_fetch);
> +	} '%s failed w/ code %d';

Why sort (no objection, just curious)?

> +# Repack if we have a large number of loose objects.
> +#
> +if (@to_fetch) {
> +	my $count_output = $repo->command('count-objects');
> +	my ($cur_loose) = ($count_output =~ /^(\d+) objects/);
> +	my $max_loose = $repo->config('mirror.maxlooseobjects') || 100;
> +	if ($cur_loose >= $max_loose) {
> +		git_cmd_try {
> +			$repo->command_noisy('repack', '-a', '-d');
> +			$repo->command_noisy('prune');
> +		} '%s failed w/ code %d';
> +	}
> +}

If we truly have a large number of objects (in pack and loose),
you do not want to do "repack -a -d", do you?

^ permalink raw reply

* [PATCH 2/3] update-ref: -d flag and ref creation safety.
From: Junio C Hamano @ 2006-09-26  4:13 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmz8o1em0.fsf_-_@assigned-by-dhcp.cox.net>

This adds -d flag to update-ref to allow safe deletion of ref.
Before deleting it, the command checks if the given <oldvalue>
still matches the value the caller thought the ref contained.

Similarly, it also accepts 0{40} as <oldvalue> to allow safe
creation of a new ref.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-update-ref.txt |    9 ++++++--
 builtin-update-ref.c             |   16 +++++++++++++--
 cache.h                          |    1 +
 refs.c                           |   41 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index e062030..723906d 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -7,7 +7,7 @@ git-update-ref - update the object name 
 
 SYNOPSIS
 --------
-'git-update-ref' [-m <reason>] <ref> <newvalue> [<oldvalue>]
+'git-update-ref' [-m <reason>] (-d <ref> <oldvalue> | <ref> <newvalue> [<oldvalue>])
 
 DESCRIPTION
 -----------
@@ -20,7 +20,8 @@ possibly dereferencing the symbolic refs
 the current value of the <ref> matches <oldvalue>.
 E.g. `git-update-ref refs/heads/master <newvalue> <oldvalue>`
 updates the master branch head to <newvalue> only if its current
-value is <oldvalue>.
+value is <oldvalue>.  You can specify 40 "0" as <oldvalue> to
+make sure that the ref you are creating does not exist.
 
 It also allows a "ref" file to be a symbolic pointer to another
 ref file by starting with the four-byte header sequence of
@@ -49,6 +50,10 @@ for reading but not for writing (so we'l
 ref symlink to some other tree, if you have copied a whole
 archive by creating a symlink tree).
 
+With `-d` flag, it deletes the named <ref> after verifying it
+still contains <oldvalue>.
+
+
 Logging Updates
 ---------------
 If config parameter "core.logAllRefUpdates" is true or the file
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index ab52833..486b8f2 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -3,15 +3,16 @@ #include "refs.h"
 #include "builtin.h"
 
 static const char git_update_ref_usage[] =
-"git-update-ref <refname> <value> [<oldval>] [-m <reason>]";
+"git-update-ref [-m <reason>] (-d <refname> <value> | <refname> <value> [<oldval>])";
 
 int cmd_update_ref(int argc, const char **argv, const char *prefix)
 {
 	const char *refname=NULL, *value=NULL, *oldval=NULL, *msg=NULL;
 	struct ref_lock *lock;
 	unsigned char sha1[20], oldsha1[20];
-	int i;
+	int i, delete;
 
+	delete = 0;
 	setup_ident();
 	git_config(git_default_config);
 
@@ -26,6 +27,10 @@ int cmd_update_ref(int argc, const char 
 				die("Refusing to perform update with \\n in message.");
 			continue;
 		}
+		if (!strcmp("-d", argv[i])) {
+			delete = 1;
+			continue;
+		}
 		if (!refname) {
 			refname = argv[i];
 			continue;
@@ -44,6 +49,13 @@ int cmd_update_ref(int argc, const char 
 
 	if (get_sha1(value, sha1))
 		die("%s: not a valid SHA1", value);
+
+	if (delete) {
+		if (oldval)
+			usage(git_update_ref_usage);
+		return delete_ref(refname, sha1);
+	}
+
 	hashclr(oldsha1);
 	if (oldval && get_sha1(oldval, oldsha1))
 		die("%s: not a valid old SHA1", oldval);
diff --git a/cache.h b/cache.h
index 97debd0..851f4c0 100644
--- a/cache.h
+++ b/cache.h
@@ -179,6 +179,7 @@ struct lock_file {
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
 extern void rollback_lock_file(struct lock_file *);
+extern int delete_ref(const char *, unsigned char *sha1);
 
 /* Environment bits from configuration mechanism */
 extern int use_legacy_headers;
diff --git a/refs.c b/refs.c
index b4f6d4f..4d499b0 100644
--- a/refs.c
+++ b/refs.c
@@ -212,6 +212,32 @@ int get_ref_sha1(const char *ref, unsign
 	return read_ref(git_path("refs/%s", ref), sha1);
 }
 
+int delete_ref(const char *refname, unsigned char *sha1)
+{
+	struct ref_lock *lock;
+	int err, i, ret = 0;
+
+	lock = lock_any_ref_for_update(refname, sha1);
+	if (!lock)
+		return 1;
+	i = strlen(lock->lk->filename) - 5; /* .lock */
+	lock->lk->filename[i] = 0;
+	err = unlink(lock->lk->filename);
+	if (err) {
+		ret = 1;
+		error("unlink(%s) failed: %s",
+		      lock->lk->filename, strerror(errno));
+	}
+	lock->lk->filename[i] = '.';
+
+	err = unlink(lock->log_file);
+	if (err && errno != ENOENT)
+		fprintf(stderr, "warning: unlink(%s) failed: %s",
+			lock->log_file, strerror(errno));
+
+	return ret;
+}
+
 /*
  * Make sure "ref" is something reasonable to have under ".git/refs/";
  * We do not like it if:
@@ -268,6 +294,19 @@ static struct ref_lock *verify_lock(stru
 {
 	char buf[40];
 	int nr, fd = open(lock->ref_file, O_RDONLY);
+
+	if (is_null_sha1(old_sha1)) {
+		/* we wanted to make sure it did not exist */
+		if (fd < 0 && errno == ENOENT)
+			return lock;
+		if (0 <= fd) {
+			close(fd);
+			error("ref %s should be absent but it exists",
+			      lock->ref_file);
+			unlock_ref(lock);
+			return NULL;
+		}
+	}
 	if (fd < 0 && errno != ENOENT) {
 		error("Can't verify ref %s", lock->ref_file);
 		unlock_ref(lock);
@@ -296,7 +335,7 @@ static struct ref_lock *lock_ref_sha1_ba
 	const char *orig_path = path;
 	struct ref_lock *lock;
 	struct stat st;
-	int mustexist = 0;
+	int mustexist = old_sha1 && !is_null_sha1(old_sha1);
 
 	lock = xcalloc(1, sizeof(struct ref_lock));
 	lock->lock_fd = -1;
-- 
1.4.2.1.g7a39b

^ permalink raw reply related

* [PATCH 1/3] Clean-up lock-ref implementation
From: Junio C Hamano @ 2006-09-26  4:13 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmz8o1em0.fsf_-_@assigned-by-dhcp.cox.net>

This drops "mustexist" parameter lock_ref_sha1() and lock_any_ref_forupdate()
functions take.  Existing callers were always passing 0.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * A bit of clean-up before the real changes...

 builtin-update-ref.c |    2 +-
 fetch.c              |    2 +-
 refs.c               |   20 +++++++++-----------
 refs.h               |    4 ++--
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 90a3da5..ab52833 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -48,7 +48,7 @@ int cmd_update_ref(int argc, const char 
 	if (oldval && get_sha1(oldval, oldsha1))
 		die("%s: not a valid old SHA1", oldval);
 
-	lock = lock_any_ref_for_update(refname, oldval ? oldsha1 : NULL, 0);
+	lock = lock_any_ref_for_update(refname, oldval ? oldsha1 : NULL);
 	if (!lock)
 		return 1;
 	if (write_ref_sha1(lock, sha1, msg) < 0)
diff --git a/fetch.c b/fetch.c
index 34df8d3..74585c4 100644
--- a/fetch.c
+++ b/fetch.c
@@ -266,7 +266,7 @@ int pull(int targets, char **target, con
 		if (!write_ref || !write_ref[i])
 			continue;
 
-		lock[i] = lock_ref_sha1(write_ref[i], NULL, 0);
+		lock[i] = lock_ref_sha1(write_ref[i], NULL);
 		if (!lock[i]) {
 			error("Can't lock ref %s", write_ref[i]);
 			goto unlock_and_fail;
diff --git a/refs.c b/refs.c
index 5e65314..b4f6d4f 100644
--- a/refs.c
+++ b/refs.c
@@ -264,12 +264,11 @@ int check_ref_format(const char *ref)
 	}
 }
 
-static struct ref_lock *verify_lock(struct ref_lock *lock,
-	const unsigned char *old_sha1, int mustexist)
+static struct ref_lock *verify_lock(struct ref_lock *lock, const unsigned char *old_sha1)
 {
 	char buf[40];
 	int nr, fd = open(lock->ref_file, O_RDONLY);
-	if (fd < 0 && (mustexist || errno != ENOENT)) {
+	if (fd < 0 && errno != ENOENT) {
 		error("Can't verify ref %s", lock->ref_file);
 		unlock_ref(lock);
 		return NULL;
@@ -292,11 +291,12 @@ static struct ref_lock *verify_lock(stru
 
 static struct ref_lock *lock_ref_sha1_basic(const char *path,
 	int plen,
-	const unsigned char *old_sha1, int mustexist)
+	const unsigned char *old_sha1)
 {
 	const char *orig_path = path;
 	struct ref_lock *lock;
 	struct stat st;
+	int mustexist = 0;
 
 	lock = xcalloc(1, sizeof(struct ref_lock));
 	lock->lock_fd = -1;
@@ -321,23 +321,21 @@ static struct ref_lock *lock_ref_sha1_ba
 		die("unable to create directory for %s", lock->ref_file);
 	lock->lock_fd = hold_lock_file_for_update(lock->lk, lock->ref_file, 1);
 
-	return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
+	return old_sha1 ? verify_lock(lock, old_sha1) : lock;
 }
 
-struct ref_lock *lock_ref_sha1(const char *ref,
-	const unsigned char *old_sha1, int mustexist)
+struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
 {
 	if (check_ref_format(ref))
 		return NULL;
 	return lock_ref_sha1_basic(git_path("refs/%s", ref),
-		5 + strlen(ref), old_sha1, mustexist);
+		5 + strlen(ref), old_sha1);
 }
 
-struct ref_lock *lock_any_ref_for_update(const char *ref,
-	const unsigned char *old_sha1, int mustexist)
+struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1)
 {
 	return lock_ref_sha1_basic(git_path("%s", ref),
-		strlen(ref), old_sha1, mustexist);
+		strlen(ref), old_sha1);
 }
 
 void unlock_ref(struct ref_lock *lock)
diff --git a/refs.h b/refs.h
index 553155c..d3bc295 100644
--- a/refs.h
+++ b/refs.h
@@ -24,10 +24,10 @@ extern int for_each_remote_ref(int (*fn)
 extern int get_ref_sha1(const char *ref, unsigned char *sha1);
 
 /** Locks a "refs/" ref returning the lock on success and NULL on failure. **/
-extern struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1, int mustexist);
+extern struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1);
 
 /** Locks any ref (for 'HEAD' type refs). */
-extern struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int mustexist);
+extern struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1);
 
 /** Release any lock taken but not written. **/
 extern void unlock_ref(struct ref_lock *lock);
-- 
1.4.2.1.g7a39b

^ permalink raw reply related

* Re: git send-email woes
From: Junio C Hamano @ 2006-09-26  4:11 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: git
In-Reply-To: <20060925215127.GA3965@uranus.ravnborg.org>

Sam Ravnborg <sam@ravnborg.org> writes:

>> @@ -431,6 +431,11 @@ sub send_message
>>  	    $gitversion =~ s/^.* //;
>>  	}
>>  
>> +	my ($author_name) = ($from =~ /^(.*?)\s+</);
>> +	if ($author_name =~ /\./ && $author_name !~ /^".*"$/) {
>> +		my ($name, $addr) = ($from =~ /^(.*?)(\s+<.*)/);
>> +		$from = "\"$name\"$addr";
>> +	}
>>  	my $header = "From: $from
>>  To: $to
>>  Cc: $cc
>> 
> I would assume we had to do the same for all mail addresses?
> In this case also for $to and $cc and $reply_to.

I do not think so, although I admit I only had a cursory look at
the code.  In this part of the code, $to and $cc are coming from
return value of unique_email_list sub, which cleanses each
address with extract_valid_address sub.  This sub does:

	A U Thor <foo@bar.baz> => foo@bar.baz

In other words, I think it is only "$from" that needs it.

I think $to and $cc should not strip out the human-readable name,
but in this case it saved us extra trouble.   Sigh...

^ permalink raw reply

* Re: [PATCH] Cleaned up git-daemon virtual hosting support.
From: Junio C Hamano @ 2006-09-26  4:11 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: git
In-Reply-To: <E1GRhhk-0004O6-GP@jdl.com>

> Added --host=host_or_ipaddr option suport.

I wonder if --listen= might be more appropriate name:

        "Listen a.b.c.d:port" (Apache),
        "ListenAddress" (ssh daemon).
        "--listen-host=" (svnserve).

I wanted to find an example or two who uses "bind" but I think
listen is more popular.

> This is on top of my previous patch in the next branch.
> This makes virtual host support way, way less stupid.
> I'll follow up with another patch to clean up the small leaks.

Thanks.

^ permalink raw reply

* Re: [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname
From: Junio C Hamano @ 2006-09-26  4:11 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200609260157.03035.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> @@ -387,16 +400,37 @@ sub href(%) {
>  ## ======================================================================
>  ## validation, quoting/unquoting and escaping
>  
> -sub validate_input {
> -	my $input = shift;
> +sub validate_pathname {
> +	my $input = shift || return undef;
>  
> -	if ($input =~ m/^[0-9a-fA-F]{40}$/) {
> -		return $input;
> +	# no '.' or '..' as elements of path, i.e. no '.' nor '..'
> +	# at the beginning, at the end, and between slashes.
> +	if ($input =~ m!(^|/)(|\.|\.\.)(/|$)!) {
> +		return undef;
>  	}
> -	if ($input =~ m/(^|\/)(|\.|\.\.)($|\/)/) {
> +	# no doubled slashes
> +	if ($input =~ m!//!) {
>  		return undef;
>  	}

I do not think you need the second check for double-slash.  The
pattern you borrowed from the original:

	/(^|\/)(|\.|\.\.)($|\/)/)

cleverly matches an empty string with $2, so you already match
double-slash with $1 = '/' $2 = '' $3 = '/', don't you?

> +	# it must be correct pathname
> +	$input = validate_pathname($input)
> +		or return undef;
> +	# restrictions on ref name according to git-check-ref-format
> +	if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
>  		return undef;
>  	}

Why would you need validate_pathname here?

^ permalink raw reply

* Re: [PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header
From: Junio C Hamano @ 2006-09-26  4:11 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200609260159.43684.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit
> (although not documented in commit message) of quoting using quotemeta
> the filename in HTTP -content_disposition header.  Uniquify output.
>
> Just in case filename contains end of line character.

What do you mean by "uniquify output"?

^ permalink raw reply

* Re: On ref locking
From: Junio C Hamano @ 2006-09-26  4:11 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Christian Couder, git, Linus Torvalds, Johannes Schindelin
In-Reply-To: <Pine.LNX.4.64.0609251140550.9789@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Mon, 25 Sep 2006, Junio C Hamano wrote:
>...
>> What the latter means is that an updater:
>> 
>>  (1) first learns the current value of the $ref, without
>>      locking;
>> 
>>  (2) decides based on the knowledge from (1) what the next value
>>      should be;
>> 
>>  (3) gets $ref.lock, makes sure $ref still is the value it
>>      learned in (1), updates it to the desired value and
>>      releases the lock.
>> 
>> The above 3-step sequence prevents updater-updater races with an
>> extremely short critical section.  We only need to hold the lock
>> while we do compare and swap.
>
> I remember having a certain amount of disagreement over whether it's 
> better to actually take the lock in (1), and hold it through (2), or only 
> verify that it didn't change in (3) after taking the lock for real. If 
> there's nothing substantial going on in (2), it doesn't matter. If users 
> are producing content (e.g., git tag), they may want to make sure that 
> nobody else is in the middle of writing something that would conflict.
>
> I think I'd advocated getting the lock early if you're going to want it, 
> and I don't remember what the convincing argument on the other side was 
> for the cases under consideration at the time, beyond it not mattering 
> significantly.
>
> Note that we make sure to remove the lock when aborting due to signals 
> (assuming we get a chance), so the size of the critical section doesn't 
> matter too much to the chance of it getting left locked inappropriately. 
> Of course, this is harder to do with the core code for a shell script than 
> for C code.

I do not recall that arguments on the list but what you say
makes sense.  Taking a lock early makes implementation more
cumbersome on the scripting side but reduces the annoyance
factor.  You will be annoyed if the tool tells you somebody else
did something to the ref you were going to update and your
intended operation is blocked after carefully editing your tag
message or commit log message.

However, I think the distributed nature of git makes it less of
an issue for normal repositories.

Shared distribution point repositories (where everybody pushes
to and fetches from) needs to protect between updaters lest that
we lose commits (fetching a rev older while a push is in
progress is not an error -- re-feching would give you the final
result), and that is what Johannes's recent patch does.  Pushing
into a repository with working tree does have a lot of issues
(e.g. push to update the current branch head while a commit is
in progress) but doing that as the workflow has problems for
other reasons as well (i.e. push to update the current branch
would make the index out of sync even when no commit is in
progress).  It is strongly discouraged to share $GIT_DIR/refs,
just like in any halfway sane development workflow people do not
share working tree files.

So "locking before letting the user spend effort so that we
would not later have to say 'sorry, we cannot let you do that
because somebody else updated the ref in the meanwhile'" is
technically not incorrect, and we should be able to make things
work that way, but I think it pretty much is a "why bother"
issue.  "git tag" checks if the new tag being created does not
exist to reduce the annoyance factor, and the check does not
even attempt to avoid a race.  If we revalidated that the tag is
still not there while storing the tag object name to the ref in
the final step, I think that is good enough, and I think having
"lock during operation" buys us much real-life benefit.

git-commit has the same attitude, and I think it is the right
balance.  It checks if the commit being created is an initial
commit, a single-parent commit or a merge first, prepares "-p"
arguments to the commit-tree that will happen much later, then
lets the user prepare the log message.  Theoretically somebody
else could update the current HEAD in the meantime and the
resulting commit can have ancestors that are different from what
was intended.  We do check and avoid it by making sure that the
HEAD is still where we thought it would be before doing the
final.

^ permalink raw reply

* [PATCH 4/5] gitweb: Add git_url subroutine, and use it to quote full URLs
From: Jakub Narebski @ 2006-09-25 23:58 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis
In-Reply-To: <200609260153.08503.jnareb@gmail.com>

Add git_url subroutine, which does what git_param did before commit
a2f3db2f5de2a3667b0e038aa65e3e097e642e7d, and is used to quote full
URLs, currently only $home_link.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It will also be used in patch inspired by
  "[RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage"
  Message-ID: <7virjes7dq.fsf@assigned-by-dhcp.cox.net>
  http://permalink.gmane.org/gmane.comp.version-control.git/27619

 gitweb/gitweb.perl |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7fce9a6..b51e061 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -446,6 +446,15 @@ sub esc_param {
 	return $str;
 }
 
+# quote unsafe chars in whole URL, so some charactrs cannot be quoted
+sub esc_url {
+	my $str = shift;
+	$str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&=])/sprintf("%%%02X", ord($1))/eg;
+	$str =~ s/\+/%2B/g;
+	$str =~ s/ /\+/g;
+	return $str;
+}
+
 # replace invalid utf8 character with SUBSTITUTION sequence
 sub esc_html {
 	my $str = shift;
@@ -1362,7 +1371,7 @@ EOF
 	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
 	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
 	      "</a>\n";
-	print $cgi->a({-href => esc_param($home_link)}, $home_link_str) . " / ";
+	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
 		if (defined $action) {
-- 
1.4.2.1

^ permalink raw reply related

* [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname
From: Jakub Narebski @ 2006-09-25 23:57 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis
In-Reply-To: <200609260153.08503.jnareb@gmail.com>

Split validate_input subroutine into validate_pathname which is used
for $project, $file_name and $file_parent parameters, and
validate_refname which is used for $hash, $hash_base, $hash_parent and
$hash_parent_base parameters.  Reintroduce validation of $file_name
and $file_parent parameters, removed in a2f3db2f5de2a3667b0e038aa65e3

validate_pathname in addition to what validate_input did checks also
for doubled slashes and NUL character. It does not check if input is
textual hash, and does not check if all characters are from the
following set: [a-zA-Z0-9_\x80-\xff\ \t\.\/\-\+\#\~\%].

validate_refname first check if the input is textual hash, then checks
if it is valid pathname, then checks for invalid characters (according
to git-check-ref-format manpage). It does not check if all charactes
are from the [a-zA-Z0-9_\x80-\xff\ \t\.\/\-\+\#\~\%] set.

We do not reintroduce validating pathnames we got from git.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
At last.

 gitweb/gitweb.perl |   66 +++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6458d7b..7fce9a6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -200,9 +200,10 @@ if (defined $action) {
 	}
 }
 
+# parameters which are pathnames
 our $project = $cgi->param('p');
 if (defined $project) {
-	if (!validate_input($project) ||
+	if (!validate_pathname($project) ||
 	    !(-d "$projectroot/$project") ||
 	    !(-e "$projectroot/$project/HEAD") ||
 	    ($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
@@ -212,38 +213,50 @@ if (defined $project) {
 	}
 }
 
-# We have to handle those containing any characters:
 our $file_name = $cgi->param('f');
+if (defined $file_name) {
+	if (!validate_pathname($file_name)) {
+		die_error(undef, "Invalid file parameter");
+	}
+}
+
 our $file_parent = $cgi->param('fp');
+if (defined $file_parent) {
+	if (!validate_pathname($file_parent)) {
+		die_error(undef, "Invalid file parent parameter");
+	}
+}
 
+# parameters which are refnames
 our $hash = $cgi->param('h');
 if (defined $hash) {
-	if (!validate_input($hash)) {
+	if (!validate_refname($hash)) {
 		die_error(undef, "Invalid hash parameter");
 	}
 }
 
 our $hash_parent = $cgi->param('hp');
 if (defined $hash_parent) {
-	if (!validate_input($hash_parent)) {
+	if (!validate_refname($hash_parent)) {
 		die_error(undef, "Invalid hash parent parameter");
 	}
 }
 
 our $hash_base = $cgi->param('hb');
 if (defined $hash_base) {
-	if (!validate_input($hash_base)) {
+	if (!validate_refname($hash_base)) {
 		die_error(undef, "Invalid hash base parameter");
 	}
 }
 
 our $hash_parent_base = $cgi->param('hpb');
 if (defined $hash_parent_base) {
-	if (!validate_input($hash_parent_base)) {
+	if (!validate_refname($hash_parent_base)) {
 		die_error(undef, "Invalid hash parent base parameter");
 	}
 }
 
+# other parameters
 our $page = $cgi->param('pg');
 if (defined $page) {
 	if ($page =~ m/[^0-9]/) {
@@ -273,7 +286,7 @@ sub evaluate_path_info {
 		$project =~ s,/*[^/]*$,,;
 	}
 	# validate project
-	$project = validate_input($project);
+	$project = validate_pathname($project);
 	if (!$project ||
 	    ($export_ok && !-e "$projectroot/$project/$export_ok") ||
 	    ($strict_export && !project_in_list($project))) {
@@ -294,12 +307,12 @@ sub evaluate_path_info {
 		} else {
 			$action  ||= "blob_plain";
 		}
-		$hash_base ||= validate_input($refname);
-		$file_name ||= $pathname;
+		$hash_base ||= validate_refname($refname);
+		$file_name ||= validate_pathname($pathname);
 	} elsif (defined $refname) {
 		# we got "project.git/branch"
 		$action ||= "shortlog";
-		$hash   ||= validate_input($refname);
+		$hash   ||= validate_refname($refname);
 	}
 }
 evaluate_path_info();
@@ -387,16 +400,37 @@ sub href(%) {
 ## ======================================================================
 ## validation, quoting/unquoting and escaping
 
-sub validate_input {
-	my $input = shift;
+sub validate_pathname {
+	my $input = shift || return undef;
 
-	if ($input =~ m/^[0-9a-fA-F]{40}$/) {
-		return $input;
+	# no '.' or '..' as elements of path, i.e. no '.' nor '..'
+	# at the beginning, at the end, and between slashes.
+	if ($input =~ m!(^|/)(|\.|\.\.)(/|$)!) {
+		return undef;
 	}
-	if ($input =~ m/(^|\/)(|\.|\.\.)($|\/)/) {
+	# no doubled slashes
+	if ($input =~ m!//!) {
 		return undef;
 	}
-	if ($input =~ m/[^a-zA-Z0-9_\x80-\xff\ \t\.\/\-\+\#\~\%]/) {
+	# no null characters
+	if ($input =~ m!\0!) {
+		return undef;
+	}
+	return $input;
+}
+
+sub validate_refname {
+	my $input = shift || return undef;
+
+	# textual hashes are O.K.
+	if ($input =~ m/^[0-9a-fA-F]{40}$/) {
+		return $input;
+	}
+	# it must be correct pathname
+	$input = validate_pathname($input)
+		or return undef;
+	# restrictions on ref name according to git-check-ref-format
+	if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
 		return undef;
 	}
 	return $input;
-- 
1.4.2.1

^ permalink raw reply related

* [PATCH 2/5] gitweb: Use "return" instead of "return undef" for some subs
From: Jakub Narebski @ 2006-09-25 23:56 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis
In-Reply-To: <200609260153.08503.jnareb@gmail.com>

Use "return" instead of "return undef" when subroutine can return, or
always return, non-scalar (list) value.

Other places are left as is.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is inspired by "gitweb: Fix @git_base_url_list usage" patch
(74d6166751ddcf08029ffc90a14158a86f80cd40) by Petr Baudis.

 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2ad7eed..6458d7b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -106,7 +106,7 @@ our %feature = (
 
 sub gitweb_check_feature {
 	my ($name) = @_;
-	return undef unless exists $feature{$name};
+	return unless exists $feature{$name};
 	my ($sub, $override, @defaults) = (
 		$feature{$name}{'sub'},
 		$feature{$name}{'override'},
@@ -781,7 +781,7 @@ sub git_get_projects_list {
 		# 'git%2Fgit.git Linus+Torvalds'
 		# 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'
 		# 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'
-		open my ($fd), $projects_list or return undef;
+		open my ($fd), $projects_list or return;
 		while (my $line = <$fd>) {
 			chomp $line;
 			my ($path, $owner) = split ' ', $line;
-- 
1.4.2.1

^ permalink raw reply related

* [PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header
From: Jakub Narebski @ 2006-09-25 23:59 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis
In-Reply-To: <200609260153.08503.jnareb@gmail.com>

Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit
(although not documented in commit message) of quoting using quotemeta
the filename in HTTP -content_disposition header.  Uniquify output.

Just in case filename contains end of line character.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b51e061..4dd7a5d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2326,7 +2326,7 @@ sub git_project_index {
 	print $cgi->header(
 		-type => 'text/plain',
 		-charset => 'utf-8',
-		-content_disposition => qq(inline; filename="index.aux"));
+		-content_disposition => 'inline; filename="index.aux"');
 
 	foreach my $pr (@projects) {
 		if (!exists $pr->{'owner'}) {
@@ -2672,7 +2672,7 @@ sub git_blob_plain {
 	print $cgi->header(
 		-type => "$type",
 		-expires=>$expires,
-		-content_disposition => "inline; filename=\"$save_as\"");
+		-content_disposition => 'inline; filename="' . quotemeta($save_as) . '"');
 	undef $/;
 	binmode STDOUT, ':raw';
 	print <$fd>;
@@ -2846,10 +2846,11 @@ sub git_snapshot {
 
 	my $filename = basename($project) . "-$hash.tar.$suffix";
 
-	print $cgi->header(-type => 'application/x-tar',
-	                   -content_encoding => $ctype,
-	                   -content_disposition => "inline; filename=\"$filename\"",
-	                   -status => '200 OK');
+	print $cgi->header(
+		-type => 'application/x-tar',
+		-content_encoding => $ctype,
+		-content_disposition => 'inline; filename="' . quotemeta($filename) . '"',
+		-status => '200 OK');
 
 	my $git_command = git_cmd_str();
 	open my $fd, "-|", "$git_command tar-tree $hash \'$project\' | $command" or
@@ -3159,7 +3160,7 @@ sub git_blobdiff {
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => qq(inline; filename=") . quotemeta($file_name) . qq(.patch"));
+			-content_disposition => 'inline; filename="' . quotemeta($file_name) . '.patch"');
 
 		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
 
@@ -3262,7 +3263,7 @@ sub git_commitdiff {
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => qq(inline; filename="$filename"));
+			-content_disposition => 'inline; filename="' . quotemeta($filename) . '"');
 		my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
 		print <<TEXT;
 From: $co{'author'}
-- 
1.4.2.1

^ permalink raw reply related

* [PATCH 1/5] gitweb: Strip trailing slashes from $path in git_get_hash_by_path
From: Jakub Narebski @ 2006-09-25 23:54 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis
In-Reply-To: <200609260153.08503.jnareb@gmail.com>

It also removes unused local variable $tree 

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 66be619..2ad7eed 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -710,7 +710,7 @@ sub git_get_hash_by_path {
 	my $path = shift || return undef;
 	my $type = shift;
 
-	my $tree = $base;
+	$path =~ s,/+$,,;
 
 	open my $fd, "-|", git_cmd(), "ls-tree", $base, "--", $path
 		or die_error(undef, "Open git-ls-tree failed");
-- 
1.4.2.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox