{"thread":{"id":"4399","subject":"[RFC] Add first whack at interpolated daemon paths.","startedAt":"2006-06-05T03:54:59Z","lastAt":"2006-06-05T06:20:13Z","messageCount":2,"participants":["Jon Loeliger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"21252","messageId":"E1Fn6B9-00017u-BV@jdl.com","threadId":"4399","inReplyTo":null,"subject":"[RFC] Add first whack at interpolated daemon paths.","fromName":"Jon Loeliger","fromEmail":"jdl@jdl.com","sentAt":"2006-06-05T03:54:59Z","receivedAt":"2006-06-05T03:54:59Z","isPatch":false,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"\nModify git protocol to pass in client hostname and\nallow it to be interpolated into daemon source dir.\nNew --interpolated-path=<path> option.\n---\n Makefile      |    1 +\n connect.c     |   16 +++++++-\n daemon.c      |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++-------\n interpolate.c |   74 +++++++++++++++++++++++++++++++++++++\n interpolate.h |   10 +++++\n 5 files changed, 198 insertions(+), 16 deletions(-)\n\nThis is really RFC-ish.  No canonicalization of hostname is done.\nIt is backwards compatible with existing daemon path handling,\nbut also allows for future extensibility with more \"extended\"\nclient args being supplied.  The interpolate code is pretty\ngeneric, but the table driving it in this case is hard coded.\nTons of loginfo() crap left in here to be cleaned up.\n\n\ndiff --git a/Makefile b/Makefile\nindex 004c216..6a02236 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -211,6 +211,7 @@ DIFF_OBJS = \\\n LIB_OBJS = \\\n \tblob.o commit.o connect.o csum-file.o cache-tree.o base85.o \\\n \tdate.o diff-delta.o entry.o exec_cmd.o ident.o index.o \\\n+\tinterpolate.o \\\n \tobject.o pack-check.o patch-delta.o path.o pkt-line.o \\\n \tquote.o read-cache.o refs.o run-command.o dir.o \\\n \tserver-info.o setup.o sha1_file.o sha1_name.o strbuf.o \\\ndiff --git a/connect.c b/connect.c\nindex 54f7bf7..9e7b276 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -374,7 +374,13 @@ static int git_tcp_connect(int fd[2], co\n \n \tfd[0] = sockfd;\n \tfd[1] = sockfd;\n-\tpacket_write(sockfd, \"%s %s\\n\", prog, path);\n+\n+\t/*\n+\t * Separate original protocol components prog and path\n+\t * from extended components with a NUL byte.\n+\t */\n+\tpacket_write(sockfd, \"%s %s%cHOST=%s\\n\", prog, path, 0, host);\n+\n \treturn 0;\n }\n \n@@ -443,7 +449,13 @@ static int git_tcp_connect(int fd[2], co\n \n \tfd[0] = sockfd;\n \tfd[1] = sockfd;\n-\tpacket_write(sockfd, \"%s %s\\n\", prog, path);\n+\n+\t/*\n+\t * Separate original protocol components prog and path\n+\t * from extended components with a NUL byte.\n+\t */\n+\tpacket_write(sockfd, \"%s %s%cHOST=%s\\n\", prog, path, 0, host);\n+\n \treturn 0;\n }\n \ndiff --git a/daemon.c b/daemon.c\nindex 776749e..0c9ebe3 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -10,6 +10,7 @@ #include <syslog.h>\n #include \"pkt-line.h\"\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n+#include \"interpolate.h\"\n \n static int log_syslog;\n static int verbose;\n@@ -18,7 +19,8 @@ static int reuseaddr;\n 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=path] [--interpolated-path=path]\\n\"\n+\"           [--user-path | --user-path=path]\\n\"\n \"           [--reuseaddr] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n@@ -28,8 +30,15 @@ static int strict_paths = 0;\n /* If this is set, git-daemon-export-ok is not required */\n static int export_all_trees = 0;\n \n-/* Take all paths relative to this one if non-NULL */\n+/*\n+ * Take all paths relative to this one if non-NULL.\n+ * \n+ */\n static char *base_path = NULL;\n+static char *interpolated_path = NULL;\n+\n+/* Flag indicating client sent extra args. */\n+int saw_extended_args = 0;\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@@ -41,6 +50,23 @@ static char *user_path = NULL;\n static unsigned int timeout = 0;\n static unsigned int init_timeout = 0;\n \n+/*\n+ * Static table for now.  Ugh.\n+ * Feel free to make dynamic as needed.\n+ */\n+#define INTERP_SLOT_HOST\t(0)\n+#define INTERP_SLOT_DIR\t\t(1)\n+#define INTERP_SLOT_PERCENT\t(2)\n+\n+struct interp interp_table[] = {\n+\t{ \"%H\", 0},\n+\t{ \"%D\", 0},\n+\t{ \"%%\", \"%\"},\n+};\n+\n+#define N_INTERPS\t(sizeof(interp_table) / sizeof(struct interp))\n+\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@@ -142,10 +168,15 @@ static int avoid_alias(char *p)\n \t}\n }\n \n-static char *path_ok(char *dir)\n+static char *path_ok(struct interp *itable)\n {\n \tstatic char rpath[PATH_MAX];\n+\tstatic char interp_path[PATH_MAX];\n \tchar *path;\n+\tchar *dir;\n+\n+\tdir = itable[INTERP_SLOT_DIR].value;\n+\tloginfo(\"Request for '%s'\", dir);\n \n \tif (avoid_alias(dir)) {\n \t\tlogerror(\"'%s': aliased\", dir);\n@@ -174,16 +205,34 @@ static char *path_ok(char *dir)\n \t\t\tdir = rpath;\n \t\t}\n \t}\n+\telse if (interpolated_path && saw_extended_args) {\n+\t\tif (*dir != '/') {\n+\t\t\t/* Allow only absolute */\n+\t\t\tlogerror(\"'%s': Non-absolute path denied (interpolated-path active)\", dir);\n+\t\t\treturn NULL;\n+\t\t}\n+\n+\t\tloginfo(\"Before interpolation '%s'\", dir);\n+\t\tloginfo(\"Interp slot 0 (%s,%s)\",\n+\t\t\tinterp_table[0].name, interp_table[0].value);\n+\t\tloginfo(\"Interp slot 1 (%s,%s)\",\n+\t\t\tinterp_table[1].name, interp_table[1].value);\n+\t\tinterpolate(interp_path, PATH_MAX, interpolated_path,\n+\t\t\t    interp_table, N_INTERPS);\n+\t\tloginfo(\"After interpolation '%s'\", interp_path);\n+\t\tdir = interp_path;\n+\t}\n \telse if (base_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-\t\t\tdir = rpath;\n-\t\t}\n+\t\tsnprintf(rpath, PATH_MAX, \"%s%s\", base_path, dir);\n+\t\tloginfo(\"dir was %s\", dir);\n+\t\tloginfo(\"base_path is %s\", base_path);\n+\t\tloginfo(\"rpath now %s\", rpath);\n+\t\tdir = rpath;\n \t}\n \n \tpath = enter_repo(dir, strict_paths);\n@@ -223,15 +272,13 @@ 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(struct interp *itable)\n {\n \t/* Timeout as string */\n \tchar timeout_buf[64];\n \tconst char *path;\n \n-\tloginfo(\"Request for '%s'\", dir);\n-\n-\tif (!(path = path_ok(dir)))\n+\tif (!(path = path_ok(itable)))\n \t\treturn -1;\n \n \t/*\n@@ -264,10 +311,34 @@ static int upload(char *dir)\n \treturn -1;\n }\n \n+void parse_extra_args(char *extra_args, int buflen)\n+{\n+\tchar *val;\n+\tint vallen;\n+\tchar *end = extra_args + buflen;\n+\n+\twhile (extra_args < end && *extra_args) {\n+\t\tsaw_extended_args = 1;\n+\t\tloginfo(\"Extended arg %s\", extra_args);\n+\t\tif (strncasecmp(\"host=\", extra_args, 5) == 0) {\n+\t\t\tval = extra_args + 5;\n+\t\t\tvallen = strlen(val) + 1;\n+\t\t\tif (*val) {\n+\t\t\t\tchar *save = xmalloc(vallen);\n+\t\t\t\tinterp_table[INTERP_SLOT_HOST].value = save;\n+\t\t\t\tsafe_strncpy(save, val, vallen);\n+\t\t\t}\n+\t\t\t/* On to the next one */\n+\t\t\textra_args = val + vallen;\n+\t\t}\n+\t}\n+}\n+\n static int execute(void)\n {\n \tstatic char line[1000];\n-\tint len;\n+\tint len;\t/* full packet length, including extended args */\n+\tint n;\t\t/* original protocol part size */\n \n \talarm(init_timeout ? init_timeout : timeout);\n \tlen = packet_read_line(0, line, sizeof(line));\n@@ -276,8 +347,18 @@ static int execute(void)\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/*\n+\t * Check for extended args after a NUL byte.\n+\t */\n+\tn = strlen(line);\n+\tif (n != len) {\n+\t    parse_extra_args(line + n + 1, len - n - 1);\n+\t}\n+\n+\tif (!strncmp(\"git-upload-pack \", line, 16)) {\n+\t\tinterp_table[INTERP_SLOT_DIR].value = line+16;\n+\t\treturn upload(interp_table);\n+\t}\n \n \tlogerror(\"Protocol error: '%s'\", line);\n \treturn -1;\n@@ -711,6 +792,10 @@ int main(int argc, char **argv)\n \t\t\tbase_path = arg+12;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--interpolated-path=\", 20)) {\n+\t\t\tinterpolated_path = arg+20;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--reuseaddr\")) {\n \t\t\treuseaddr = 1;\n \t\t\tcontinue;\ndiff --git a/interpolate.c b/interpolate.c\nnew file mode 100644\nindex 0000000..d936022\n--- /dev/null\n+++ b/interpolate.c\n@@ -0,0 +1,74 @@\n+#include <string.h>\n+\n+#include \"interpolate.h\"\n+\n+\n+int\n+interpolate(char *result, int reslen, char *orig,\n+\t    struct interp *interps, int ninterps)\n+{\n+    int i;\n+    char *p;\n+    char *src = orig;\n+    char *dest = result;\n+    int newlen = 0;\n+\n+    char *name;\n+    char *value;\n+    int valuelen;\n+\n+    do {\n+\n+\tp = strchr(src, '%');\n+\t\n+\tif (p) {\n+\t    /*\n+\t     * Found a potential interpolation point.\n+\t     */\n+\t    for (i = 0; i < ninterps; i++) {\n+\t\tname = interps[i].name;\n+\t\tif (strncmp(p, name, strlen(name)) == 0)\n+\t\t    break;\n+\t    }\n+\n+\t    value = interps[i].value;\n+\t    valuelen = strlen(value);\n+\t    printf(\"Interp: %s to %s\\n\", name, value);\n+\n+\t    int len = p - src;\n+\t    if (newlen + len < reslen) {\n+\t\tstrncpy(dest, src, len);\n+\t\tnewlen += len;\n+\t\tdest += len;\n+\t\t*dest = 0;\n+\t\tsrc = p + strlen(name);\n+\t\tif (newlen + valuelen < reslen) {\n+\t\t    strncpy(dest, value, valuelen);\n+\t\t    newlen += valuelen;\n+\t\t    dest += valuelen;\n+\t\t    *dest = 0;\n+\t\t} else {\n+\t\t    printf(\"new value %s didn't fit.\\n\", value);\n+\t\t    return 0;\t/* something's not fitting. */\n+\t\t}\n+\t    } else {\n+\t\tprintf(\"orig part %s didn't fit.\\n\", src);\n+\t\treturn 0;\t/* something's not fitting. */\n+\t    }\n+\n+\t} else {\n+\t    /* Copy remainder */\n+\t    int len = strlen(src);\n+\t    if (newlen < reslen) {\n+\t\tstrncpy(dest, src, len);\n+\t\tdest += len;\n+\t\t*dest = 0;\n+\t    } else {\n+\t\tprintf(\"Remainder %s didn't fit.\\n\", src);\n+\t\treturn 0;\n+\t    }\n+\t}\n+    } while (p);\n+\n+    return 1;\t/* successful */\n+}\ndiff --git a/interpolate.h b/interpolate.h\nnew file mode 100644\nindex 0000000..3b710ad\n--- /dev/null\n+++ b/interpolate.h\n@@ -0,0 +1,10 @@\n+\n+struct interp {\n+\tchar *name;\n+\tchar *value;\n+};\n+\n+\n+extern int interpolate(char *result, int reslen, char *orig,\n+\t\t       struct interp *interps, int ninterps);\n+\n-- \n1.3.3.g16a4-dirty\n"},{"id":"21254","messageId":"7vac8s5dk2.fsf@assigned-by-dhcp.cox.net","threadId":"4399","inReplyTo":"E1Fn6B9-00017u-BV@jdl.com","subject":"Re: [RFC] Add first whack at interpolated daemon paths.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-05T06:20:13Z","receivedAt":"2006-06-05T06:20:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Loeliger <jdl@jdl.com> writes:\n\n> This is really RFC-ish.  No canonicalization of hostname is done.\n\nThat's fine.  RFC-ish is what gets the ball rolling.\n\n> diff --git a/Makefile b/Makefile\n> index 004c216..6a02236 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -211,6 +211,7 @@ DIFF_OBJS = \\\n>  LIB_OBJS = \\\n>  \tblob.o commit.o connect.o csum-file.o cache-tree.o base85.o \\\n>  \tdate.o diff-delta.o entry.o exec_cmd.o ident.o index.o \\\n> +\tinterpolate.o \\\n>  \tobject.o pack-check.o patch-delta.o path.o pkt-line.o \\\n>  \tquote.o read-cache.o refs.o run-command.o dir.o \\\n>  \tserver-info.o setup.o sha1_file.o sha1_name.o strbuf.o \\\n\nA separate helper library is always a good idea.  Is it\n\"interpolate\", I wonder, however?\n\n> diff --git a/connect.c b/connect.c\n> index 54f7bf7..9e7b276 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -374,7 +374,13 @@ static int git_tcp_connect(int fd[2], co\n>  \n>  \tfd[0] = sockfd;\n>  \tfd[1] = sockfd;\n> -\tpacket_write(sockfd, \"%s %s\\n\", prog, path);\n> +\n> +\t/*\n> +\t * Separate original protocol components prog and path\n> +\t * from extended components with a NUL byte.\n> +\t */\n> +\tpacket_write(sockfd, \"%s %s%cHOST=%s\\n\", prog, path, 0, host);\n> +\n\nSince packet interface reader knows the length of the total\nstring, it might make sense to use NUL as the terminator\nbetweeen extended attributes (i.e. end the format with\n\"...HOST=%s%c\" and append 0 as the last argument).  Some of our\nlater extended attributes might want to have LF in them for\nwhatever reason we do not foresee here.\n\n>  \treturn 0;\n>  }\n>  \n> @@ -443,7 +449,13 @@ static int git_tcp_connect(int fd[2], co\n>  \n>  \tfd[0] = sockfd;\n>  \tfd[1] = sockfd;\n> -\tpacket_write(sockfd, \"%s %s\\n\", prog, path);\n> +\n> +\t/*\n> +\t * Separate original protocol components prog and path\n> +\t * from extended components with a NUL byte.\n> +\t */\n> +\tpacket_write(sockfd, \"%s %s%cHOST=%s\\n\", prog, path, 0, host);\n> +\n>  \treturn 0;\n>  }\n\nWe probably would want to share this part between two\ngit_tcp_connect() implementations.  Rename two #ifdef'ed\nimplemenations of git_tcp_connect() to git_tcp_connect_sock(),\nmake its sole purpose to just open the connection and return the\nsockfd.  Write a new git_tcp_connect() which is not #ifdef'ed,\ncall git_tcp_connect_sock() from there, and do the part after\n\"if (sockfd < 0)\" in the new git_tcp_connect().\n\n> +/* Flag indicating client sent extra args. */\n> +int saw_extended_args = 0;\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> @@ -41,6 +50,23 @@ static char *user_path = NULL;\n>  static unsigned int timeout = 0;\n>  static unsigned int init_timeout = 0;\n>  \n> +/*\n> + * Static table for now.  Ugh.\n> + * Feel free to make dynamic as needed.\n> + */\n> +#define INTERP_SLOT_HOST\t(0)\n> +#define INTERP_SLOT_DIR\t\t(1)\n> +#define INTERP_SLOT_PERCENT\t(2)\n> +\n> +struct interp interp_table[] = {\n> +\t{ \"%H\", 0},\n> +\t{ \"%D\", 0},\n> +\t{ \"%%\", \"%\"},\n> +};\n\nAs Linus mentioned, %H for full host and %h for the most\nspecific part may make sense.  I wonder if %D has a practical\nvalue.  It lets you splice the client supplied directory path in\nthe middle of the final string, but would it be useful in\npractice?  Otherwise, maybe dropping %D and always appending at\nthe tail of \"interporated\" path that is tailored for the virtual\nhost might be easier to code and explain.  \n\nAs to naming, \"interp\" sounds as if we are dealing with some\n\"interpreter\" here...\n\n> +#define N_INTERPS\t(sizeof(interp_table) / sizeof(struct interp))\n\nI would use ARRAY_SIZE() from git-compat-util.h where you would\nuse N_INTERPS and drop the #define altogether.\n\n> +\telse if (interpolated_path && saw_extended_args) {\n> +\t\tif (*dir != '/') {\n> +\t\t\t/* Allow only absolute */\n> +\t\t\tlogerror(\"'%s': Non-absolute path denied (interpolated-path active)\", dir);\n> +\t\t\treturn NULL;\n> +\t\t}\n> +\n> +\t\tloginfo(\"Before interpolation '%s'\", dir);\n> +\t\tloginfo(\"Interp slot 0 (%s,%s)\",\n> +\t\t\tinterp_table[0].name, interp_table[0].value);\n> +\t\tloginfo(\"Interp slot 1 (%s,%s)\",\n> +\t\t\tinterp_table[1].name, interp_table[1].value);\n> +\t\tinterpolate(interp_path, PATH_MAX, interpolated_path,\n> +\t\t\t    interp_table, N_INTERPS);\n> +\t\tloginfo(\"After interpolation '%s'\", interp_path);\n> +\t\tdir = interp_path;\n> +\t}\n\nI suspect it would be easier to maintain the site if you let the\nsite administrator the default virtual host name, and for\nrequests from older clients use that name to interpolate %H, and\nalways use this codepath and nothing else (in other words, lose\n\"saw_extended_args\" check) for both old and new clients.\n\nWith your patch, the administrator needs to configure the daemon\nwith --base_path=/mnt1/git.or.cz/git --interpolate=/mnt1/%H/git and\nmaintain them in sync.\n\n> diff --git a/interpolate.c b/interpolate.c\n>...\n> +\tif (p) {\n> +\t    /*\n> +\t     * Found a potential interpolation point.\n> +\t     */\n> +\t    for (i = 0; i < ninterps; i++) {\n> +\t\tname = interps[i].name;\n> +\t\tif (strncmp(p, name, strlen(name)) == 0)\n> +\t\t    break;\n> +\t    }\n> +\n> +\t    value = interps[i].value;\n> +\t    valuelen = strlen(value);\n> +\t    printf(\"Interp: %s to %s\\n\", name, value);\n\nWouldn't a daemon misconfigured with --interpolate=\"/mnt/%X\"\nbarf here by overstepping interps[] array?\n"}]}