{"thread":{"id":"5406","subject":"[PATCH] full featured formating function of the --{base,user}_path arguments,","startedAt":"2006-08-27T11:39:18Z","lastAt":"2006-08-28T06:37:42Z","messageCount":3,"participants":["Pierre Habouzit","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25982","messageId":"11566787581030-git-send-email-madcoder@debian.org","threadId":"5406","inReplyTo":null,"subject":"[PATCH] full featured formating function of the --{base,user}_path arguments,","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2006-08-27T11:39:18Z","receivedAt":"2006-08-27T11:39:18Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n Documentation/git-daemon.txt |   54 +++++++++++\n daemon.c                     |  207 ++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 240 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 0f7d274..796ae7e 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -11,6 +11,8 @@ SYNOPSIS\n 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n              [--timeout=n] [--init-timeout=n] [--strict-paths]\n              [--base-path=path] [--user-path | --user-path=path]\n+             [--base-path-fmt=pathfmt] [--user-path-fmt=pathfmt]\n+             [--default-hostname=hostname]\n \t     [--reuseaddr] [--detach] [--pid-file=file] [directory...]\n \n DESCRIPTION\n@@ -45,6 +47,10 @@ OPTIONS\n \t'git://example.com/hello.git', `git-daemon` will interpret the path\n \tas '/srv/git/hello.git'.\n \n+--base-path-fmt=pathfmt::\n+\tWorks like --base-path, but uses a format instead. See Path\n+\tFormats section below.\n+\n --export-all::\n \tAllow pulling from all directories that look like GIT repositories\n \t(have the 'objects' and 'refs' subdirectories), even if they\n@@ -79,9 +85,20 @@ OPTIONS\n \ttaken as a request to access `path/foo` repository in\n \tthe home directory of user `alice`.\n \n+--user-path-fmt=pathfmt::\n+\tAllow ~user notation to be used in requests. The path used to look\n+\tfor the git repository is a format string, see Path Formats\n+\tsection below.\n+\n --verbose::\n \tLog details about the incoming connections and requested files.\n \n+--default-hostname=hostname::\n+\tBackward compatibility switch: argument is used as a fallback\n+\thostname to use with virtual hosting (to fill the `%h` format in\n+\tpath formats) when the client is too old to provide the information\n+\tin the query (older than 1.4.0 series).\n+\n --reuseaddr::\n \tUse SO_REUSEADDR when binding the listening socket.\n \tThis allows the server to restart without waiting for\n@@ -98,10 +115,45 @@ OPTIONS\n \t--strict-paths is specified this will also include subdirectories\n \tof each named directory.\n \n+Path Formats\n+------------\n+\n+%h::\n+\trequested hostname. That won't work with clients older than 1.4.0\n+\tthat do not pass the hostname in their query, and in that case, the\n+\tvalue of --default-hostname (if any) is used.\n+\n+%p::\n+\trequested path.\n+\n+%P::\n+\trequested path without the first slash when exists (`/`).\n+\n+%u::\n+\trequested username (only allowed for --user-path-fmt).\n+\n+%%::\n+\tplain `%`.\n+\n+Examples\n+~~~~~~~~\n+\n+* `--base-path-fmt=/srv/git/%P` emulates `--base-path=/srv/git`\n+\n+* `--base-path-fmt=/srv/git/%h%p` will look into\n+  `/srv/git/git.example.com/foo` if the request goes to\n+  git://git.example.com/foo\n+\n+* `--user-path-fmt=/home/%u/public_git/%P` works mostly like\n+  `--user-path=public_git` (the difference is that it assumes that home\n+  directories live in `/home/username` instead of using the real `username`\n+  home).\n+\n Author\n ------\n Written by Linus Torvalds <torvalds@osdl.org>, YOSHIFUJI Hideaki\n-<yoshfuji@linux-ipv6.org> and the git-list <git@vger.kernel.org>\n+<yoshfuji@linux-ipv6.org>, Pierre Habouzit <madcoder@debian.org>,\n+and the git-list <git@vger.kernel.org>\n \n Documentation\n --------------\ndiff --git a/daemon.c b/daemon.c\nindex 012936f..a2fb9b4 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -19,6 +19,8 @@ static const char daemon_usage[] =\n \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n \"           [--timeout=n] [--init-timeout=n] [--strict-paths]\\n\"\n \"           [--base-path=path] [--user-path | --user-path=path]\\n\"\n+\"           [--base-path-fmt=pathfmt] [--user-path-fmt=pathfmt]\\n\"\n+\"           [--default-hostname=hostname]\\n\"\n \"           [--reuseaddr] [--detach] [--pid-file=file] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n@@ -29,18 +31,27 @@ static int strict_paths;\n static int export_all_trees;\n \n /* Take all paths relative to this one if non-NULL */\n-static char *base_path;\n+static struct {\n+\tconst char *path;\n+\tint use_as_fmt;\n+} base_path;\n \n /* If defined, ~user notation is allowed and the string is inserted\n  * after ~user/.  E.g. a request to git://host/~alice/frotz would\n  * go to /home/alice/pub_git/frotz with --user-path=pub_git.\n  */\n-static const char *user_path;\n+static struct {\n+\tconst char *path;\n+\tint use_as_fmt;\n+} user_path;\n \n /* Timeout, and initial timeout */\n static unsigned int timeout;\n static unsigned int init_timeout;\n \n+/* default host used when the client is old and does not provides host= */\n+static char *default_host;\n+\n static void logreport(int priority, const char *err, va_list params)\n {\n \t/* We should do a single write so that it is atomic and output\n@@ -148,7 +159,107 @@ static int avoid_alias(char *p)\n \t}\n }\n \n-static char *path_ok(char *dir)\n+static void check_path_fmt(const char *check, int allow_user)\n+{\n+\tconst char *p = check;\n+\n+\twhile (*p) {\n+\t\tif (*p++ != '%')\n+\t\t\tcontinue;\n+\n+\t\tswitch (*p) {\n+\t\tcase '\\0':\n+\t\t\tdie(\"invalid format: <%s> ends with an unescaped %%\",\n+\t\t\t    check);\n+\n+\t\tcase '%':\n+\t\tcase 'p': case 'P':\n+\t\tcase 'h':\n+\t\t\tbreak;\n+\n+\t\tcase 'u':\n+\t\t\tif (allow_user)\n+\t\t\t\tbreak;\n+\t\t\t/* fallthrough */\n+\n+\t\tdefault:\n+\t\t\tdie(\"invalid format: <%s> uses unknown specifier %%%c\",\n+\t\t\t    check, *p);\n+\t\t}\n+\n+\t\tp++;\n+\t}\n+}\n+\n+static char *\n+git_path_fmt(char rpath[PATH_MAX], const char *fmt,\n+             const char *vhost, const char *path,\n+             const char *username, int namlen)\n+{\n+\tconst char *p = fmt;\n+\tint pos = 0;\n+\n+\twhile (*p && pos < PATH_MAX) {\n+\t\tif (*p != '%') {\n+\t\t\trpath[pos++] = *p++;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tswitch (*++p) {\n+\t\tcase '%':\n+\t\t\trpath[pos++] = *p;\n+\t\t\tbreak;\n+\n+\t\tcase 'h':\n+\t\t\tif (!vhost && !default_host) {\n+\t\t\t\tlogerror(\"abort: missing host=, no --default-host given, client is too old\");\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t\t\n+\t\t\tif (!vhost) {\n+\t\t\t\tlogerror(\"old client without host=, using <%s>\", default_host);\n+\t\t\t\tpos += strlcpy(rpath + pos, default_host, PATH_MAX - pos);\n+\t\t\t} else {\n+\t\t\t\tpos += strlcpy(rpath + pos, vhost, PATH_MAX - pos);\n+\t\t\t}\n+\t\t\tbreak;\n+\n+\t\tcase 'P':\n+\t\t\tpos += strlcpy(rpath + pos, path + (*path == '/'),\n+\t\t\t\t       PATH_MAX - pos);\n+\t\t\tbreak;\n+\n+\t\tcase 'p':\n+\t\t\tpos += strlcpy(rpath + pos, path, PATH_MAX - pos);\n+\t\t\tbreak;\n+\n+\t\tcase 'u':\n+\t\t\tpos += snprintf(rpath + pos, PATH_MAX - pos, \"%.*s\",\n+\t\t\t\t\tnamlen, username);\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tp++;\n+\t}\n+\n+\tif (pos >= PATH_MAX) {\n+\t\tlogerror(\"generated path is too long\");\n+\t\treturn NULL;\n+\t}\n+\n+\trpath[pos] = '\\0';\n+\n+\treturn rpath;\n+}\n+\n+static inline char *\n+git_base_path_fmt(char rpath[PATH_MAX], const char *fmt,\n+                  const char *vhost, const char *path)\n+{\n+\treturn git_path_fmt(rpath, fmt, vhost, path, NULL, 0);\n+}\n+\n+static char *path_ok(char *dir, char *vhost)\n {\n \tstatic char rpath[PATH_MAX];\n \tchar *path;\n@@ -159,11 +270,11 @@ static char *path_ok(char *dir)\n \t}\n \n \tif (*dir == '~') {\n-\t\tif (!user_path) {\n+\t\tif (!user_path.path) {\n \t\t\tlogerror(\"'%s': User-path not allowed\", dir);\n \t\t\treturn NULL;\n \t\t}\n-\t\tif (*user_path) {\n+\t\tif (*user_path.path) {\n \t\t\t/* Got either \"~alice\" or \"~alice/foo\";\n \t\t\t * rewrite them to \"~alice/%s\" or\n \t\t\t * \"~alice/%s/foo\".\n@@ -174,24 +285,45 @@ static char *path_ok(char *dir)\n \t\t\t\tslash = dir + restlen;\n \t\t\tnamlen = slash - dir;\n \t\t\trestlen -= namlen;\n-\t\t\tloginfo(\"userpath <%s>, request <%s>, namlen %d, restlen %d, slash <%s>\", user_path, dir, namlen, restlen, slash);\n-\t\t\tsnprintf(rpath, PATH_MAX, \"%.*s/%s%.*s\",\n-\t\t\t\t namlen, dir, user_path, restlen, slash);\n-\t\t\tdir = rpath;\n+\n+\t\t\tif (user_path.use_as_fmt) {\n+\t\t\t\tloginfo(\"host <%s>, \"\n+\t\t\t\t\t\"userpathfmt <%s>, request <%s>, \"\n+\t\t\t\t\t\"namlen %d, restlen %d, slash <%s>\",\n+\t\t\t\t\tvhost,\n+\t\t\t\t\tuser_path.path, dir,\n+\t\t\t\t\tnamlen, restlen, slash);\n+\t\t\t\tdir = git_path_fmt(rpath, user_path.path, vhost,\n+\t\t\t\t\t\t   slash, dir + 1, namlen - 1);\n+\t\t\t} else {\n+\t\t\t\tloginfo(\"userpath <%s>, request <%s>, \"\n+\t\t\t\t\t\"namlen %d, restlen %d, slash <%s>\",\n+\t\t\t\t\tuser_path.path, dir,\n+\t\t\t\t\tnamlen, restlen, slash);\n+\t\t\t\tsnprintf(rpath, PATH_MAX, \"%.*s/%s%.*s\",\n+\t\t\t\t\t namlen, dir, user_path.path, restlen, slash);\n+\t\t\t\tdir = rpath;\n+\t\t\t}\n \t\t}\n \t}\n-\telse if (base_path) {\n+\telse if (base_path.path) {\n \t\tif (*dir != '/') {\n \t\t\t/* Allow only absolute */\n \t\t\tlogerror(\"'%s': Non-absolute path denied (base-path active)\", dir);\n \t\t\treturn NULL;\n \t\t}\n-\t\telse {\n-\t\t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path, dir);\n+\n+\t\tif (base_path.use_as_fmt) {\n+\t\t\tdir = git_base_path_fmt(rpath, base_path.path, vhost, dir);\n+\t\t} else {\n+\t\t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path.path, dir);\n \t\t\tdir = rpath;\n \t\t}\n \t}\n \n+\tif (!dir)\n+\t\treturn NULL;\n+\n \tpath = enter_repo(dir, strict_paths);\n \n \tif (!path) {\n@@ -229,7 +361,7 @@ static char *path_ok(char *dir)\n \treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n-static int upload(char *dir)\n+static int upload(char *dir, char *vhost)\n {\n \t/* Timeout as string */\n \tchar timeout_buf[64];\n@@ -237,7 +369,7 @@ static int upload(char *dir)\n \n \tloginfo(\"Request for '%s'\", dir);\n \n-\tif (!(path = path_ok(dir)))\n+\tif (!(path = path_ok(dir, vhost)))\n \t\treturn -1;\n \n \t/*\n@@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr\n {\n \tstatic char line[1000];\n \tint pktlen, len;\n+\tchar *vhost = NULL;\n \n \tif (addr) {\n \t\tchar addrbuf[256] = \"\";\n@@ -303,15 +436,30 @@ #endif\n \talarm(0);\n \n \tlen = strlen(line);\n-\tif (pktlen != len)\n+\n+\tif (pktlen != len) {\n+\t\tint arg_pos = len + 1;\n+\n \t\tloginfo(\"Extended attributes (%d bytes) exist <%.*s>\",\n \t\t\t(int) pktlen - len,\n-\t\t\t(int) pktlen - len, line + len + 1);\n+\t\t\t(int) pktlen - len, line + arg_pos);\n+\n+\t\twhile (arg_pos < pktlen) {\n+\t\t\tint arg_len = strlen(line + arg_pos);\n+\n+\t\t\tif (!strncmp(\"host=\", line + arg_pos, 5)) {\n+\t\t\t\tvhost = line + arg_pos + 5;\n+\t\t\t}\n+\n+\t\t\targ_pos += arg_len + 1;\n+\t\t}\n+\t}\n+\n \tif (len && line[len-1] == '\\n')\n \t\tline[--len] = 0;\n \n \tif (!strncmp(\"git-upload-pack \", line, 16))\n-\t\treturn upload(line+16);\n+\t\treturn upload(line+16, vhost);\n \n \tlogerror(\"Protocol error: '%s'\", line);\n \treturn -1;\n@@ -767,19 +915,38 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(arg, \"--base-path=\", 12)) {\n-\t\t\tbase_path = arg+12;\n+\t\t\tbase_path.path = arg+12;\n+\t\t\tbase_path.use_as_fmt = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--base-path-fmt=\", 16)) {\n+\t\t\tbase_path.path = arg+16;\n+\t\t\tbase_path.use_as_fmt = 1;\n+\t\t\tcheck_path_fmt(base_path.path, 0);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--reuseaddr\")) {\n \t\t\treuseaddr = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--default-hostname=\", 19)) {\n+\t\t\tdefault_host = arg + 19;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--user-path\")) {\n-\t\t\tuser_path = \"\";\n+\t\t\tuser_path.path = \"\";\n+\t\t\tuser_path.use_as_fmt = 0;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(arg, \"--user-path=\", 12)) {\n-\t\t\tuser_path = arg + 12;\n+\t\t\tuser_path.path = arg + 12;\n+\t\t\tuser_path.use_as_fmt = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--user-path-fmt=\", 16)) {\n+\t\t\tuser_path.path = arg + 16;\n+\t\t\tuser_path.use_as_fmt = 1;\n+\t\t\tcheck_path_fmt(user_path.path, 1);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(arg, \"--pid-file=\", 11)) {\n-- \n1.4.1.1\n"},{"id":"26030","messageId":"7vd5altob5.fsf@assigned-by-dhcp.cox.net","threadId":"5406","inReplyTo":"11566787581030-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] full featured formating function of the --{base,user}_path arguments,","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-28T05:35:42Z","receivedAt":"2006-08-28T05:35:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> +static struct {\n> +\tconst char *path;\n> +\tint use_as_fmt;\n> +} base_path;\n>  \n>  /* If defined, ~user notation is allowed and the string is inserted\n>   * after ~user/.  E.g. a request to git://host/~alice/frotz would\n>   * go to /home/alice/pub_git/frotz with --user-path=pub_git.\n>   */\n> -static const char *user_path;\n> +static struct {\n> +\tconst char *path;\n> +\tint use_as_fmt;\n> +} user_path;\n\nMaybe it does not matter much, but I wonder if we want to keep\ntwo structs the same type, like:\n\n        static struct {\n                const char *path;\n                int use_as_fmt;\n        } base_path, user_path;\n\nI also wondered if we can just extend the semantics of base_path\nand user_path to autodetect the fmt-ness of them, but that means\nwe would break existing setups that uses per-cent in the\npathname.  Arguably that would not be so common and we may not\nneed to worry about such an installation, though.  What do you\nthink?\n\n> @@ -174,24 +285,45 @@ static char *path_ok(char *dir)\n>  \t\t\t\tslash = dir + restlen;\n>  \t\t\tnamlen = slash - dir;\n>  \t\t\trestlen -= namlen;\n> +\n> +\t\t\tif (user_path.use_as_fmt) {\n> +\t\t\t\tloginfo(\"host <%s>, \"\n> +\t\t\t\t\t\"userpathfmt <%s>, request <%s>, \"\n> +\t\t\t\t\t\"namlen %d, restlen %d, slash <%s>\",\n> +\t\t\t\t\tvhost,\n> +\t\t\t\t\tuser_path.path, dir,\n> +\t\t\t\t\tnamlen, restlen, slash);\n> +\t\t\t\tdir = git_path_fmt(rpath, user_path.path, vhost,\n> +\t\t\t\t\t\t   slash, dir + 1, namlen - 1);\n\nWhen vhost is NULL you would feed it to \"%s\", which I think\nglibc works around with (null) fine but other C libraries would\nnot like it.  git_path_fmt()'s logging does not have this\nproblem, though.\n\n> +\telse if (base_path.path) {\n>  \t\tif (*dir != '/') {\n>  \t\t\t/* Allow only absolute */\n>  \t\t\tlogerror(\"'%s': Non-absolute path denied (base-path active)\", dir);\n>  \t\t\treturn NULL;\n>  \t\t}\n> +\n> +\t\tif (base_path.use_as_fmt) {\n> +\t\t\tdir = git_base_path_fmt(rpath, base_path.path, vhost, dir);\n> +\t\t} else {\n> +\t\t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path.path, dir);\n\nThe level of logging in this branch and in user_path.use_as_fmt\nbranch are inconsistent.  Maybe the more detailed one above I\ncommented about vhost==NULL case was primarily meant for\ndebugging and you forgot to remove it?\n\n> @@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr\n> @@ -303,15 +436,30 @@ #endif\n>  \talarm(0);\n>  \n>  \tlen = strlen(line);\n> +\n> +\tif (pktlen != len) {\n> +\t\tint arg_pos = len + 1;\n> +\n>  \t\tloginfo(\"Extended attributes (%d bytes) exist <%.*s>\",\n>  \t\t\t(int) pktlen - len,\n> +\t\t\t(int) pktlen - len, line + arg_pos);\n> +\n> +\t\twhile (arg_pos < pktlen) {\n> +\t\t\tint arg_len = strlen(line + arg_pos);\n> +\n> +\t\t\tif (!strncmp(\"host=\", line + arg_pos, 5)) {\n> +\t\t\t\tvhost = line + arg_pos + 5;\n> +\t\t\t}\n> +\n> +\t\t\targ_pos += arg_len + 1;\n> +\t\t}\n> +\t}\n> +\n\nI think it is easier to do:\n\n\tif (!vhost)\n        \tvhost = default_host;\n\nand have git_base_path_fmt() barf if the format calls for %h and\nvhost passed to it is NULL.  Lack of \"host=\" in the request is\nlogged here already.\n"},{"id":"26033","messageId":"200608280837.47870.madcoder@debian.org","threadId":"5406","inReplyTo":"7vd5altob5.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] full featured formating function of the --{base,user}_path arguments,","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2006-08-28T06:37:42Z","receivedAt":"2006-08-28T06:37:42Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Le lun 28 août 2006 07:35, Junio C Hamano a écrit :\n> Pierre Habouzit <madcoder@debian.org> writes:\n> > +static struct {\n> > +\tconst char *path;\n> > +\tint use_as_fmt;\n> > +} base_path;\n> >\n> >  /* If defined, ~user notation is allowed and the string is\n> > inserted * after ~user/.  E.g. a request to git://host/~alice/frotz\n> > would * go to /home/alice/pub_git/frotz with --user-path=pub_git.\n> > */\n> > -static const char *user_path;\n> > +static struct {\n> > +\tconst char *path;\n> > +\tint use_as_fmt;\n> > +} user_path;\n>\n> Maybe it does not matter much, but I wonder if we want to keep\n> two structs the same type, like:\n>\n>         static struct {\n>                 const char *path;\n>                 int use_as_fmt;\n>         } base_path, user_path;\n>\n> I also wondered if we can just extend the semantics of base_path\n> and user_path to autodetect the fmt-ness of them, but that means\n> we would break existing setups that uses per-cent in the\n> pathname.  Arguably that would not be so common and we may not\n> need to worry about such an installation, though.  What do you\n> think?\n\nI think that merging the structs is indeed good, but that I do not like \nto break (even non common) setups when avoidable. but well, I've no \n*very* strong opinion on this, and if the consensus is that extending \nthe argument with a format is good enough, I'll go for it, it would \nsimplify the code a bit, and remove two arguments, both are good.\n\n>\n> > @@ -174,24 +285,45 @@ static char *path_ok(char *dir)\n> >  \t\t\t\tslash = dir + restlen;\n> >  \t\t\tnamlen = slash - dir;\n> >  \t\t\trestlen -= namlen;\n> > +\n> > +\t\t\tif (user_path.use_as_fmt) {\n> > +\t\t\t\tloginfo(\"host <%s>, \"\n> > +\t\t\t\t\t\"userpathfmt <%s>, request <%s>, \"\n> > +\t\t\t\t\t\"namlen %d, restlen %d, slash <%s>\",\n> > +\t\t\t\t\tvhost,\n> > +\t\t\t\t\tuser_path.path, dir,\n> > +\t\t\t\t\tnamlen, restlen, slash);\n> > +\t\t\t\tdir = git_path_fmt(rpath, user_path.path, vhost,\n> > +\t\t\t\t\t\t   slash, dir + 1, namlen - 1);\n>\n> When vhost is NULL you would feed it to \"%s\", which I think\n> glibc works around with (null) fine but other C libraries would\n> not like it.  git_path_fmt()'s logging does not have this\n> problem, though.\n\nuuhhh, I've always thought that passing NULL to a %s format was allowed \nby the C standard ... I've no copy here but one at work, I will verify \nit (for my peace of mind) but I agree it's not pretty.\n\n> > +\telse if (base_path.path) {\n> >  \t\tif (*dir != '/') {\n> >  \t\t\t/* Allow only absolute */\n> >  \t\t\tlogerror(\"'%s': Non-absolute path denied (base-path active)\",\n> > dir); return NULL;\n> >  \t\t}\n> > +\n> > +\t\tif (base_path.use_as_fmt) {\n> > +\t\t\tdir = git_base_path_fmt(rpath, base_path.path, vhost, dir);\n> > +\t\t} else {\n> > +\t\t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path.path, dir);\n>\n> The level of logging in this branch and in user_path.use_as_fmt\n> branch are inconsistent.  Maybe the more detailed one above I\n> commented about vhost==NULL case was primarily meant for\n> debugging and you forgot to remove it?\n\nwell, it was here before, so I left it, but I do not liked it much \neither, so if it's ok to remove it, I'd be glad to.\n\n> > @@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr\n> > @@ -303,15 +436,30 @@ #endif\n> >  \talarm(0);\n> >\n> >  \tlen = strlen(line);\n> > +\n> > +\tif (pktlen != len) {\n> > +\t\tint arg_pos = len + 1;\n> > +\n> >  \t\tloginfo(\"Extended attributes (%d bytes) exist <%.*s>\",\n> >  \t\t\t(int) pktlen - len,\n> > +\t\t\t(int) pktlen - len, line + arg_pos);\n> > +\n> > +\t\twhile (arg_pos < pktlen) {\n> > +\t\t\tint arg_len = strlen(line + arg_pos);\n> > +\n> > +\t\t\tif (!strncmp(\"host=\", line + arg_pos, 5)) {\n> > +\t\t\t\tvhost = line + arg_pos + 5;\n> > +\t\t\t}\n> > +\n> > +\t\t\targ_pos += arg_len + 1;\n> > +\t\t}\n> > +\t}\n> > +\n>\n> I think it is easier to do:\n>\n> \tif (!vhost)\n>         \tvhost = default_host;\n>\n> and have git_base_path_fmt() barf if the format calls for %h and\n> vhost passed to it is NULL.  Lack of \"host=\" in the request is\n> logged here already.\n\nhmmm, lack of host= is only logged in the git_path_fmt function atm. Its \npresence is logged though. But I could obviously log that it's missing \nwhile parsing the extending attributes, and then only need to barf when \nthe %h is actually needed in the format string. That seems indeed \neasier. Will do.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"}]}