Skip to contentSkip to conversation

git-daemon virtual hosting implementation.

PATCH In review Pierre Habouzit 7 participants 17 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 git-daemon virtual hosting implementation.

just add the hostname in the path when using --base-path and --user-path. this should be enough for most needs.

---
Here is a proposal for daemon side virtualhosting support.

This is quite simple, and just uses the hostname in the path where it looks for the git repository. Only works in conjuction of --base-path or --user-path btw.

 Documentation/git-daemon.txt |   12 +++++++
 daemon.c                     |   73 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 73 insertions(+), 12 deletions(-)
Documentation/git-daemon.txt +12 -0
index 0f7d274..ab5b232 100644
@@ -79,6 +79,18 @@ OPTIONS
 	taken as a request to access `path/foo` repository in
 	the home directory of user `alice`.
 
+--use-vhosts::
+	Use git daemon cheap virtual hosting scheme.
++
+Used with --base-path=/foo, it will search the repositories in /foo/HOSTNAME
+instead of /foo - if you try to pull git://git.example.com/hello.git, it will
+search into /foo/git.example.com/hello.git.
++
+When using --user-path the HOSTNAME is appended to the path as well, so for
+--user-path=some/path, request to git://git.example.com/~alice/foo is taken as
+a request to access some/path/git.example.com/foo in the home directory of
+user `alice`.
+
 --verbose::
 	Log details about the incoming connections and requested files.
 
daemon.c +61 -12

145 lines not on this page · raw message

Signature · 1 line
1.4.1.1
#1 lore raw
Quoted 8 lines · Pierre Habouzit

just add the hostname in the path when using --base-path and --user-path. this should be enough for most needs.

---
Here is a proposal for daemon side virtualhosting support.

daemon.c static char *path_ok(char *dir) · line 167

Quoted 8 lines
daemon.c +5 -0
@@ -158,6 +160,11 @@ static char *path_ok(char *dir)
 		return NULL;
 	}
 
+	if (use_vhosts && !vhost) {
+		logerror("using virtual hosting, and not host= was specified !");
+		return NULL;
+	}
+

This part is objectionable -- older clients do not give "host=". I think the plan, when virtual hosting was proposed and we added this to the client side first, was to treat older clients as if they specified the "primary" host. So we would need some mechanism to say where the repositories of the "primary" host lives.

daemon.c static char *path_ok(char *dir) · line 194

Quoted 10 lines
daemon.c +10 -0
+			if (use_vhosts) {
+				loginfo("host <%s>, "
+					"userpath <%s>, request <%s>, "
+					"namlen %d, restlen %d, slash <%s>",
+					vhost,
+					user_path, dir,
+					namlen, restlen, slash);
+				snprintf(rpath, PATH_MAX, "%.*s/%s/%s%.*s",
+					 namlen, dir, user_path, vhost,
+					 restlen, slash);

I am not sure if the interaction between user-path and vhost should go like this, but I do not think of a good alternative to suggest right now. Your code allows ~user/host1 and ~user/host2 to host different set of repositories, but I suspect if somebody is setting up a virtual hosting of two hosts, he might want to have two distinct set of users (iow to pretend that ~user exist only on host1 but not on host2). I dunno.

#2 lore raw

PATCH 1/1 git-daemon: more powerful base-path/user-path settings, using formats.

Allow a form of virtualhosting, when %h format is used.

---

This is intended to be a more flexible solution, that also gives virtual hosting as a bonus. I still see no way to deal with older clients when virtual hosting is used by the admin though, having a "default" hostname won't solve anything at all anyway.

 Documentation/git-daemon.txt |   42 ++++++++++
 daemon.c                     |  170 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 201 insertions(+), 11 deletions(-)
Documentation/git-daemon.txt +42 -0
index 0f7d274..0aa34d4 100644
@@ -11,6 +11,7 @@ SYNOPSIS
 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]
              [--timeout=n] [--init-timeout=n] [--strict-paths]
              [--base-path=path] [--user-path | --user-path=path]
+             [--base-path-fmt=pathfmt] [--user-path-fmt=pathfmt]
 	     [--reuseaddr] [--detach] [--pid-file=file] [directory...]
 
 DESCRIPTION
