Hi Willy,
Here are two patches, rebased on current HEAD.
The first allows IPv6 with square brackets, the second parses colon to
look for a port.
The logic is meant to be backward compatible with current behavior : as
you suggested, starting from the end, it looks for either a colon or a
closing bracket.
If a colon is found, it’s considered as a port separator, thus behaving
like previously.
If something else is found, either a closing bracket or nothing at all,
then it consider that no port were provided.
str2ip2 is then in charge of validating wether the string is a valid IP
address or not.
Malformed IP addresses, whatever its format, will be rejected.
--
Mildis
Le 2015-10-13 21:05, Willy Tarreau a écrit :
On Tue, Oct 13, 2015 at 08:10:03PM +0200, Mildis wrote:
Le 2015-10-10 15:49, Willy Tarreau a écrit :
>Hi,
>@@ -856,11 +883,28 @@ struct sockaddr_storage *str2sa_range(const char
>*str, int *low, int *high, char
> else { /* IPv4 and IPv6 */
> int use_fqdn = 0;
>
>- port1 = strrchr(str2, ':');
>- if (port1)
>- *port1++ = '\0';
>- else
>+ /* IPv6 wildcard */
>+ if (!strcmp(str2, "::")) {
>+ port1 = "";
>+ }
>
>You're changing the parser here, because it now accepts "::" in a place
>where a port was expected.
May I ask : isn???t the parser just a parser ? Making the caller???s
responsibility to use or not the ports returned either in the
sockaddr_storage or the 2 int depending on the context ? (???bind???
requires a port but ???server??????s port is optional)
Is str2sa_range aware of the context it???s called ?
str2sa_range() parses ports, it relies on str2ip2() which only takes
addresses. So according to this it *is* the caller. Your change above
broke this because you're making the former ignore its own
responsibility
to properly consider the port which is mandatory if the address
contains
a colon. And that's precisely because str2sa_range() doesn't have to be
aware of the context it's called that we want it to behave
consistently.
Regards,
Willy
From 393ec4d4778c126e59fa7bd24e733bb32ca282a9 Mon Sep 17 00:00:00 2001
From: mildis <[email protected]>
Date: Sat, 17 Oct 2015 15:38:51 +0200
Subject: [PATCH 1/2] OPTIM: config: allow square-bracketed literal IPv6.
To enhance readability, allows IPv6 literal addresses with square brackets.
Last colon is still mandatory for address/port separation.
---
src/standard.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/src/standard.c b/src/standard.c
index b5f4bf4..2e064b5 100644
--- a/src/standard.c
+++ b/src/standard.c
@@ -620,9 +620,31 @@ const char *invalid_domainchar(const char *name) {
struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, int resolve)
{
struct hostent *he;
+ /* add 2 chars for square brackets */
+ char tmpip[INET6_ADDRSTRLEN+2] = { '\0' };
+ /* point to str by default, if FQDN processing is required */
+ char *tmpipptr = (char *)str;
+
+ /* check IPv6 with square brackets */
+ if (str[0] == '[') {
+ /* copy and strip the opening bracket */
+ strncpy(tmpip, str+1, sizeof(tmpip));
+ size_t iplength = strlen(tmpip);
+ if (tmpip[iplength - 1] != ']') {
+ /* if address started with bracket,
+ it should end with bracket */
+ goto fail;
+ }
+ else {
+ /* switch to tmpip as we are working on a literal bracketed IPv6 */
+ tmpipptr = tmpip;
+ /* strip last bracket */
+ tmpipptr[iplength-1] = '\0';
+ }
+ }
/* Any IPv6 address */
- if (str[0] == ':' && str[1] == ':' && !str[2]) {
+ if (tmpipptr[0] == ':' && tmpipptr[1] == ':' && !tmpipptr[2]) {
if (!sa->ss_family || sa->ss_family == AF_UNSPEC)
sa->ss_family = AF_INET6;
else if (sa->ss_family != AF_INET6)
@@ -631,7 +653,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
}
/* Any address for the family, defaults to IPv4 */
- if (!str[0] || (str[0] == '*' && !str[1])) {
+ if (!tmpipptr[0] || (tmpipptr[0] == '*' && !tmpipptr[1])) {
if (!sa->ss_family || sa->ss_family == AF_UNSPEC)
sa->ss_family = AF_INET;
return sa;
@@ -639,14 +661,14 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
/* check for IPv6 first */
if ((!sa->ss_family || sa->ss_family == AF_UNSPEC || sa->ss_family == AF_INET6) &&
- inet_pton(AF_INET6, str, &((struct sockaddr_in6 *)sa)->sin6_addr)) {
+ inet_pton(AF_INET6, tmpipptr, &((struct sockaddr_in6 *)sa)->sin6_addr)) {
sa->ss_family = AF_INET6;
return sa;
}
/* then check for IPv4 */
if ((!sa->ss_family || sa->ss_family == AF_UNSPEC || sa->ss_family == AF_INET) &&
- inet_pton(AF_INET, str, &((struct sockaddr_in *)sa)->sin_addr)) {
+ inet_pton(AF_INET, tmpipptr, &((struct sockaddr_in *)sa)->sin_addr)) {
sa->ss_family = AF_INET;
return sa;
}
@@ -654,7 +676,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
if (!resolve)
return NULL;
- if (!dns_hostname_validation(str, NULL))
+ if (!dns_hostname_validation(tmpipptr, NULL))
return NULL;
#ifdef USE_GETADDRINFO
@@ -668,7 +690,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
hints.ai_flags = 0;
hints.ai_protocol = 0;
- if (getaddrinfo(str, NULL, &hints, &result) == 0) {
+ if (getaddrinfo(tmpipptr, NULL, &hints, &result) == 0) {
if (!sa->ss_family || sa->ss_family == AF_UNSPEC)
sa->ss_family = result->ai_family;
else if (sa->ss_family != result->ai_family)
@@ -689,7 +711,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
}
#endif
/* try to resolve an IPv4/IPv6 hostname */
- he = gethostbyname(str);
+ he = gethostbyname(tmpipptr);
if (he) {
if (!sa->ss_family || sa->ss_family == AF_UNSPEC)
sa->ss_family = he->h_addrtype;
@@ -747,10 +769,11 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
* the first byte of the address.
* - "fd@" => an integer must follow, and is a file descriptor number.
*
- * Also note that in order to avoid any ambiguity with IPv6 addresses, the ':'
- * is mandatory after the IP address even when no port is specified. NULL is
- * returned if the address cannot be parsed. The <low> and <high> ports are
- * always initialized if non-null, even for non-IP families.
+ * IPv6 addresses can be declared with or without square brackets. Also note
+ * that in order to avoid any ambiguity with IPv6 addresses, the last colon ':'
+ * is mandatory even when no port is specified. NULL is returned if the address
+ * cannot be parsed. The <low> and <high> ports are always initialized if
+ * non-null, even for non-IP families.
*
* If <pfx> is non-null, it is used as a string prefix before any path-based
* address (typically the path to a unix socket).
--
2.3.8 (Apple Git-58)
From d40c9d26e6db7855807ebfd48923c284e3769554 Mon Sep 17 00:00:00 2001
From: mildis <[email protected]>
Date: Thu, 22 Oct 2015 20:14:54 +0200
Subject: [PATCH 2/2] OPTIM: config: make last colon optional for IPv6 literal
square-bracketed notation
---
src/standard.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/standard.c b/src/standard.c
index 2e064b5..e93375c 100644
--- a/src/standard.c
+++ b/src/standard.c
@@ -879,12 +879,28 @@ struct sockaddr_storage *str2sa_range(const char *str, int *low, int *high, char
else { /* IPv4 and IPv6 */
int use_fqdn = 0;
- port1 = strrchr(str2, ':');
- if (port1)
- *port1++ = '\0';
- else
- port1 = "";
-
+ /* search for : or ] whatever comes first */
+ char *end = str2 + strlen(str2);
+ char *chr = end-1;
+ for (; chr >= str2; chr--) {
+ if (*chr == ']' || *chr == ':')
+ break;
+ }
+
+ if (*chr == ':') {
+ /* Found a colon before a closing-bracket, must be a port separator.
+ * This guarantee backward compatibility.
+ */
+ *chr++ = '\0';
+ port1 = chr;
+ }
+ else {
+ /* Either no colon and no closing-bracket
+ * or directly ending with a closing-bracket.
+ * However, no port */
+ port1 = "";
+ }
+
if (str2ip2(str2, &ss, 0) == NULL) {
use_fqdn = 1;
if (!resolve || str2ip2(str2, &ss, 1) == NULL) {
--
2.3.8 (Apple Git-58)
