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

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:07

"H. Peter Anvin" [off-list ref] 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.

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

From: Anton Altaparmakov <hidden>
Date: 2016-06-15 22:42:07

On Mon, 2005-09-26 at 21:19 -0700, Junio C Hamano wrote:
"H. Peter Anvin" [off-list ref] writes:
quoted
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.
quoted
+	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.
quoted
-	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/

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

From: "H. Peter Anvin" <hpa@zytor.com>
Date: 2016-06-15 22:42:07

Junio C Hamano wrote:
A microNit.  You could lose 'int ok' and return 1 directly where
you assign 1 to it and break.
I guess I personally prefer the coding style where the straigh-line flow 
of control is the normal one.  It prevents the "oops" of someone wanting 
to add code to it later.
quoted
-	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.
I have restored this and make the requested RPM changes.  I have left a 
pullable tree at:

master.kernel.org:/home/hpa/git/daemon.git

... in order to preserve the commit structure.

	-hpa
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help