@@ -45,6 +46,10 @@ OPTIONS
 	'git://example.com/hello.git', `git-daemon` will interpret the path
 	as '/srv/git/hello.git'.
 
+--base-path-fmt=pathfmt::
+	Works like --base-path, but uses a format instead. See Path
+	Formats section below.
+
 --export-all::
 	Allow pulling from all directories that look like GIT repositories
 	(have the 'objects' and 'refs' subdirectories), even if they
@@ -79,6 +84,11 @@ OPTIONS
 	taken as a request to access `path/foo` repository in
 	the home directory of user `alice`.
 
+--user-path-fmt=pathfmt::
+	Allow ~user notation to be used in requests. The path used to look
+	for the git repository is a format string, see Path Formats
+	section below.
+
 --verbose::
 	Log details about the incoming connections and requested files.
 
@@ -98,6 +108,38 @@ OPTIONS
 	--strict-paths is specified this will also include subdirectories
 	of each named directory.
 
+Path Formats
+------------
+
+%h::
+	requested hostname (won't work with clients older than 1.4.0).
+
+%p::
+	requested path.
+
+%P::
+	requested path without the first slash when exists (`/`).
+
+%u::
+	requested username (only allowed for --user-path-fmt).
+
+%%::
+	plain '%'
+
+Examples
+~~~~~~~~
+
+* `--base-path-fmt=/srv/git/%P` emulates `--base-path=/srv/git`
+
+* `--base-path-fmt=/srv/git/%h%p` will look into
+  `/srv/git/git.example.com/foo` if the request goes to
+  git://git.example.com/foo
+
+* `--user-path-fmt=/home/%u/public_git/%P` works mostly like
+  `--user-path=public_git` (the difference is that it assumes that home
+  directories live in `/home/username` instead of using the real `username`
+  home).
+
 Author
 ------
 Written by Linus Torvalds <[email protected]>, YOSHIFUJI Hideaki
daemon.c +159 -11

257 lines not on this page · raw message

Signature · 1 line
1.4.2.g44c10-dirty
#3 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 9 lines · Pierre Habouzit

Allow a form of virtualhosting, when %h format is used.

---

This is intended to be a more flexible solution, that also gives virtual hosting as a bonus. I still see no way to deal with older clients when virtual hosting is used by the admin though, having a "default" hostname won't solve anything at all anyway.

I mildly disagree about the last sentence. Enabling virtual hosting does not have to mean all virtual hosts are treated equal. It is conceivable that a site hosts the primary, "collection of public repositories everybody would want to go to" set, with supplemental ones for specific audiences that are done via virtual hosting. General public who would want to access the primary one can come with older clients that way, and only the narrower audiences have to be told to upgrade.

The client-side host= support was done post 1.4.0-rc1 timeframe so we have to be nicer to 1.3 based people, at least give them a way to slurp newer version with their client ;-).

Haven't looked at the rest of the patch yet. Will comment later.

#4 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 11 lines · Junio C Hamano
Quoted 9 lines · Pierre Habouzit

Allow a form of virtualhosting, when %h format is used.

---

This is intended to be a more flexible solution, that also
gives virtual hosting as a bonus. I still see no way to deal with older clients when virtual hosting is used by the admin though, having a "default" hostname won't solve anything at all anyway.

Quoted 8 lines

I mildly disagree about the last sentence. Enabling virtual hosting does not have to mean all virtual hosts are treated equal. It is conceivable that a site hosts the primary, "collection of public repositories everybody would want to go to" set, with supplemental ones for specific audiences that are done via virtual hosting. General public who would want to access the primary one can come with older clients that way, and only the narrower audiences have to be told to upgrade.

hmm, yes, that's indeed fair. Well, adding a --default-hostname that is used if no host= is passed is completely obvious and straightforward and solves that issue.

I also spotted a bug in the git_path_fmt function, I test 'pos' at the end of the loop, but the overflow test is not done when there is no format involved, wich could /theorically/ lead to buffer overflow. So please disregard that issue, I will at least provide a patch that fixes that.

Quoted 2 lines

Haven't looked at the rest of the patch yet. Will comment later.

np, TIA

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org
#5 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

In reply to Pierre Habouzit
Quoted 7 lines · Pierre Habouzit

Allow a form of virtualhosting, when %h format is used.

---

This is intended to be a more flexible solution, that also gives virtual hosting as a bonus.

