{"thread":{"id":"17167","subject":"[PATCH 1/3] git-daemon: single-line logs","startedAt":"2009-01-14T10:48:10Z","lastAt":"2009-01-14T19:25:28Z","messageCount":13,"participants":["Jan Engelhardt","Junio C Hamano","Jeff King","Adeodato Simó","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"100392","messageId":"alpine.LSU.2.00.0901141147120.16109@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":null,"subject":"[PATCH 1/3] git-daemon: single-line logs","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T10:48:10Z","receivedAt":"2009-01-14T10:48:10Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\n\nparent v1.6.1\n\ngit-daemon: single-line logs\n\nHaving just a single line per connection attempt, much like Apache\nhttpd2 access logs, makes log parsing much easier, especially when\njust glancing over it non-automated.\n\nSigned-off-by: Jan Engelhardt \n\n---\n daemon.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\nIndex: git-1.6.1/daemon.c\n===================================================================\n--- git-1.6.1.orig/daemon.c\n+++ git-1.6.1/daemon.c\n@@ -295,12 +295,13 @@ static int git_daemon_config(const char\n \treturn 0;\n }\n \n-static int run_service(char *dir, struct daemon_service *service)\n+static int run_service(char *dir, struct daemon_service *service,\n+ const char *origin, const char *vhost)\n {\n \tconst char *path;\n \tint enabled = service->enabled;\n \n-\tloginfo(\"Request %s for '%s'\", service->name, dir);\n+\tloginfo(\"%s->%s %s \\\"%s\\\"\\n\", origin, vhost, service->name, dir);\n \n \tif (!enabled && !service->overridable) {\n \t\tlogerror(\"'%s': service not enabled.\", service->name);\n@@ -507,10 +508,10 @@ static void parse_extra_args(char *extra\n static int execute(struct sockaddr *addr)\n {\n \tstatic char line[1000];\n+\tchar addrbuf[256] = \"\";\n \tint pktlen, len, i;\n \n \tif (addr) {\n-\t\tchar addrbuf[256] = \"\";\n \t\tint port = -1;\n \n \t\tif (addr->sa_family == AF_INET) {\n@@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr\n \t\t\tport = ntohs(sin6_addr->sin6_port);\n #endif\n \t\t}\n-\t\tloginfo(\"Connection from %s:%d\", addrbuf, port);\n \t\tsetenv(\"REMOTE_ADDR\", addrbuf, 1);\n \t}\n \telse {\n@@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr\n \talarm(0);\n \n \tlen = strlen(line);\n-\tif (pktlen != len)\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 \tif (len && line[len-1] == '\\n') {\n \t\tline[--len] = 0;\n \t\tpktlen--;\n@@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr\n \t\t\t * Note: The directory here is probably context sensitive,\n \t\t\t * and might depend on the actual service being performed.\n \t\t\t */\n-\t\t\treturn run_service(line + namelen + 5, s);\n+\t\t\treturn run_service(line + namelen + 5, s,\n+\t\t\t addrbuf, hostname);\n \t\t}\n \t}\n \n"},{"id":"100393","messageId":"alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141147120.16109@fbirervta.pbzchgretzou.qr","subject":"[PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T10:48:38Z","receivedAt":"2009-01-14T10:48:38Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nparent v1.6.1\n\ngit-daemon: use getnameinfo to resolve hostname\n\nThis is much shorter than inet_ntop'ing, and also translated\nunresolvable addresses into a string.\n\nSigned-off-by: Jan Engelhardt \n\n---\n daemon.c | 26 +++++++-------------------\n 1 file changed, 7 insertions(+), 19 deletions(-)\n\nIndex: git-1.6.1/daemon.c\n===================================================================\n--- git-1.6.1.orig/daemon.c\n+++ git-1.6.1/daemon.c\n@@ -512,25 +512,13 @@ static int execute(struct sockaddr *addr\n \tint pktlen, len, i;\n \n \tif (addr) {\n-\t\tint port = -1;\n-\n-\t\tif (addr->sa_family == AF_INET) {\n-\t\t\tstruct sockaddr_in *sin_addr = (void *) addr;\n-\t\t\tinet_ntop(addr->sa_family, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));\n-\t\t\tport = ntohs(sin_addr->sin_port);\n-#ifndef NO_IPV6\n-\t\t} else if (addr && addr->sa_family == AF_INET6) {\n-\t\t\tstruct sockaddr_in6 *sin6_addr = (void *) addr;\n-\n-\t\t\tchar *buf = addrbuf;\n-\t\t\t*buf++ = '['; *buf = '\\0'; /* stpcpy() is cool */\n-\t\t\tinet_ntop(AF_INET6, &sin6_addr->sin6_addr, buf, sizeof(addrbuf) - 1);\n-\t\t\tstrcat(buf, \"]\");\n-\n-\t\t\tport = ntohs(sin6_addr->sin6_port);\n-#endif\n-\t\t}\n-\t\tsetenv(\"REMOTE_ADDR\", addrbuf, 1);\n+\t\ti = getnameinfo(addr, (addr->sa_family == AF_INET6) ?\n+\t\t sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in),\n+\t\t addrbuf, sizeof(addrbuf), NULL, 0, 0);\n+\t\tif (i == 0)\n+\t\t\tsetenv(\"REMOTE_ADDR\", addrbuf, 1);\n+\t\telse\n+\t\t\tunsetenv(\"REMOTE_ADDR\");\n \t}\n \telse {\n \t\tunsetenv(\"REMOTE_ADDR\");\n"},{"id":"100394","messageId":"alpine.LSU.2.00.0901141148390.16109@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr","subject":"[PATCH 3/3] git-daemon: vhost support","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T10:49:05Z","receivedAt":"2009-01-14T10:49:05Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nparent v1.6.1\n\ngit-daemon: support vhosts\n\nSince git clients usually send the target hostname in the request\nsimilar to the \"Host:\" HTTP header, one can do virtual hosting.\n\nSigned-off-by: Jan Engelhardt \n\n---\n daemon.c | 22 +++++++++++++++++++---\n 1 file changed, 19 insertions(+), 3 deletions(-)\n\nIndex: git-1.6.1/daemon.c\n===================================================================\n--- git-1.6.1.orig/daemon.c\n+++ git-1.6.1/daemon.c\n@@ -2,6 +2,7 @@\n #include \"pkt-line.h\"\n #include \"exec_cmd.h\"\n \n+#include \n #include \n \n #ifndef HOST_NAME_MAX\n@@ -21,7 +22,7 @@ static const char daemon_usage[] =\n \" [--timeout=n] [--init-timeout=n] [--max-connections=n]\\n\"\n \" [--strict-paths] [--base-path=path] [--base-path-relaxed]\\n\"\n \" [--user-path | --user-path=path]\\n\"\n-\" [--interpolated-path=path]\\n\"\n+\" [--interpolated-path=path] [--vhost]\\n\"\n \" [--reuseaddr] [--detach] [--pid-file=file]\\n\"\n \" [--[enable|disable|allow-override|forbid-override]=service]\\n\"\n \" [--inetd | [--listen=host_or_ipaddr] [--port=n]\\n\"\n@@ -36,6 +37,7 @@ static int strict_paths;\n static int export_all_trees;\n \n /* Take all paths relative to this one if non-NULL */\n+static bool enable_vhosting;\n static char *base_path;\n static char *interpolated_path;\n static int base_path_relaxed;\n@@ -309,8 +311,18 @@ static int run_service(char *dir, struct\n \t\treturn -1;\n \t}\n \n-\tif (!(path = path_ok(dir)))\n-\t\treturn -1;\n+\tif (enable_vhosting) {\n+\t\tchar vdir[256];\n+\n+\t\tif (avoid_alias(dir) != 0)\n+\t\t\treturn -1;\n+\t\tsnprintf(vdir, sizeof(vdir), \"/%s%s\", hostname, dir);\n+\t\tif ((path = path_ok(vdir)) == NULL)\n+\t\t\treturn -1;\n+\t} else {\n+\t\tif ((path = path_ok(dir)) == NULL)\n+\t\t\treturn -1;\n+\t}\n \n \t/*\n \t * Security on the cheap.\n@@ -1046,6 +1058,10 @@ int main(int argc, char **argv)\n \t\t\tmake_service_overridable(arg + 18, 0);\n \t\t\tcontinue;\n \t\t}\n+\t\tif (strcmp(arg, \"--vhost\") == 0) {\n+\t\t\tenable_vhosting = true;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n"},{"id":"100398","messageId":"7vy6xe2kbx.fsf@gitster.siamese.dyndns.org","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141147120.16109@fbirervta.pbzchgretzou.qr","subject":"Re: [PATCH 1/3] git-daemon: single-line logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-14T11:33:38Z","receivedAt":"2009-01-14T11:33:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Engelhardt writes:\n\n> parent v1.6.1\n>\n> git-daemon: single-line logs\n\nPlease drop these two needless lines when/if you are submitting patches\nfor inclusion..\n\n> Having just a single line per connection attempt, much like Apache\n> httpd2 access logs, makes log parsing much easier, especially when\n> just glancing over it non-automated.\n\nWhile I like the motivation, and I wish the log were as terse as possible\nfrom the day one, I think changing the output format unconditionally like\nthis patch does is a horrible idea. I'd expect there are many people who\nalready have their infrastructure set up to parse the current output; this\npatch actively breaks things for them, doesn't it?\n\n> @@ -295,12 +295,13 @@ static int git_daemon_config(const char\n> \treturn 0;\n> }\n> \n> -static int run_service(char *dir, struct daemon_service *service)\n> +static int run_service(char *dir, struct daemon_service *service,\n> + const char *origin, const char *vhost)\n> {\n> \tconst char *path;\n> \tint enabled = service->enabled;\n> \n> -\tloginfo(\"Request %s for '%s'\", service->name, dir);\n> +\tloginfo(\"%s->%s %s \\\"%s\\\"\\n\", origin, vhost, service->name, dir);\n\nMental note. You are adding origin and vhost probably because you are\nlosing them from elsewhere..\n\n> @@ -507,10 +508,10 @@ static void parse_extra_args(char *extra\n> static int execute(struct sockaddr *addr)\n> {\n> \tstatic char line[1000];\n> +\tchar addrbuf[256] = \"\";\n> \tint pktlen, len, i;\n> \n> \tif (addr) {\n> -\t\tchar addrbuf[256] = \"\";\n> \t\tint port = -1;\n> \n> \t\tif (addr->sa_family == AF_INET) {\n> @@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr\n> \t\t\tport = ntohs(sin6_addr->sin6_port);\n> #endif\n> \t\t}\n> -\t\tloginfo(\"Connection from %s:%d\", addrbuf, port);\n\nMental note. Port is not logged anymore here.\n\n> @@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr\n> \talarm(0);\n> \n> \tlen = strlen(line);\n> -\tif (pktlen != len)\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\nMental note. XA are not logged here anymore.\n\n> @@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr\n> \t\t\t * Note: The directory here is probably context sensitive,\n> \t\t\t * and might depend on the actual service being performed.\n> \t\t\t */\n> -\t\t\treturn run_service(line + namelen + 5, s);\n> +\t\t\treturn run_service(line + namelen + 5, s,\n> +\t\t\t addrbuf, hostname);\n> \t\t}\n> \t}\n\nSo not just you are changing the format, but you are losing information as\nwell.\n\nBy the way, I think hostname has already been freed and NULLed at this\ncall site. Aren't you getting entries like:\n\n\t192.168.0.1->(null) upload-pack \"/pub/git.git\"\n\nin your log?\n"},{"id":"100399","messageId":"7vsknm2kbs.fsf@gitster.siamese.dyndns.org","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr","subject":"Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-14T11:33:43Z","receivedAt":"2009-01-14T11:33:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Engelhardt writes:\n\n> parent v1.6.1\n>\n> git-daemon: use getnameinfo to resolve hostname\n>\n> This is much shorter than inet_ntop'ing, and also translated\n> unresolvable addresses into a string.\n\ntranslated? (I think you meant \"translates\" but my English is bad, so I\nam double checking).\n\nThis indeed is much nicer, provided if it is available at least as widely\nas inet_ntop() is.\n\nWe seem to ship inet_ntop() in compat/; a few questions.\n\n (1) Do we need similar compat/ function for getnameinfo()? I am guessing\n that most likely places are the ones that need NO_INET_NTOP and\n NO_INET_PTON, and googling seems to indicate old Cygwin and HP-UX\n 11.00 may be among them.\n\n (2) Do we still use inet_ntop() elsewhere, and if not, can we remove the\n compat/ definitions?\n"},{"id":"100400","messageId":"7vmydu2kbj.fsf@gitster.siamese.dyndns.org","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141148390.16109@fbirervta.pbzchgretzou.qr","subject":"Re: [PATCH 3/3] git-daemon: vhost support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-14T11:33:52Z","receivedAt":"2009-01-14T11:33:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Engelhardt writes:\n\n> parent v1.6.1\n>\n> git-daemon: support vhosts\n>\n> Since git clients usually send the target hostname in the request\n> similar to the \"Host:\" HTTP header, one can do virtual hosting.\n\nIsn't this what --interpolated-path option (especially H and CH\ninterpolations) is about?\n"},{"id":"100401","messageId":"20090114122536.GA5939@coredump.intra.peff.net","threadId":"17167","inReplyTo":"alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr","subject":"Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-14T12:25:36Z","receivedAt":"2009-01-14T12:25:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:\n\n> This is much shorter than inet_ntop'ing, and also translated\n> unresolvable addresses into a string.\n\nEr, doesn't this totally change the meaning of REMOTE_ADDR from an IP\naddress to a hostname? That seems bad because:\n\n - people already have hooks that compare REMOTE_ADDR against an\n address, so we are breaking their hooks\n\n - we are losing IP information in favor of hostname information; since\n (I assume) this is primarily intended for IP-based access control,\n we are adding an extra layer of indirection in the middle of our\n security model (i.e., I used to have to spoof an IP to fool your\n hook, but now I can do that _or_ spoof DNS).\n\nSo at the very least, you should be adding REMOTE_HOST in _addition_ to\nREMOTE_ADDR, not instead of. But that still leaves one final concern,\nwhich is that some git-daemon admins might not want to pay the cost for\na reverse lookup for every request. It's extra network traffic, and adds\nextra latency to the process (but I don't personally run git-daemon, and\nI don't know whether big sites like kernel.org actually care about\nthis).\n\n-Peff\n"},{"id":"100403","messageId":"alpine.LSU.2.00.0901141342060.23212@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"7vy6xe2kbx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] git-daemon: single-line logs","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T13:03:33Z","receivedAt":"2009-01-14T13:03:33Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Wednesday 2009-01-14 12:33, Junio C Hamano wrote:\n>\n>> parent v1.6.1\n>>\n>> git-daemon: single-line logs\n>\n>Please drop these two needless lines when/if you are submitting patches\n>for inclusion..\n\nThe patches are produced by my git-export-patch script (or in this\nspecific case it was quilt); so they usually look like this.\nEspecially when I do make mental notes that do not go into the\ncommit log, the patch is tacked on, as in, for example,\nhttp://marc.info/?l=netfilter-devel&m=123191159015731&w=2\n\n>> Having just a single line per connection attempt, much like Apache\n>> httpd2 access logs, makes log parsing much easier, especially when\n>> just glancing over it non-automated.\n>\n>While I like the motivation, and I wish the log were as terse as possible\n>from the day one,\n\nWell, why did no one do it like that then? My guess is that it was not a \n\"real\" logging infrastructure but more of a debug aid. Especially when \nit required --syslog --verbose to pass to git-daemon this seems like \n--debug=yesPlease.\n\n>I think changing the output format unconditionally like\n>this patch does is a horrible idea. I'd expect there are many people who\n>already have their infrastructure set up to parse the current output; this\n>patch actively breaks things for them, doesn't it?\n\nProbably. Which just shows that git-daemon is in need of\nsome configuration .. thing so that each user can choose\nhis own if desired.\n\n\n>> @@ -295,12 +295,13 @@ static int git_daemon_config(const char\n>> -static int run_service(char *dir, struct daemon_service *service)\n>> +static int run_service(char *dir, struct daemon_service *service,\n>> + const char *origin, const char *vhost)\n>> {\n>> \tconst char *path;\n>> \tint enabled = service->enabled;\n>> \n>> -\tloginfo(\"Request %s for '%s'\", service->name, dir);\n>> +\tloginfo(\"%s->%s %s \\\"%s\\\"\\n\", origin, vhost, service->name, dir);\n>\n>Mental note. You are adding origin and vhost probably because you are\n>losing them from elsewhere..\n\nNot quite sure what you mean by losing.\n\nBut in 1.6.0.x, run_service had a variable interp of type itable or so \nand it was possible to use interp[SLOT_DIR].val without someone raising \na hand declaring I lost them elsewhere ;-)\n\n>> @@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr\n>> \t\t\tport = ntohs(sin6_addr->sin6_port);\n>> #endif\n>> \t\t}\n>> -\t\tloginfo(\"Connection from %s:%d\", addrbuf, port);\n>\n>Mental note. Port is not logged anymore here.\n\nRight, I did not see a need for it, and it in itself just stood\nin the way of getting 1-line-output.\n\n>> @@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr\n>> \talarm(0);\n>> \n>> \tlen = strlen(line);\n>> -\tif (pktlen != len)\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>\n>Mental note. XA are not logged here anymore.\n\nI only ever saw the hostname XA, and this is still logged.\n\n>> @@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr\n>> \t\t\t * Note: The directory here is probably context sensitive,\n>> \t\t\t * and might depend on the actual service being performed.\n>> \t\t\t */\n>> -\t\t\treturn run_service(line + namelen + 5, s);\n>> +\t\t\treturn run_service(line + namelen + 5, s,\n>> +\t\t\t addrbuf, hostname);\n>> \t\t}\n>> \t}\n>\n>So not just you are changing the format, but you are losing information as\n>well.\n>\n>By the way, I think hostname has already been freed and NULLed at this\n>call site. Aren't you getting entries like:\n>\n>\t192.168.0.1->(null) upload-pack \"/pub/git.git\"\n>\n>in your log?\n\nNo. Which means someone succeeded at obfuscating daemon.c.\nIt seems that parse_extra_args() sets hostname again after it has been \nNULLified just moments ago.\n"},{"id":"100404","messageId":"alpine.LSU.2.00.0901141404150.23212@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"7vsknm2kbs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T13:06:12Z","receivedAt":"2009-01-14T13:06:12Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Wednesday 2009-01-14 12:33, Junio C Hamano wrote:\n>> git-daemon: use getnameinfo to resolve hostname\n>>\n>> This is much shorter than inet_ntop'ing, and also translated\n>> unresolvable addresses into a string.\n>\n>translated? (I think you meant \"translates\" but my English is bad, so I\n>am double checking).\n\nyes, keyboard slipped away.\n\n>This indeed is much nicer, provided if it is available at least as widely\n>as inet_ntop() is.\n\nBoth inet_ntop and getnameinfo are in POSIX.1-2001.\n\n> (1) Do we need similar compat/ function for getnameinfo()? I am guessing\n> that most likely places are the ones that need NO_INET_NTOP and\n> NO_INET_PTON, and googling seems to indicate old Cygwin and HP-UX\n> 11.00 may be among them.\n>\n> (2) Do we still use inet_ntop() elsewhere, and if not, can we remove the\n> compat/ definitions?\n>\n\nYes, it is still used elsewhere.\n"},{"id":"100405","messageId":"alpine.LSU.2.00.0901141409060.23212@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"7vmydu2kbj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] git-daemon: vhost support","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T13:15:43Z","receivedAt":"2009-01-14T13:15:43Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Wednesday 2009-01-14 12:33, Junio C Hamano wrote:\n>> git-daemon: support vhosts\n>>\n>> Since git clients usually send the target hostname in the request\n>> similar to the \"Host:\" HTTP header, one can do virtual hosting.\n>\n>Isn't this what --interpolated-path option (especially H and CH\n>interpolations) is about?\n\nLooks like it. In this case this third patch is not needed.\n"},{"id":"100410","messageId":"20090114141723.GA6984@chistera.yi.org","threadId":"17167","inReplyTo":"20090114122536.GA5939@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-14T14:17:23Z","receivedAt":"2009-01-14T14:17:23Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* Jeff King [Wed, 14 Jan 2009 07:25:36 -0500]:\n\n> On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:\n\n> > This is much shorter than inet_ntop'ing, and also translated\n> > unresolvable addresses into a string.\n\n> Er, doesn't this totally change the meaning of REMOTE_ADDR from an IP\n> address to a hostname?\n\nYes, I believe so.\n\nHowever, AFAIK you can obtain the intended behavior if you pass\nNI_NUMERICHOST as a flag to the getnameinfo() call. With that, this\npatch can be still considered for inclusing if the original \"don't\nhardcode protocol-specific bits\" is still deemed worthy.\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \n- Why are you whispering?\n- Because I just think that no matter where she is, my mom can hear this\n conversation.\n -- Rory and Lane\n"},{"id":"100411","messageId":"76718490901140622i1c29cd96u1b30042ad9ecb5d9@mail.gmail.com","threadId":"17167","inReplyTo":"20090114122536.GA5939@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-14T14:22:45Z","receivedAt":"2009-01-14T14:22:45Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Jan 14, 2009 at 7:25 AM, Jeff King wrote:\n> So at the very least, you should be adding REMOTE_HOST in _addition_ to\n> REMOTE_ADDR, not instead of. But that still leaves one final concern,\n> which is that some git-daemon admins might not want to pay the cost for\n> a reverse lookup for every request. It's extra network traffic, and adds\n> extra latency to the process (but I don't personally run git-daemon, and\n> I don't know whether big sites like kernel.org actually care about\n> this).\n\nSpeaking for large sites everywhere, yes they do care. Enabling DNS\nlookups must be configurable.\n\nj.\n"},{"id":"100463","messageId":"alpine.LSU.2.00.0901142014380.24672@fbirervta.pbzchgretzou.qr","threadId":"17167","inReplyTo":"20090114122536.GA5939@coredump.intra.peff.net","subject":"Re: [2/3] git-daemon: use getnameinfo to resolve hostname","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2009-01-14T19:25:28Z","receivedAt":"2009-01-14T19:25:28Z","isPatch":false,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Wednesday 2009-01-14 13:25, Jeff King wrote:\n>On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:\n>\n>> This is much shorter than inet_ntop'ing, and also translated\n>> unresolvable addresses into a string.\n>\n>Er, doesn't this totally change the meaning of REMOTE_ADDR from an IP\n>address to a hostname? That seems bad because:\n>[...]\n> - people already have hooks that compare REMOTE_ADDR against an\n> address, so we are breaking their hooks\n>[...]\n>So at the very least, you should be adding REMOTE_HOST in _addition_ to\n>REMOTE_ADDR, not instead of.\n\nGood catch. It's always good to have someone else look through it.\nChanged, and below is the proposition as a non-mergable diff.\n\nIn case getnameinfo fails, the IP address from inet_ntop\nshould be left in addrbuf, is not it?\n\nAnd yeah, it does not have a flag to disable DNS resolution, but\nit's a draft for now.\n\n---8<---\ngit-daemon: resolve source host's DNS\n\nTry to resolve DNS addresses so that run_service() can print the\nname of the host from which the request originated.\n[addrbuf is passed to run_service as a result of patch 1/3]\n\n---\n daemon.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\nIndex: git-1.6.1/daemon.c\n===================================================================\n--- git-1.6.1.orig/daemon.c\n+++ git-1.6.1/daemon.c\n@@ -530,6 +530,10 @@ static int execute(struct sockaddr *addr\n #endif\n \t\t}\n \t\tsetenv(\"REMOTE_ADDR\", addrbuf, 1);\n+\t\tgetnameinfo(addr, (addr->sa_family == AF_INET6) ?\n+\t\t\tsizeof(struct sockaddr_in6) :\n+\t\t\tsizeof(struct sockaddr_in),\n+\t\t\taddrbuf, sizeof(addrbuf), NULL, 0, 0);\n \t}\n \telse {\n \t\tunsetenv(\"REMOTE_ADDR\");\n"}]}