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

[Pierre Habouzit <madcoder@debian.org>]

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

Le mer 23 août 2006 22:11, Junio C Hamano a écrit :
> 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.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]