Re: git-daemon: path validation, export all option

From: Anton Altaparmakov <aia21@cam.ac.uk>
Date: Sep 27, 2005, 08:30 UTC
Message-ID: <1127809831.28407.6.camel@imp.csi.cam.ac.uk>
In-Reply-To: <7vslvr6t1u.fsf@assigned-by-dhcp.cox.net>

On Mon, 2005-09-26 at 21:19 -0700, Junio C Hamano wrote:

Show 28 quoted lines

> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> > A first attempt to make git-daemon a bit more suitable for kernel.org 
> > use: it allows the user to specify a whitelist of directories, rejects 
> > paths which have . or .. in them (to avoid bypassing the whitelist), and 
> > allows for an --export-all option.
> >
> > Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> 
> I understand the motivation behind --export-all and directory
> whitelist and these changes look good.  Thanks.
> 
> > +	if ( ok_paths && *ok_paths ) {
> > +		int ok = 0;
> > +...
> > +	}
> > +
> > +	return 1;		/* Path acceptable */
> > +}
> 
> A microNit.  You could lose 'int ok' and return 1 directly where
> you assign 1 to it and break.
> 
> > -	chdir(".git");
> 
> I am unsure about this removal of "minor convenience feature".
> Although I do not think git-daemon is widely used on the field,
> this change breaks existing setup if there is any.

Please drop this one line change. It certainly breaks my personal setup. And all git tools are happy with being given the "master" directory or the "master/.git" so there is no reason for git-daemon not to accept that, too.

If hpa really can't live with the chdir, maybe we could add a "--strict-git-paths" option or something that will not do the chdir? It would be only a few lines of code in git-daemon to parse the option and then the chdir would become

if (!strict_git_paths)
	chdir(".git");

Best regards,

        Anton

-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/