Nicely done, almost.

Having to have the distinction between %p and %P formats feels somewhat unwieldy, though. Not that I have a better suggestion.

daemon.c static int strict_paths; · line 40

Quoted 2 lines
daemon.c +2 -0
+int is_base_path_fmt;
+int is_user_path_fmt;

I prefer these to be of type "static int".

Although I am not an authority of variable naming, these sound funny to me. "is_XXX()" as a function name feels natural, "is_XXX" as a variable name does not --- it is not clear what the predicate is talking about.

Maybe "use_fmt_for_base_path" is easier to understand? I dunno. Or "user_path_is_fmt"? That's more logical but still somewhat feels funny.

#6 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 11 lines · Junio C Hamano
Quoted 7 lines · Pierre Habouzit

Allow a form of virtualhosting, when %h format is used.

---

This is intended to be a more flexible solution, that also
gives virtual hosting as a bonus.

Nicely done, almost.

Quoted 7 lines

Having to have the distinction between %p and %P formats feels somewhat unwieldy, though. Not that I have a better suggestion.

daemon.c static int strict_paths; · line 40

Quoted 2 lines
daemon.c +2 -0
+int is_base_path_fmt;
+int is_user_path_fmt;

I prefer these to be of type "static int".

omg, how did I missed that.

Quoted 8 lines

Although I am not an authority of variable naming, these sound funny to me. "is_XXX()" as a function name feels natural, "is_XXX" as a variable name does not --- it is not clear what the predicate is talking about.

Maybe "use_fmt_for_base_path" is easier to understand? I dunno. Or "user_path_is_fmt"? That's more logical but still somewhat feels funny.

agreed.

There is also a second patch that never made it to the list that fixes:
* some indentation problems due to a bad vimrc
* --default-hostname switch (to handle virtual hosts even with older clients)
* possible overflow in the formatting method.

I'll recompute a new patch that superseeds that one, and merge your comments and my never sent patch too.

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org
#7 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 8 lines · Pierre Habouzit

There is also a second patch that never made it to the list that fixes:
* some indentation problems due to a bad vimrc
* --default-hostname switch (to handle virtual hosts even with older clients)
* possible overflow in the formatting method.

I'll recompute a new patch that superseeds that one, and merge your comments and my never sent patch too.

I have to admit that I kinda liked JDL's simpler one first (and it has been in production use for some time). We'll see.

About vger potentially throwing things away, I use this script (called "taboo.perl") to check my messages before sending them out.

Obviously the taboo-word list itself is not attached here, but the actual script should have a copy of it after the __DATA__ marker.

-- >8 --

#!/usr/bin/perl -w

my $tmpl = '	if (%%PATTERN%%) {
		print "$lineno ${_}matches %%QPATTERN%%\n";
		return;
	}

';
my $stmt = "";
my $in_header = 1;

while (<DATA>) {
	if (/^\$global_taboo_body =/) {
		$in_header = 0;
	}
	next if (/^\043/ || /^\$/ || /^END$/ || /^\s*$/);
	chomp;
	my $p = $_;
	if ($in_header) {
		$p = '/^[-\w_]*:/ && ' . $p;
	}
	my $q = quotemeta($p);
	my $stmt1 = $tmpl;
	$stmt1 =~ s|%%PATTERN%%|$p|g;
	$stmt1 =~ s|%%QPATTERN%%|$q|g;
	$stmt .= $stmt1;
}

close DATA;

$stmt = 'sub check {
	my ($line, $lineno) = @_;

' . $stmt . '
}
';
eval $stmt;

