full featured formating function of the --{base,user}_path arguments,

daemon.c +4 -0

+static struct {
+	const char *path;
+	int use_as_fmt;
+} base_path;

daemon.c +0 -0

@@ -174,24 +285,45 @@ static char *path_ok(char *dir)
 				slash = dir + restlen;
 			namlen = slash - dir;
 			restlen -= namlen;

daemon.c +7 -0

@@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr

@@ -303,15 +436,30 @@ #endif
 	alarm(0);
 
 	len = strlen(line);
+
+	if (pktlen != len) {
+		int arg_pos = len + 1;
+
 		loginfo("Extended attributes (%d bytes) exist <%.*s>",
 			(int) pktlen - len,
+			(int) pktlen - len, line + arg_pos);
+
+		while (arg_pos < pktlen) {

Quoted 9 lines · Pierre Habouzit

daemon.c +4 -0

+static struct {
+	const char *path;
+	int use_as_fmt;
+} base_path;
 
 /* If defined, ~user notation is allowed and the string is

inserted * after ~user/. E.g. a request to git://host/~alice/frotz
would * go to /home/alice/pub_git/frotz with --user-path=pub_git. */

daemon.c static int strict_paths; · line 46

Quoted 5 lines

daemon.c +4 -1

-static const char *user_path;
+static struct {
+	const char *path;
+	int use_as_fmt;
+} user_path;

Maybe it does not matter much, but I wonder if we want to keep two structs the same type, like:

        static struct {
                const char *path;
                int use_as_fmt;
        } base_path, user_path;

I also wondered if we can just extend the semantics of base_path and user_path to autodetect the fmt-ness of them, but that means we would break existing setups that uses per-cent in the pathname. Arguably that would not be so common and we may not need to worry about such an installation, though. What do you think?

Quoted 12 lines

daemon.c +7 -0

@@ -174,24 +285,45 @@ static char *path_ok(char *dir)
 				slash = dir + restlen;
 			namlen = slash - dir;
 			restlen -= namlen;
+
+			if (user_path.use_as_fmt) {
+				loginfo("host <%s>, "
+					"userpathfmt <%s>, request <%s>, "
+					"namlen %d, restlen %d, slash <%s>",
+					vhost,
+					user_path.path, dir,

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

Quoted 3 lines

daemon.c +3 -0

+					namlen, restlen, slash);
+				dir = git_path_fmt(rpath, user_path.path, vhost,
+						   slash, dir + 1, namlen - 1);

When vhost is NULL you would feed it to "%s", which I think glibc works around with (null) fine but other C libraries would not like it. git_path_fmt()'s logging does not have this problem, though.

Quoted 8 lines

daemon.c +3 -0

+	else if (base_path.path) {
 		if (*dir != '/') {
 			/* Allow only absolute */
 			logerror("'%s': Non-absolute path denied (base-path active)",
 dir); return NULL;
 		}
+
+		if (base_path.use_as_fmt) {

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

Quoted 3 lines

daemon.c +3 -0

+			dir = git_base_path_fmt(rpath, base_path.path, vhost, dir);
+		} else {
+			snprintf(rpath, PATH_MAX, "%s%s", base_path.path, dir);

The level of logging in this branch and in user_path.use_as_fmt branch are inconsistent. Maybe the more detailed one above I commented about vhost==NULL case was primarily meant for debugging and you forgot to remove it?

Quoted 14 lines

daemon.c +7 -0

@@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr

@@ -303,15 +436,30 @@ #endif
 	alarm(0);
 
 	len = strlen(line);
+
+	if (pktlen != len) {
+		int arg_pos = len + 1;
+
 		loginfo("Extended attributes (%d bytes) exist <%.*s>",
 			(int) pktlen - len,
+			(int) pktlen - len, line + arg_pos);
+
+		while (arg_pos < pktlen) {

daemon.c #endif · line 457

Quoted 10 lines

daemon.c +10 -0

+			int arg_len = strlen(line + arg_pos);
+
+			if (!strncmp("host=", line + arg_pos, 5)) {
+				vhost = line + arg_pos + 5;
+			}
+
+			arg_pos += arg_len + 1;
+		}
+	}
+

I think it is easier to do:

	if (!vhost)
        	vhost = default_host;

and have git_base_path_fmt() barf if the format calls for %h and vhost passed to it is NULL. Lack of "host=" in the request is logged here already.