{"thread":{"id":"19501","subject":"[PATCH] imap-send: add support for IPv6","startedAt":"2009-05-25T19:13:54Z","lastAt":"2009-05-27T10:50:11Z","messageCount":3,"participants":["Benjamin Kramer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"114688","messageId":"4A1AEDF2.7060307@googlemail.com","threadId":"19501","inReplyTo":null,"subject":"[PATCH] imap-send: add support for IPv6","fromName":"Benjamin Kramer","fromEmail":"benny.kra@googlemail.com","sentAt":"2009-05-25T19:13:54Z","receivedAt":"2009-05-25T19:13:54Z","isPatch":true,"sender":{"key":"benny.kra@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/16542?v=4"},"body":"Add IPv6 support by implementing name resolution with the\nprotocol agnostic getaddrinfo(3) API. The old gethostbyname(3)\ncode is still available when git is compiled with NO_IPV6.\n\nSigned-off-by: Benjamin Kramer <benny.kra@googlemail.com>\n---\n imap-send.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 51 insertions(+), 3 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 8154cb2..861268a 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -982,9 +982,7 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n \tstruct imap_store *ctx;\n \tstruct imap *imap;\n \tchar *arg, *rsp;\n-\tstruct hostent *he;\n-\tstruct sockaddr_in addr;\n-\tint s, a[2], preauth;\n+\tint s = -1, a[2], preauth;\n \tpid_t pid;\n \n \tctx = xcalloc(sizeof(*ctx), 1);\n@@ -1021,6 +1019,51 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n \n \t\timap_info(\"ok\\n\");\n \t} else {\n+#ifndef NO_IPV6\n+\t\tstruct addrinfo hints, *ai0, *ai;\n+\t\tint gai;\n+\t\tchar portstr[6];\n+\n+\t\tsnprintf(portstr, sizeof(portstr), \"%hu\", srvc->port);\n+\n+\t\tmemset(&hints, 0, sizeof(hints));\n+\t\thints.ai_socktype = SOCK_STREAM;\n+\t\thints.ai_protocol = IPPROTO_TCP;\n+\n+\t\timap_info(\"Resolving %s... \", srvc->host);\n+\t\tgai = getaddrinfo(srvc->host, portstr, &hints, &ai);\n+\t\tif (gai) {\n+\t\t\tfprintf(stderr, \"getaddrinfo: %s\\n\", gai_strerror(gai));\n+\t\t\tgoto bail;\n+\t\t}\n+\t\timap_info(\"ok\\n\");\n+\n+\t\tfor (ai0 = ai; ai; ai = ai->ai_next) {\n+\t\t\tchar addr[NI_MAXHOST];\n+\n+\t\t\ts = socket(ai->ai_family, ai->ai_socktype,\n+\t\t\t\t   ai->ai_protocol);\n+\t\t\tif (s < 0)\n+\t\t\t\tcontinue;\n+\n+\t\t\tgetnameinfo(ai->ai_addr, ai->ai_addrlen, addr,\n+\t\t\t\t    sizeof(addr), NULL, 0, NI_NUMERICHOST);\n+\t\t\timap_info(\"Connecting to [%s]:%s... \", addr, portstr);\n+\n+\t\t\tif (connect(s, ai->ai_addr, ai->ai_addrlen) < 0) {\n+\t\t\t\tclose(s);\n+\t\t\t\ts = -1;\n+\t\t\t\tperror(\"connect\");\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tbreak;\n+\t\t}\n+\t\tfreeaddrinfo(ai0);\n+#else /* NO_IPV6 */\n+\t\tstruct hostent *he;\n+\t\tstruct sockaddr_in addr;\n+\n \t\tmemset(&addr, 0, sizeof(addr));\n \t\taddr.sin_port = htons(srvc->port);\n \t\taddr.sin_family = AF_INET;\n@@ -1040,7 +1083,12 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n \t\timap_info(\"Connecting to %s:%hu... \", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port));\n \t\tif (connect(s, (struct sockaddr *)&addr, sizeof(addr))) {\n \t\t\tclose(s);\n+\t\t\ts = -1;\n \t\t\tperror(\"connect\");\n+\t\t}\n+#endif\n+\t\tif (s < 0) {\n+\t\t\tfputs(\"Error: unable to connect to server.\\n\", stderr);\n \t\t\tgoto bail;\n \t\t}\n \n-- \n1.6.3.1.153.g63801e\n"},{"id":"114770","messageId":"7v7i03kqjq.fsf@alter.siamese.dyndns.org","threadId":"19501","inReplyTo":"4A1AEDF2.7060307@googlemail.com","subject":"Re: [PATCH] imap-send: add support for IPv6","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-27T06:28:41Z","receivedAt":"2009-05-27T06:28:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Benjamin Kramer <benny.kra@googlemail.com> writes:\n\n> Add IPv6 support by implementing name resolution with the\n> protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)\n> code is still available when git is compiled with NO_IPV6.\n\n> diff --git a/imap-send.c b/imap-send.c\n> index 8154cb2..861268a 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -1021,6 +1019,51 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n>  \n>  \t\timap_info(\"ok\\n\");\n>  \t} else {\n> +#ifndef NO_IPV6\n> +\t\tstruct addrinfo hints, *ai0, *ai;\n> +\t\tint gai;\n> +\t\tchar portstr[6];\n> +\n> +\t\tsnprintf(portstr, sizeof(portstr), \"%hu\", srvc->port);\n\nWe already use %h length specifier to explicitly say the parameter is\na short in the IPV4 part of this program, so I am sure this won't regress\nanything for people, but I wonder what the point of it is...  (I am not\nasking nor even suggesting to change this, by the way).\n\n> +\t\tmemset(&hints, 0, sizeof(hints));\n> +\t\thints.ai_socktype = SOCK_STREAM;\n> +\t\thints.ai_protocol = IPPROTO_TCP;\n> +\n> +\t\timap_info(\"Resolving %s... \", srvc->host);\n> +\t\tgai = getaddrinfo(srvc->host, portstr, &hints, &ai);\n> +\t\tif (gai) {\n> +\t\t\tfprintf(stderr, \"getaddrinfo: %s\\n\", gai_strerror(gai));\n> +\t\t\tgoto bail;\n\nIt is Ok for now (as existing codepath liberally uses fprintf() and\nfputs() to report errors), but ideally we should start converting these to\nerror() calls, I think, in a follow-up patch.\n\n> +\t\t}\n> +\t\timap_info(\"ok\\n\");\n> +\n> +\t\tfor (ai0 = ai; ai; ai = ai->ai_next) {\n> +\t\t\tchar addr[NI_MAXHOST];\n> +\n> +\t\t\ts = socket(ai->ai_family, ai->ai_socktype,\n> +\t\t\t\t   ai->ai_protocol);\n> +\t\t\tif (s < 0)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tgetnameinfo(ai->ai_addr, ai->ai_addrlen, addr,\n> +\t\t\t\t    sizeof(addr), NULL, 0, NI_NUMERICHOST);\n> +\t\t\timap_info(\"Connecting to [%s]:%s... \", addr, portstr);\n\nIs forcing to NUMERICHOST done to match IPV4 codepath that does\ninet_ntoa()?  I guess that makes sense.\n\n> +\t\t\tif (connect(s, ai->ai_addr, ai->ai_addrlen) < 0) {\n> +\t\t\t\tclose(s);\n> +\t\t\t\ts = -1;\n> +\t\t\t\tperror(\"connect\");\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tfreeaddrinfo(ai0);\n> +#else /* NO_IPV6 */\n> +\t\tstruct hostent *he;\n> +\t\tstruct sockaddr_in addr;\n> +\n>  \t\tmemset(&addr, 0, sizeof(addr));\n>  \t\taddr.sin_port = htons(srvc->port);\n>  \t\taddr.sin_family = AF_INET;\n> @@ -1040,7 +1083,12 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n>  \t\timap_info(\"Connecting to %s:%hu... \", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port));\n>  \t\tif (connect(s, (struct sockaddr *)&addr, sizeof(addr))) {\n>  \t\t\tclose(s);\n> +\t\t\ts = -1;\n>  \t\t\tperror(\"connect\");\n> +\t\t}\n> +#endif\n> +\t\tif (s < 0) {\n> +\t\t\tfputs(\"Error: unable to connect to server.\\n\", stderr);\n>  \t\t\tgoto bail;\n>  \t\t}\n\nI do not use imap-send myself, and I suspect not many people do so either,\nso I originally guessed the best way to judge if this has any portability\n(or other) issues is to directly apply to 'master' and see if anybody\nscreams.\n\nThanks.\n"},{"id":"114790","messageId":"4A1D1AE3.9030900@googlemail.com","threadId":"19501","inReplyTo":"7v7i03kqjq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] imap-send: add support for IPv6","fromName":"Benjamin Kramer","fromEmail":"benny.kra@googlemail.com","sentAt":"2009-05-27T10:50:11Z","receivedAt":"2009-05-27T10:50:11Z","isPatch":true,"sender":{"key":"benny.kra@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/16542?v=4"},"body":"Junio C Hamano wrote:\n> We already use %h length specifier to explicitly say the parameter is\n> a short in the IPV4 part of this program, so I am sure this won't regress\n> anything for people, but I wonder what the point of it is...  (I am not\n> asking nor even suggesting to change this, by the way).\n\ngetaddrinfo(3) takes the port as a string so we have to convert it. I tried\nto match the style of the existing code with the format string. The portstr\nbuffer is 6 chars long so the highest possible unsigned short 65535 fits in\nexactly.\n\n> It is Ok for now (as existing codepath liberally uses fprintf() and\n> fputs() to report errors), but ideally we should start converting these to\n> error() calls, I think, in a follow-up patch.\n\nLooking at the number of fprintfs in imap-send.c a simple search/replace\nprobably won't do the job here. I tried to contact the original author but\nhis mail address seems to be dead ...\n\n> Is forcing to NUMERICHOST done to match IPV4 codepath that does\n> inet_ntoa()?  I guess that makes sense.\n\nWe need to get the IP string, otherwise the output of imap-send would\nmake no sense. Here's what imap-send outputs when I try to connect to\nlocalhost.\n\nWithout the patch:\n    Resolving localhost... ok\n    Connecting to 127.0.0.1:993... connect: Connection refused\n\nWith the patch:\n    Resolving localhost... ok\n    Connecting to [::1]:993... connect: Connection refused\n    Connecting to [fe80::1%lo0]:993... connect: Connection refused\n    Connecting to [127.0.0.1]:993... connect: Connection refused\n    Error: unable to connect to server.\n\nUsing the hostname instead of the IP address here wouldn't be very useful.\n"}]}