wireaddr: rework port parsing for weird addresses.

We save wireaddr to databases as a string (which is pretty dumb) but
it turned out that my local node saved '[::ffff:127.0.0.1]:49150'
which our parser can't parse.

Thus I've reworked the parser to make fewer assumptions:
parse_ip_port() is renamed to separate_address_and_port() and is now
far more accepting of different forms, and returns failure only on
grossly malformed strings.  Otherwise it overwrites its *port arg only
if there's a port specified.  I also made it static.

Then fromwire_wireaddr() hands the resulting address to inet_pton to
figure out if it's actually valid.

Cc: William Casarin <jb55@jb55.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/test/run-ip_port_parsing.c

@@ -2,6 +2,7 @@
 
 #include <stdio.h>
 #include <assert.h>
+#include <ccan/mem/mem.h>
 
 /* AUTOGENERATED MOCKS START */
 /* Generated stub for fromwire */
@@ -31,36 +32,44 @@ int main(void)
 	char *ip;
 	u16 port;
 
+	/* Grossly invalid. */
+	assert(!separate_address_and_port(ctx, "[", &ip, &port));
+	assert(!separate_address_and_port(ctx, "[123", &ip, &port));
+	assert(!separate_address_and_port(ctx, "[::1]:8f", &ip, &port));
+	assert(!separate_address_and_port(ctx, "127.0.0.1:8f", &ip, &port));
+	assert(!separate_address_and_port(ctx, "127.0.0.1:0", &ip, &port));
+	assert(!separate_address_and_port(ctx, "127.0.0.1:ff", &ip, &port));
+
 	/* ret = getaddrinfo("[::1]:80", NULL, NULL, &res); */
-	assert(parse_ip_port(ctx, "[::1]:80", &ip, &port));
+	assert(separate_address_and_port(ctx, "[::1]:80", &ip, &port));
 	assert(streq(ip, "::1"));
 	assert(port == 80);
 
-	assert(!parse_ip_port(ctx, "ip6-localhost", &ip, &port));
+	port = 0;
+	assert(separate_address_and_port(ctx, "ip6-localhost", &ip, &port));
 	assert(streq(ip, "ip6-localhost"));
 	assert(port == 0);
 
-	assert(!parse_ip_port(ctx, "::1", &ip, &port));
+	assert(separate_address_and_port(ctx, "::1", &ip, &port));
 	assert(streq(ip, "::1"));
 	assert(port == 0);
 
-	assert(parse_ip_port(ctx, "192.168.1.1:8000", &ip, &port));
+	assert(separate_address_and_port(ctx, "192.168.1.1:8000", &ip, &port));
 	assert(streq(ip, "192.168.1.1"));
 	assert(port == 8000);
 
-	assert(!parse_ip_port(ctx, "192.168.2.255", &ip, &port));
+	port = 0;
+	assert(separate_address_and_port(ctx, "192.168.2.255", &ip, &port));
 	assert(streq(ip, "192.168.2.255"));
 	assert(port == 0);
 
 	// unusual but possibly valid case
-	assert(!parse_ip_port(ctx, "[::1]", &ip, &port));
+	assert(separate_address_and_port(ctx, "[::1]", &ip, &port));
 	assert(streq(ip, "::1"));
 	assert(port == 0);
 
 	// service names not supported yet
-	assert(!parse_ip_port(ctx, "[::1]:http", &ip, &port));
+	assert(!separate_address_and_port(ctx, "[::1]:http", &ip, &port));
-	assert(streq(ip, "::1"));
-	assert(port == 0);
 
 	// localhost hostnames for backward compat
 	parse_wireaddr("localhost", &addr, 200);
@@ -79,6 +88,8 @@ int main(void)
 	ip = fmt_wireaddr(ctx, &addr);
 	assert(streq(ip, "[2001:db8:85a3::8a2e:370:7334]:9777"));
 
+	assert(parse_wireaddr("[::ffff:127.0.0.1]:49150", &addr, 1));
+	assert(addr.port == 49150);
 	tal_free(ctx);
 	return 0;
 }

common/wireaddr.c