while (<>) {
	check($_, $.);
}
my $how_to_update_this_script = <<'EOF' ;
	( sed -e '/^__DATA__$$/q' taboo.perl && \
	  wget -q -O - http://vger.kernel.org/majordomo-taboos.txt ) \
		>taboo.perl+
	if diff -u taboo.perl taboo.perl+; \
	then \
		rm -f taboo.perl+; \
		echo >&2 No changes.; \
	else \
		mv taboo.perl+ taboo.perl; \
		chmod +x taboo.perl; \
	fi

EOF

__DATA__

#8 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 3 lines · Junio C Hamano

About vger potentially throwing things away, I use this script (called "taboo.perl") to check my messages before sending them out.

that was not it, I was biten (again) by git-send-mail that uses strftime (localized) to generate rfc822 dates, making them unparseable :|

I've resent the aggregated patch, that should work right now.

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org
#9 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 7 lines · Pierre Habouzit
Quoted 3 lines · Junio C Hamano

About vger potentially throwing things away, I use this script (called "taboo.perl") to check my messages before sending them out.

that was not it, I was biten (again) by git-send-mail that uses strftime (localized) to generate rfc822 dates, making them unparseable :|

Update your git, or use ./git-send-email.perl directly from git repository (from 'master' branch, as 'next' branch version uses Git.pm). Strftime was replaced by pure Perl to generate rfc2822 date some time ago...

Signature · 3 lines
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
#10 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

Quoted 6 lines · Jakub Narebski
Quoted 5 lines · Pierre Habouzit
Quoted 3 lines · Junio C Hamano

About vger potentially throwing things away, I use this script (called "taboo.perl") to check my messages before sending them out.

Quoted 8 lines
Quoted 3 lines

that was not it, I was biten (again) by git-send-mail that uses strftime (localized) to generate rfc822 dates, making them unparseable :|

Update your git, or use ./git-send-email.perl directly from git repository (from 'master' branch, as 'next' branch version uses Git.pm). Strftime was replaced by pure Perl to generate rfc2822 date some time ago...

I know that, I've done that on many of my system, but not the one where I hack git ... how lame isn't it ? :)

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org
#11 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

In reply to Junio C Hamano
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Junio" == Junio C Hamano <[email protected]> writes:

Junio> About vger potentially throwing things away, I use this script Junio> (called "taboo.perl") to check my messages before sending them Junio> out.

Junio> Obviously the taboo-word list itself is not attached here, but Junio> the actual script should have a copy of it after the __DATA__ Junio> marker.

With "Inline::Files" from the CPAN, you could have a switch where the script updates itself with the new list:

    use Inline::Files;
    if (@ARGV == 1 and $ARGV[0] eq "-update") {
      use LWP::Simple;
      my $list = get "http://example.com/foo/bar.txt";
      open DATA, ">$DATA" or die "cannot write myself: $!";
      print DATA $list;
      close DATA;
      exit 0;
    }

    ... rest of your program here ...
    ... read using <DATA> as before ...

    __DATA__
    the list will magically go here.
Signature · 4 lines
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<[email protected]> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
#12 lore raw

Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.

In reply to Junio C Hamano

The other day, Junio lamented:

Quoted 2 lines

I have to admit that I kinda liked JDL's simpler one first (and it has been in production use for some time). We'll see.

I think the two aspects of my implementation that are favorable are the slightly more general table-driven string interplotion routine and the generalization of the interface to the upload() call here:

    @@ -310,8 +377,14 @@ #endif
    	if (len && line[len-1] == '\n')
    		line[--len] = 0;
     
    -	if (!strncmp("git-upload-pack ", line, 16))
    -		return upload(line+16);
    +	if (len != pktlen) {
    +	    parse_extra_args(line + len + 1, pktlen - len - 1);
    +	}
    +
    +	if (!strncmp("git-upload-pack ", line, 16)) {
    +		interp_table[INTERP_SLOT_DIR].value = line+16;
    +		return upload(interp_table);
    +	}

Naturally, I only placed entries into the interpolation table that I needed to get my code working, but it could easily be extended and filled with additional entries such as the %u for user paths and %IP for IP address, etc. I might even recommend some form of lower-case-izing option too.

In any event, we should clearly attempt to unify my proposed implementation with Pierre's proposal.

jdl

#13 lore raw
In reply to Junio C Hamano
Quoted 12 lines · Junio C Hamano
Quoted 11 lines · Pierre Habouzit

just add the hostname in the path when using --base-path and --user-path. this should be enough for most needs.

---
Here is a proposal for daemon side virtualhosting support.

daemon.c +0 -0
@@ -158,6 +160,11 @@ static char *path_ok(char *dir)
 		return NULL;
 	}
 
Quoted 12 lines

daemon.c static char *path_ok(char *dir) · line 167

