Re: [PATCH] git-daemon virtual hosting implementation.

From: Pierre Habouzit <madcoder@debian.org>
Date: Aug 23, 2006, 20:56 UTC
Message-ID: <200608232256.10108.madcoder@debian.org>
In-Reply-To: <7vmz9vgqlm.fsf@assigned-by-dhcp.cox.net>

Le mer 23 août 2006 22:11, Junio C Hamano a écrit :

Show 24 quoted lines

> Pierre Habouzit <madcoder@debian.org> writes:
> > just add the hostname in the path when using --base-path and
> > --user-path. this should be enough for most needs.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >  Here is a proposal for daemon side virtualhosting support.
> >
> > @@ -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.

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.

Show 18 quoted lines

> > +			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.

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 ?

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org