@@ -65,6 +65,51 @@ char *fmt_wireaddr(const tal_t *ctx, const struct wireaddr *a)
 }
 REGISTER_TYPE_TO_STRING(wireaddr, fmt_wireaddr);
 
+/* Valid forms:
+ *
+ * [anything]:<number>
+ * anything-without-colons-or-left-brace:<number>
+ * anything-without-colons
+ * string-with-multiple-colons
+ *
+ * Returns false if it wasn't one of these forms.  If it returns true,
+ * it only overwrites *port if it was specified by <number> above.
+ */
+static bool separate_address_and_port(tal_t *ctx, const char *arg,
+				      char **addr, u16 *port)
+{
+	char *portcolon;
+
+	if (strstarts(arg, "[")) {
+		char *end = strchr(arg, ']');
+		if (!end)
+			return false;
+		/* Copy inside [] */
+		*addr = tal_strndup(ctx, arg + 1, end - arg - 1);
+		portcolon = strchr(end+1, ':');
+	} else {
+		portcolon = strchr(arg, ':');
+		if (portcolon) {
+			/* Disregard if there's more than one : or if it's at
+			   the start or end */
+			if (portcolon != strrchr(arg, ':')
+			    || portcolon == arg
+			    || portcolon[1] == '\0')
+				portcolon = NULL;
+		}
+		if (portcolon)
+			*addr = tal_strndup(ctx, arg, portcolon - arg);
+		else
+			*addr = tal_strdup(ctx, arg);
+	}
+
+	if (portcolon) {
+		char *endp;
+		*port = strtol(portcolon + 1, &endp, 10);
+		return *port != 0 && *endp == '\0';
+	}
+	return true;
+}
 
 bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport)
 {
@@ -78,13 +123,15 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport)
 	res = false;
 	port = defport;
 
-	if (!parse_ip_port(tmpctx, arg, &ip, &port))
+	if (!separate_address_and_port(tmpctx, arg, &ip, &port)) {
-		port = defport;
+		tal_free(tmpctx);
+		return false;
+	}
 
 	/* FIXME: change arg to addr[:port] and use getaddrinfo? */
-	if (streq(arg, "localhost"))
+	if (streq(ip, "localhost"))
 		ip = "127.0.0.1";
-	else if (streq(arg, "ip6-localhost"))
+	else if (streq(ip, "ip6-localhost"))
 		ip = "::1";
 
 	memset(&addr->addr, 0, sizeof(addr->addr));
@@ -106,37 +153,3 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport)
 	tal_free(tmpctx);
 	return res;
 }
-
-// NOTE: arg is assumed to be an ipv4/6 addr string with optional port
-bool parse_ip_port(tal_t *ctx, const char *arg, char **ip, u16 *port) {
-	*port = 0;
-	*ip = tal_strdup(ctx, arg);
-
-	bool ipv6 = strchr(*ip, '.') == NULL;
-	bool has_brackets = strchr(*ip, '[');
-	bool has_colon = strchr(*ip, ':');
-
-	// we have an ip addr with no port
-	if ((ipv6 && !has_brackets) || (!ipv6 && !has_colon))
-		return false;
-
-	/* IPv6 can have [ ], trim them here */
-	if (ipv6 && strstarts(*ip, "[") && strends(*ip, "]")) {
-		(*ip)++;
-		(*ip)[strlen(*ip)-1] = '\0';
-		return false;
-	}
-
-	// we have a port, let's go to it
-	const char *last_colon = strrchr(*ip, ':');
-	assert(last_colon);
-
-	// chop off port
-	(*ip)[last_colon - *ip - ipv6] = '\0';
-
-	// skip over first [ if ipv6
-	if (ipv6) (*ip)++;
-
-	*port = atoi(last_colon + 1);
-	return *port != 0;
-}

common/wireaddr.h

@@ -41,7 +41,6 @@ struct wireaddr {
 void towire_wireaddr(u8 **pptr, const struct wireaddr *addr);
 bool fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr);
 
-bool parse_ip_port(tal_t *ctx, const char *arg, char **ip, u16 *port);
 bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 port);
 
 char *fmt_wireaddr(const tal_t *ctx, const struct wireaddr *a);