Quoted 5 lines
daemon.c +4 -0
+	if (use_vhosts && !vhost) {
+		logerror("using virtual hosting, and not host= was specified
 !"); +		return NULL;
+	}
+

This part is objectionable -- older clients do not give "host=". I think the plan, when virtual hosting was proposed and we added this to the client side first, was to treat older clients as if they specified the "primary" host. So we would need some mechanism to say where the repositories of the "primary" host lives.

fair enough, so I suppose there is many choices:
* replace --use-vhosts with --use-vhosts=${default hostname} but I do not like it very much
* use hostname -f as the default hostname (not very nice either)
* use the magic _default_ (à la apache) hostname ?

But just note that if a git repository is accessible on an host that is not the default, and only on that one, an older client won't be able to clone the repository anyway, because he will be redirected to the default virtual host. Meaning, that someone that uses virtual hosts will break older clients anyway.

what would be nicer would be a way to gracefully reject older clients in that case with an understandable error message, so that the user knows why it does not work. I don't know PROTO_GIT very well yet,so I don't know if older client support such communications yet or not.

Quoted 11 lines

daemon.c static char *path_ok(char *dir) · line 194

Quoted 10 lines
daemon.c +10 -0
+			if (use_vhosts) {
+				loginfo("host <%s>, "
+					"userpath <%s>, request <%s>, "
+					"namlen %d, restlen %d, slash <%s>",
+					vhost,
+					user_path, dir,
+					namlen, restlen, slash);
+				snprintf(rpath, PATH_MAX, "%.*s/%s/%s%.*s",
+					 namlen, dir, user_path, vhost,
+					 restlen, slash);
Quoted 7 lines

I am not sure if the interaction between user-path and vhost should go like this, but I do not think of a good alternative to suggest right now. Your code allows ~user/host1 and ~user/host2 to host different set of repositories, but I suspect if somebody is setting up a virtual hosting of two hosts, he might want to have two distinct set of users (iow to pretend that ~user exist only on host1 but not on host2). I dunno.

Another option would be not to support virtual hosts, but instead superseed the --base-path and --user-path with some --base-path-fmt and --user-path-fmt where the user can specify how to build the path with simple sprintf-like formats. One could e.g. support:
* %% obviously ;
* %h that will be replaced with the hostname
* %u (only for --user-path-fmt)
* %p (asked path)
* ...

I think that's more clever, and allow more flexible use of the virtual hosting code. It e.g. allow to use the virtual host scheme for the base-path repos and to disallow it for the users.

--*-path and --*-path-fmt are obviously mutually exclusive.

What do you think ?

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org
#14 lore raw
Quoted 9 lines · Pierre Habouzit

Another option would be not to support virtual hosts, but instead superseed the --base-path and --user-path with some --base-path-fmt and --user-path-fmt where the user can specify how to build the path with simple sprintf-like formats. One could e.g. support:
* %% obviously ;
* %h that will be replaced with the hostname
* %u (only for --user-path-fmt)
* %p (asked path)
* ...

And this is exactly what I have implemented and
running on my system today!

Quoted 7 lines

I think that's more clever, and allow more flexible use of the virtual hosting code. It e.g. allow to use the virtual host scheme for the base-path repos and to disallow it for the users.

--*-path and --*-path-fmt are obviously mutually exclusive.

What do you think ?

I kinda like it... :-)

jdl

#15 lore raw
Quoted 12 lines · Jon Loeliger
Quoted 9 lines · Pierre Habouzit

Another option would be not to support virtual hosts, but instead superseed the --base-path and --user-path with some --base-path-fmt and --user-path-fmt where the user can specify how to build the path with simple sprintf-like formats. One could e.g. support:
* %% obviously ;
* %h that will be replaced with the hostname
* %u (only for --user-path-fmt)
* %p (asked path)
* ...

Quoted 2 lines

And this is exactly what I have implemented and
running on my system today!

Where is the patch ;-)?

#16 lore raw
Quoted 1 line · Junio C Hamano

Where is the patch ;-)?

Heh.

Well, I rewrote the interpolation code. As you pointed out earlier, it was crap. I can send that in; no problem.

There was some outstanding debate if this was actually the right way to go about things. Specifically, the problem of canonical hostname. A proposal was to use peer IP addresses instead, and I just haven't gotten around to messing with that yet. So that is why I hadn't sent it in yet. Sorry.

But, I'll rebase it and send it in this evening.

jdl