resolv: add IPv6 support to inet_net_pton()
Checks
Commit Message
Dear all,
This changeset adds support to inet_net_pton() to convert IPv6 network
numbers (IPv6 prefixes with CIDR notation) from presentation format to
network format.
The starting point of this changeset was OpenBSD's
libc/net/inet_net_pton.c (r1.13) implementation of inet_net_pton_ipv6().
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/net/inet_net_pton.c?annotate=1.13
The OpenBSD implementation was adapted to glibc as following:
1) Use strncpy() instead of strlcpy()
2) Use strtol() instead of strtonum()
3) Updated comments
I've tested the changeset on Debian Bookworm.
Kind regards,
Job
Signed-off-by: Job Snijders <job@fastly.com>
Comments
* Job Snijders via Libc-alpha:
> + strncpy(buf, src, sizeof(buf) - 1);
> + buf[sizeof(buf) - 1] = '\0';
> +
> + sep = strchr(buf, '/');
> + if (sep != NULL)
> + *sep++ = '\0';
> +
> + if (inet_pton(AF_INET6, buf, &in6) != 1) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
Thanks for the patch.
We have __inet_pton_length for internal use. You can use it to avoid
the copy. Would you be interested in reworking your patch based on it?
Florian
Dear all,
On 12/22/22 18:54, Job Snijders via Libc-alpha wrote:
> Dear all,
>
> This changeset adds support to inet_net_pton() to convert IPv6 network
> numbers (IPv6 prefixes with CIDR notation) from presentation format to
> network format.
>
> The starting point of this changeset was OpenBSD's
> libc/net/inet_net_pton.c (r1.13) implementation of inet_net_pton_ipv6().
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/net/inet_net_pton.c?annotate=1.13
> The OpenBSD implementation was adapted to glibc as following:
>
> 1) Use strncpy() instead of strlcpy()
Would someone please add a function to glibc that truncates a string, while
still producing a string (as opposed to a null-padded fixed-width character
sequence)?
Here goes an extract of the yet-unreleased strncpy(3) manual page from the Linux
man-pages master branch:
DESCRIPTION
These functions copy the string pointed to by src into a null‐padded
character sequence at the fixed‐width buffer pointed to by dst. If the
destination buffer, limited by its size, isn’t large enough to hold the
copy, the resulting character sequence is truncated. For the differ‐
ence between the two functions, see RETURN VALUE.
An implementation of these functions might be:
char *
stpncpy(char *restrict dst, const char *restrict src, size_t sz)
{
bzero(dst, sz);
return mempcpy(dst, src, strnlen(src, sz));
}
char *
strncpy(char *restrict dst, const char *restrict src, size_t sz)
{
stpncpy(dst, src, sz);
return dst;
}
[...]
CAVEATS
The name of these functions is confusing. These functions produce a
null‐padded character sequence, not a string (see string_copying(7)).
It’s impossible to distinguish truncation by the result of the call,
from a character sequence that just fits the destination buffer; trun‐
cation should be detected by comparing the length of the input string
with the size of the destination buffer.
I'll be releasing the a new man-pages version very soon (a week at most), so
that this page and also the new string_copying(7) overview are widely available.
Cheers,
Alex
> 2) Use strtol() instead of strtonum()
> 3) Updated comments
>
> I've tested the changeset on Debian Bookworm.
>
> Kind regards,
>
> Job
>
>
> Signed-off-by: Job Snijders <job@fastly.com>
>
> diff --git resolv/inet_net_pton.c resolv/inet_net_pton.c
> index aab9b7b582..163e76e1a5 100644
> --- resolv/inet_net_pton.c
> +++ resolv/inet_net_pton.c
> @@ -1,4 +1,6 @@
> /*
> + * Copyright (c) 2022 Job Snijders <job@fastly.com>
> + * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
> * Copyright (c) 1996,1999 by Internet Software Consortium.
> *
> * Permission to use, copy, modify, and distribute this software for any
> @@ -35,13 +37,16 @@
>
> static int inet_net_pton_ipv4 (const char *src, u_char *dst,
> size_t size) __THROW;
> +static int inet_net_pton_ipv6 (const char *src, u_char *dst,
> + size_t size) __THROW;
>
> /*
> - * static int
> + * int
> * inet_net_pton(af, src, dst, size)
> - * convert network number from presentation to network format.
> - * accepts hex octets, hex strings, decimal octets, and /CIDR.
> - * "size" is in bytes and describes "dst".
> + * Convert network number from presentation format to network format.
> + * If "af" is set to AF_INET, accept various formats like hex octets,
> + * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
> + * IPv6 addresses. "size" is in bytes and describes "dst".
> * return:
> * number of bits, either imputed classfully or specified with /CIDR,
> * or -1 if some failure occurred (check errno). ENOENT means it was
> @@ -55,6 +60,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
> switch (af) {
> case AF_INET:
> return (inet_net_pton_ipv4(src, dst, size));
> + case AF_INET6:
> + return (inet_net_pton_ipv6(src, dst, size));
> default:
> __set_errno (EAFNOSUPPORT);
> return (-1);
> @@ -196,3 +203,64 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
> __set_errno (EMSGSIZE);
> return (-1);
> }
> +
> +
> +/*
> + * Convert an IPv6 prefix from presentation format to network format.
> + * Return the number of bits specified, or -1 as error (check errno).
> + */
> +static int
> +inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
> +{
> + struct in6_addr in6;
> + int bits;
> + long lbits;
> + size_t bytes;
> + char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> + char *ep, *sep;
> +
> + strncpy(buf, src, sizeof(buf) - 1);
The -1 above is unnecessary.
> + buf[sizeof(buf) - 1] = '\0';
> +
> + sep = strchr(buf, '/');
> + if (sep != NULL)
> + *sep++ = '\0';
> +
> + if (inet_pton(AF_INET6, buf, &in6) != 1) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> +
> + if (sep == NULL) {
> + bits = 128;
> + goto out;
> + }
> +
> + if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> +
> + errno = 0;
> + lbits = strtol(sep, &ep, 10);
> + if (sep[0] == '\0' || *ep != '\0') {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> + if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
> + || (lbits > 128 || lbits < 0)) {
> + __set_errno (EMSGSIZE);
> + return (-1);
> + }
> + bits = lbits;
> +
> + out:
> + bytes = (bits + 7) / 8;
> + if (bytes > size) {
> + __set_errno (EMSGSIZE);
> + return (-1);
> + }
> +
> + memcpy(dst, &in6.s6_addr, bytes);
> + return (bits);
> +}
--
<http://www.alejandro-colomar.es/>
On 12/22/22 19:28, Alejandro Colomar wrote:>> 1) Use strncpy() instead of strlcpy()
>
> Would someone please add a function to glibc that truncates a string, while
> still producing a string (as opposed to a null-padded fixed-width character
> sequence)?
>
> Here goes an extract of the yet-unreleased strncpy(3) manual page from the Linux
> man-pages master branch:
>
>
> DESCRIPTION
> These functions copy the string pointed to by src into a null‐padded
> character sequence at the fixed‐width buffer pointed to by dst. If the
> destination buffer, limited by its size, isn’t large enough to hold the
> copy, the resulting character sequence is truncated. For the differ‐
> ence between the two functions, see RETURN VALUE.
>
> An implementation of these functions might be:
>
> char *
> stpncpy(char *restrict dst, const char *restrict src, size_t sz)
> {
> bzero(dst, sz);
> return mempcpy(dst, src, strnlen(src, sz));
> }
>
> char *
> strncpy(char *restrict dst, const char *restrict src, size_t sz)
> {
> stpncpy(dst, src, sz);
> return dst;
> }
>
> [...]
>
> CAVEATS
> The name of these functions is confusing. These functions produce a
> null‐padded character sequence, not a string (see string_copying(7)).
>
> It’s impossible to distinguish truncation by the result of the call,
> from a character sequence that just fits the destination buffer; trun‐
> cation should be detected by comparing the length of the input string
> with the size of the destination buffer.
>
> I'll be releasing the a new man-pages version very soon (a week at most), so
> that this page and also the new string_copying(7) overview are widely available.
I released a moment ago. So, I'd suggest either adding strlcpy(3)&strlcat(3) to
glibc, or stpecpy(3) (defined here:
<http://www.alejandro-colomar.es/src/alx/alx/libstp.git/tree/src/stp/stpe/stpecpy.c>).
Or even both, since each of them serves a purpose: strlcpy(3)&strlcat(3) are
for simpler code where performance and truncation are not a concern, and
stpecpy(3) does the same but faster with just a few more bytes of code (and
detecting truncation is even simpler).
I'll send an actual patch for adding stpecpy(3), to discuss with some actual code.
Cheers,
Alex
--
<http://www.alejandro-colomar.es/>
> On 22 Dec 2022, at 20:25, Alejandro Colomar via Libc-alpha <libc-alpha@sourceware.org> wrote:
>
> On 12/22/22 19:28, Alejandro Colomar wrote:>> 1) Use strncpy() instead of strlcpy()
>> Would someone please add a function to glibc that truncates a string, while still producing a string (as opposed to a null-padded fixed-width character sequence)?
>> Here goes an extract of the yet-unreleased strncpy(3) manual page from the Linux man-pages master branch:
>> DESCRIPTION
>> These functions copy the string pointed to by src into a null‐padded
>> character sequence at the fixed‐width buffer pointed to by dst. If the
>> destination buffer, limited by its size, isn’t large enough to hold the
>> copy, the resulting character sequence is truncated. For the differ‐
>> ence between the two functions, see RETURN VALUE.
>> An implementation of these functions might be:
>> char *
>> stpncpy(char *restrict dst, const char *restrict src, size_t sz)
>> {
>> bzero(dst, sz);
>> return mempcpy(dst, src, strnlen(src, sz));
>> }
>> char *
>> strncpy(char *restrict dst, const char *restrict src, size_t sz)
>> {
>> stpncpy(dst, src, sz);
>> return dst;
>> }
>> [...]
>> CAVEATS
>> The name of these functions is confusing. These functions produce a
>> null‐padded character sequence, not a string (see string_copying(7)).
>> It’s impossible to distinguish truncation by the result of the call,
>> from a character sequence that just fits the destination buffer; trun‐
>> cation should be detected by comparing the length of the input string
>> with the size of the destination buffer.
>> I'll be releasing the a new man-pages version very soon (a week at most), so that this page and also the new string_copying(7) overview are widely available.
>
> I released a moment ago. So, I'd suggest either adding strlcpy(3)&strlcat(3) to glibc, or stpecpy(3) (defined here: <http://www.alejandro-colomar.es/src/alx/alx/libstp.git/tree/src/stp/stpe/stpecpy.c>). Or even both, since each of them serves a purpose: strlcpy(3)&strlcat(3) are for simpler code where performance and truncation are not a concern, and stpecpy(3) does the same but faster with just a few more bytes of code (and detecting truncation is even simpler).
>
strlcpy and strlcat is in POSIX next (https://www.austingroupbugs.net/view.php?id=986) and will be in glibc
in due course, I believe.
See https://sourceware.org/bugzilla/show_bug.cgi?id=178. Florian sent a patch a few months ago
but I don't think anything happened with it yet (https://patchwork.sourceware.org/project/glibc/patch/87fsjp7rqz.fsf@oldenburg.str.redhat.com/).
I'll update a bug with this new information, as well.
Best,
sam
> On 23 Dec 2022, at 06:55, Sam James via Libc-alpha <libc-alpha@sourceware.org> wrote:
> strlcpy and strlcat is in POSIX next (https://www.austingroupbugs.net/view.php?id=986) and will be in glibc
> in due course, I believe.
>
> See https://sourceware.org/bugzilla/show_bug.cgi?id=178. Florian sent a patch a few months ago
> but I don't think anything happened with it yet (https://patchwork.sourceware.org/project/glibc/patch/87fsjp7rqz.fsf@oldenburg.str.redhat.com/).
>
Ah, I didn't read over the discussion again as I thought I'd remembered it all,
but since then, Paul's objection bug w/ austingroup has been closed, so I don't see
a reason not to move forward.
Hey Sam!
On 12/23/22 08:00, Sam James wrote:
>
>
>> On 23 Dec 2022, at 06:55, Sam James via Libc-alpha <libc-alpha@sourceware.org> wrote:
>
>> strlcpy and strlcat is in POSIX next (https://www.austingroupbugs.net/view.php?id=986) and will be in glibc
>> in due course, I believe.
>>
>> See https://sourceware.org/bugzilla/show_bug.cgi?id=178. Florian sent a patch a few months ago
>> but I don't think anything happened with it yet (https://patchwork.sourceware.org/project/glibc/patch/87fsjp7rqz.fsf@oldenburg.str.redhat.com/).
>>
> Ah, I didn't read over the discussion again as I thought I'd remembered it all,
> but since then, Paul's objection bug w/ austingroup has been closed, so I don't see
> a reason not to move forward.
Ahh, good then. BTW, could you please link to Paul's objection? I couldn't
find it. Anyway, I'll CC him.
I'll reply on the other thread, since I still think we need the other function.
Cheers,
Alex
P.S.: I think you may have libc-alpha as 'Zack Weinberg via Libc-alpha' on you
address book? Or maybe it's my thunderbird? I think it's not me.
--
<http://www.alejandro-colomar.es/>
Oops, I accidentally CCd the wrong Paul. I intended to CC Eggert :P Added.
On 12/23/22 12:42, Alejandro Colomar wrote:
> Hey Sam!
>
> On 12/23/22 08:00, Sam James wrote:
>>
>>
>>> On 23 Dec 2022, at 06:55, Sam James via Libc-alpha
>>> <libc-alpha@sourceware.org> wrote:
>>
>>> strlcpy and strlcat is in POSIX next
>>> (https://www.austingroupbugs.net/view.php?id=986) and will be in glibc
>>> in due course, I believe.
>>>
>>> See https://sourceware.org/bugzilla/show_bug.cgi?id=178. Florian sent a patch
>>> a few months ago
>>> but I don't think anything happened with it yet
>>> (https://patchwork.sourceware.org/project/glibc/patch/87fsjp7rqz.fsf@oldenburg.str.redhat.com/).
>>>
>> Ah, I didn't read over the discussion again as I thought I'd remembered it all,
>> but since then, Paul's objection bug w/ austingroup has been closed, so I
>> don't see
>> a reason not to move forward.
>
> Ahh, good then. BTW, could you please link to Paul's objection? I couldn't
> find it. Anyway, I'll CC him.
>
> I'll reply on the other thread, since I still think we need the other function.
>
> Cheers,
>
> Alex
>
> P.S.: I think you may have libc-alpha as 'Zack Weinberg via Libc-alpha' on you
> address book? Or maybe it's my thunderbird? I think it's not me.
>
>
--
<http://www.alejandro-colomar.es/>
> On 23 Dec 2022, at 11:42, Alejandro Colomar <alx.manpages@gmail.com> wrote:
>
> Hey Sam!
>
> On 12/23/22 08:00, Sam James wrote:
>>> On 23 Dec 2022, at 06:55, Sam James via Libc-alpha <libc-alpha@sourceware.org> wrote:
>>> strlcpy and strlcat is in POSIX next (https://www.austingroupbugs.net/view.php?id=986) and will be in glibc
>>> in due course, I believe.
>>>
>>> See https://sourceware.org/bugzilla/show_bug.cgi?id=178. Florian sent a patch a few months ago
>>> but I don't think anything happened with it yet (https://patchwork.sourceware.org/project/glibc/patch/87fsjp7rqz.fsf@oldenburg.str.redhat.com/).
>>>
>> Ah, I didn't read over the discussion again as I thought I'd remembered it all,
>> but since then, Paul's objection bug w/ austingroup has been closed, so I don't see
>> a reason not to move forward.
>
> Ahh, good then. BTW, could you please link to Paul's objection? I couldn't find it. Anyway, I'll CC him.
https://austingroupbugs.net/view.php?id=1591
>
> I'll reply on the other thread, since I still think we need the other function.
>
> Cheers,
>
> Alex
>
> P.S.: I think you may have libc-alpha as 'Zack Weinberg via Libc-alpha' on you address book? Or maybe it's my thunderbird? I think it's not me.
>
Oops! I'll fix. It was automatic.
Dear all,
This is a refresh of a diff I proposed back in December 2022 to add IPv6
support to inet_net_pton().
The starting point of this changeset was OpenBSD's
libc/net/inet_net_pton.c (r1.13) implementation of inet_net_pton_ipv6().
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/net/inet_net_pton.c?annotate=1.13
The OpenBSD implementation was adapted to glibc as following:
1) Use strtol() instead of strtonum()
2) Updated comments
I've tested the changeset on Debian Bookworm.
Your feedback is appreciated!
Kind regards,
Job
Signed-off: Job Snijders <job@fastly.com>
---
resolv/inet_net_pton.c | 78 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 74 insertions(+), 4 deletions(-)
diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
index 63a47b7394..27e121c0cb 100644
--- a/resolv/inet_net_pton.c
+++ b/resolv/inet_net_pton.c
@@ -1,4 +1,6 @@
/*
+ * Copyright (c) 2024 Job Snijders <job@fastly.com>
+ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
* Copyright (c) 1996,1999 by Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
@@ -35,13 +37,16 @@
static int inet_net_pton_ipv4 (const char *src, u_char *dst,
size_t size) __THROW;
+static int inet_net_pton_ipv6 (const char *src, u_char *dst,
+ size_t size) __THROW;
/*
- * static int
+ * int
* inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * Convert network number from presentation format to network format.
+ * If "af" is set to AF_INET, accept various formats like hex octets,
+ * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
+ * IPv6 addresses. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
@@ -55,6 +60,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
switch (af) {
case AF_INET:
return (inet_net_pton_ipv4(src, dst, size));
+ case AF_INET6:
+ return (inet_net_pton_ipv6(src, dst, size));
default:
__set_errno (EAFNOSUPPORT);
return (-1);
@@ -196,3 +203,66 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
__set_errno (EMSGSIZE);
return (-1);
}
+
+
+/*
+ * Convert an IPv6 prefix from presentation format to network format.
+ * Return the number of bits specified, or -1 as error (check errno).
+ */
+static int
+inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
+{
+ struct in6_addr in6;
+ int bits;
+ long lbits;
+ size_t bytes;
+ char buf[INET6_ADDRSTRLEN + sizeof("/128")];
+ char *ep, *sep;
+
+ if (strlcpy(buf, src, sizeof(buf)) >= sizeof(buf)) {
+ errno = EMSGSIZE;
+ return (-1);
+ }
+
+ sep = strchr(buf, '/');
+ if (sep != NULL)
+ *sep++ = '\0';
+
+ if (inet_pton(AF_INET6, buf, &in6) != 1) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ if (sep == NULL) {
+ bits = 128;
+ goto out;
+ }
+
+ if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ errno = 0;
+ lbits = strtol(sep, &ep, 10);
+ if (sep[0] == '\0' || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
+ || (lbits > 128 || lbits < 0)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ bits = lbits;
+
+ out:
+ bytes = (bits + 7) / 8;
+ if (bytes > size) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+
+ memcpy(dst, &in6.s6_addr, bytes);
+ return (bits);
+}
Dear all,
Slight tweaks compared to what I sent out earlier today:
* for consistency use the __set_errno macro to set errno
* preserve errno unless setting it explicitly (avoid errno stomping)
Original message:
On Sun, Mar 17, 2024 at 01:23:04AM +0000, Job Snijders wrote:
> This is a refresh of a diff I proposed back in December 2022 to add IPv6
> support to inet_net_pton().
>
> The starting point of this changeset was OpenBSD's
> libc/net/inet_net_pton.c (r1.13) implementation of inet_net_pton_ipv6().
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/net/inet_net_pton.c?annotate=1.13
>
> The OpenBSD implementation was adapted to glibc as following:
>
> 1) Use strtol() instead of strtonum()
> 2) Updated comments
>
> I've tested the changeset on Debian Bookworm.
>
> Your feedback is appreciated!
Kind regards,
Job
Signed-off: Job Snijders <job@fastly.com>
---
resolv/inet_net_pton.c | 80 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 76 insertions(+), 4 deletions(-)
diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
index 63a47b7394..217e18263c 100644
--- a/resolv/inet_net_pton.c
+++ b/resolv/inet_net_pton.c
@@ -1,4 +1,6 @@
/*
+ * Copyright (c) 2024 Job Snijders <job@fastly.com>
+ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
* Copyright (c) 1996,1999 by Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
@@ -35,13 +37,16 @@
static int inet_net_pton_ipv4 (const char *src, u_char *dst,
size_t size) __THROW;
+static int inet_net_pton_ipv6 (const char *src, u_char *dst,
+ size_t size) __THROW;
/*
- * static int
+ * int
* inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * Convert network number from presentation format to network format.
+ * If "af" is set to AF_INET, accept various formats like hex octets,
+ * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
+ * IPv6 addresses. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
@@ -55,6 +60,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
switch (af) {
case AF_INET:
return (inet_net_pton_ipv4(src, dst, size));
+ case AF_INET6:
+ return (inet_net_pton_ipv6(src, dst, size));
default:
__set_errno (EAFNOSUPPORT);
return (-1);
@@ -196,3 +203,68 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
__set_errno (EMSGSIZE);
return (-1);
}
+
+
+/*
+ * Convert an IPv6 prefix from presentation format to network format.
+ * Return the number of bits specified, or -1 as error (check errno).
+ */
+static int
+inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
+{
+ struct in6_addr in6;
+ int bits, save_errno;
+ long lbits;
+ size_t bytes;
+ char buf[INET6_ADDRSTRLEN + sizeof("/128")];
+ char *ep, *sep;
+
+ save_errno = errno;
+
+ if (strlcpy(buf, src, sizeof(buf)) >= sizeof(buf)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+
+ sep = strchr(buf, '/');
+ if (sep != NULL)
+ *sep++ = '\0';
+
+ if (inet_pton(AF_INET6, buf, &in6) != 1) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ if (sep == NULL) {
+ bits = 128;
+ goto out;
+ }
+
+ if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ __set_errno (0);
+ lbits = strtol(sep, &ep, 10);
+ if (sep[0] == '\0' || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
+ || (lbits > 128 || lbits < 0)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ bits = lbits;
+
+ out:
+ bytes = (bits + 7) / 8;
+ if (bytes > size) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ __set_errno (save_errno);
+ memcpy(dst, &in6.s6_addr, bytes);
+ return (bits);
+}
* Job Snijders:
> +/*
> + * Convert an IPv6 prefix from presentation format to network format.
> + * Return the number of bits specified, or -1 as error (check errno).
> + */
> +static int
> +inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
> +{
> + struct in6_addr in6;
> + int bits, save_errno;
> + long lbits;
> + size_t bytes;
> + char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> + char *ep, *sep;
> +
> + save_errno = errno;
> +
> + if (strlcpy(buf, src, sizeof(buf)) >= sizeof(buf)) {
> + __set_errno (EMSGSIZE);
> + return (-1);
> + }
> +
> + sep = strchr(buf, '/');
> + if (sep != NULL)
> + *sep++ = '\0';
> +
> + if (inet_pton(AF_INET6, buf, &in6) != 1) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
I think you can use __inet_pton_length here. Then you won't need to
make a copy.
Thanks,
Florian
On Sun, Mar 17, 2024 at 12:18:14PM +0100, Florian Weimer wrote:
> * Job Snijders:
> > +/*
> > + * Convert an IPv6 prefix from presentation format to network format.
> > + * Return the number of bits specified, or -1 as error (check errno).
> > + */
> > +static int
> > +inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
> > +{
> > + struct in6_addr in6;
> > + int bits, save_errno;
> > + long lbits;
> > + size_t bytes;
> > + char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> > + char *ep, *sep;
> > +
> > + save_errno = errno;
> > +
> > + if (strlcpy(buf, src, sizeof(buf)) >= sizeof(buf)) {
> > + __set_errno (EMSGSIZE);
> > + return (-1);
> > + }
> > +
> > + sep = strchr(buf, '/');
> > + if (sep != NULL)
> > + *sep++ = '\0';
> > +
> > + if (inet_pton(AF_INET6, buf, &in6) != 1) {
> > + __set_errno (ENOENT);
> > + return (-1);
> > + }
>
> I think you can use __inet_pton_length here. Then you won't need to
> make a copy.
You mean the copy into char buf[]?
Perhaps something along the lines of the following?
Kind regards,
Job
Signed-off: Job Snijders <job@fastly.com>
---
resolv/inet_net_pton.c | 74 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 4 deletions(-)
diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
index 63a47b7394..6f10142215 100644
--- a/resolv/inet_net_pton.c
+++ b/resolv/inet_net_pton.c
@@ -1,4 +1,6 @@
/*
+ * Copyright (c) 2024 Job Snijders <job@fastly.com>
+ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
* Copyright (c) 1996,1999 by Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
@@ -19,6 +21,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <resolv/resolv-internal.h>
#include <assert.h>
#include <ctype.h>
@@ -35,13 +38,16 @@
static int inet_net_pton_ipv4 (const char *src, u_char *dst,
size_t size) __THROW;
+static int inet_net_pton_ipv6 (const char *src, u_char *dst,
+ size_t size) __THROW;
/*
- * static int
+ * int
* inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * Convert network number from presentation format to network format.
+ * If "af" is set to AF_INET, accept various formats like hex octets,
+ * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
+ * IPv6 addresses. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
@@ -55,6 +61,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
switch (af) {
case AF_INET:
return (inet_net_pton_ipv4(src, dst, size));
+ case AF_INET6:
+ return (inet_net_pton_ipv6(src, dst, size));
default:
__set_errno (EAFNOSUPPORT);
return (-1);
@@ -196,3 +204,61 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
__set_errno (EMSGSIZE);
return (-1);
}
+
+
+/*
+ * Convert an IPv6 prefix from presentation format to network format.
+ * Return the number of bits specified, or -1 as error (check errno).
+ */
+static int
+inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
+{
+ struct in6_addr in6;
+ int bits, save_errno;
+ long lbits;
+ size_t bytes;
+ char *ep, *sep;
+
+ save_errno = errno;
+
+ sep = strchr(src, '/');
+
+ if (__inet_pton_length(AF_INET6, src, sep ? sep - src : strlen(src),
+ &in6) != 1) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ if (sep == NULL) {
+ bits = 128;
+ goto out;
+ }
+
+ if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ __set_errno (0);
+ lbits = strtol(sep, &ep, 10);
+ if (sep[0] == '\0' || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
+ || (lbits > 128 || lbits < 0)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ bits = lbits;
+
+ out:
+ bytes = (bits + 7) / 8;
+ if (bytes > size) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ __set_errno (save_errno);
+ memcpy(dst, &in6.s6_addr, bytes);
+ return (bits);
+}
On Mär 18 2024, Job Snijders wrote:
> + __set_errno (0);
> + lbits = strtol(sep, &ep, 10);
> + if (sep[0] == '\0' || *ep != '\0') {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> + if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
> + || (lbits > 128 || lbits < 0)) {
> + __set_errno (EMSGSIZE);
> + return (-1);
I think the first half of the error check is redundant since we only
accept values in the range [0,128] anyway.
On Mon, Mar 18, 2024 at 10:23:27AM +0100, Andreas Schwab wrote:
> On Mär 18 2024, Job Snijders wrote:
> > + __set_errno (0);
> > + lbits = strtol(sep, &ep, 10);
> > + if (sep[0] == '\0' || *ep != '\0') {
> > + __set_errno (ENOENT);
> > + return (-1);
> > + }
> > + if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
> > + || (lbits > 128 || lbits < 0)) {
> > + __set_errno (EMSGSIZE);
> > + return (-1);
>
> I think the first half of the error check is redundant since we only
> accept values in the range [0,128] anyway.
This is an idiomatic error check. The compiler can optimize parts of
it, if the compiler feels they are not not neccessary. An argument
in support of idiomatic checks is that someone can visually inspect,
recognize, and know no checks were missed.
I'd like it if this libc implementation had strtonum() support like
described here: https://flak.tedunangst.com/post/the-design-of-strtonum
Would the project appreciate a patch?
Kind regards,
Job
On Mär 18 2024, Job Snijders wrote:
> On Mon, Mar 18, 2024 at 10:23:27AM +0100, Andreas Schwab wrote:
>> On Mär 18 2024, Job Snijders wrote:
>> > + __set_errno (0);
>> > + lbits = strtol(sep, &ep, 10);
>> > + if (sep[0] == '\0' || *ep != '\0') {
>> > + __set_errno (ENOENT);
>> > + return (-1);
>> > + }
>> > + if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
>> > + || (lbits > 128 || lbits < 0)) {
>> > + __set_errno (EMSGSIZE);
>> > + return (-1);
>>
>> I think the first half of the error check is redundant since we only
>> accept values in the range [0,128] anyway.
>
> This is an idiomatic error check. The compiler can optimize parts of
> it, if the compiler feels they are not not neccessary.
But it unnecessarily modifies errno (which the compiler cannot optimize
out), and sets it to zero when no error occurs, which no library routine
must ever do.
On Tue, 19 Mar 2024 at 18:21, Andreas Schwab <schwab@suse.de> wrote:
> On Mär 18 2024, Job Snijders wrote:
>
> > On Mon, Mar 18, 2024 at 10:23:27AM +0100, Andreas Schwab wrote:
> >> On Mär 18 2024, Job Snijders wrote:
> >> > + __set_errno (0);
> >> > + lbits = strtol(sep, &ep, 10);
> >> > + if (sep[0] == '\0' || *ep != '\0') {
> >> > + __set_errno (ENOENT);
> >> > + return (-1);
> >> > + }
> >> > + if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
> >> > + || (lbits > 128 || lbits < 0)) {
> >> > + __set_errno (EMSGSIZE);
> >> > + return (-1);
> >>
> >> I think the first half of the error check is redundant since we only
> >> accept values in the range [0,128] anyway.
> >
> > This is an idiomatic error check. The compiler can optimize parts of
> > it, if the compiler feels they are not not neccessary.
>
> But it unnecessarily modifies errno (which the compiler cannot optimize
> out), and sets it to zero when no error occurs, which no library routine
> must ever do.
The proposal at hand does save errno at the start and restores it at the
end to avoid unnecessary stomping on errno (if nothing bad happened). I’m
not entirely sure how removing the idiomatic check helps in that regard? I
might be missing something.
Kind regards,
Job
>
On Mär 19 2024, Job Snijders wrote:
> The proposal at hand does save errno at the start and restores it at the
> end to avoid unnecessary stomping on errno (if nothing bad happened).
Sorry, I missed that.
> I’m not entirely sure how removing the idiomatic check helps in that
> regard? I might be missing something.
It's still unnecessary code that makes it harder to understand, which is
bad.
On Tue, Mar 19, 2024 at 10:50:14AM +0100, Andreas Schwab wrote:
> On Mär 19 2024, Job Snijders wrote:
>
> > The proposal at hand does save errno at the start and restores it at the
> > end to avoid unnecessary stomping on errno (if nothing bad happened).
>
> Sorry, I missed that.
>
> > I’m not entirely sure how removing the idiomatic check helps in that
> > regard? I might be missing something.
>
> It's still unnecessary code that makes it harder to understand, which is
> bad.
From POSIX's strtol() description:
"If the correct value is outside the range of representable values,
LONG_MAX or LONG_MIN is returned (according to the sign of the value),
and errno is set to [ERANGE]."
https://pubs.opengroup.org/onlinepubs/7908799/xsh/strtol.html
The above to me means that it is not enough to just check whether errno
is set to ERANGE, but one also has to check whether LONG_MAX or LONG_MIN
were returned. It's an 'AND' operation.
Then, if errno is NOT set to ERANGE, then we have to additionally check
whether the value is within contextual bounds.
So I don't think it is correct to skimp on checks here, to me idiomatic
checks do not equate 'bad'.
I'm not here to argue, please take from the patch whatever you want, I
just want glibc inet_net_pton() to finally have IPv6 support.
Kind regards,
Job
On Fri, Mar 22, 2024, at 12:16 AM, Job Snijders wrote:
> On Tue, Mar 19, 2024 at 10:50:14AM +0100, Andreas Schwab wrote:
>> It's still unnecessary code that makes it harder to understand, which is
>> bad.
This seems to be the crux of the disagreement - Andreas says this code
is longer than necessary and that makes it harder to understand, Job
says the extra code makes it more idiomatic and therefore *easier* to
understand. Yes?
> "If the correct value is outside the range of representable values,
> LONG_MAX or LONG_MIN is returned (according to the sign of the value),
> and errno is set to [ERANGE]."
> https://pubs.opengroup.org/onlinepubs/7908799/xsh/strtol.html
>
> The above to me means that it is not enough to just check whether errno
> is set to ERANGE, but one also has to check whether LONG_MAX or LONG_MIN
> were returned. It's an 'AND' operation.
Not quite. POSIX specs are written as directives to the implementor.
A better way to read this is: If the correct values is outside the
representable range, strtol does two things: it sets errno to ERANGE,
and it returns the representable value farthest away from zero,
with the appropriate sign.
Note also this sentence
"Because 0, LONG_MIN and LONG_MAX are returned on error and are
also valid returns on success, an application wishing to check
for error situations should set errno to 0, then call strtol(),
then check errno."
This implicitly makes a guarantee that strtol *will not* set errno
to a nonzero value unless one of the specified error conditions
occurs. (Most C library functions are permitted to set errno nonzero
even if they succeed. No C library function is ever allowed to set
errno to zero.) That means it's safe to look at errno *before*
looking at the return value, as the code under discussion does.
> Then, if errno is NOT set to ERANGE, then we have to additionally
> check whether the value is within contextual bounds.
Here I agree with Andreas. If there are contextual bounds that are
a strict subset of the representable range (i.e. neither LONG_MAX
nor LONG_MIN is contextually valid) then it *doesn't matter* whether
LONG_MAX or LONG_MIN was actually the input value or whether it was
produced as a result of overflow; it's contextually invalid either
way, so the contextual bounds check subsumes the overflow check.
> So I don't think it is correct to skimp on checks here, to me
> idiomatic checks do not equate 'bad'.
Let's recap - the code under discussion is
+ save_errno = errno;
...
+ __set_errno (0);
+ lbits = strtol(sep, &ep, 10);
+ if (sep[0] == '\0' || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
+ || (lbits > 128 || lbits < 0)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
...
+ __set_errno (save_errno);
Tangentially, "sep[0] == '\0'" is not the conventional way to check
for strtol not having advanced ep at all. It *works*, but only
because "*ep == '\0'" catches all the cases it misses, and I didn't
see that until *after* I started writing this paragraph, which
originally would have claimed that this was an outright bug. Please
change that to "ep == sep", even if you make no other changes.
Anyway, I would have written this as
lbits = strtol(sep, &ep, 10);
if (ep == sep || *ep != '\0') {
__set_errno(ENOENT);
return -1;
}
if (lbits < 0 || lbits > 128) {
__set_errno(EMSGSIZE);
return -1;
}
and I do think this is an improvement over what you had, largely
because it eliminates the need to save and restore the original value
of errno. inet_pton *is* allowed to set errno nonzero despite having
succeeded, the save/restore was only necessary because of internally
setting it to *zero*, and if we're not going to look at the errno
return from strtol then we don't need that.
It *does* mean future readers have to think for a moment to confirm
that one of the three cases where strtol might set errno is impossible
(invalid base, 10 is statically valid) and the other two are covered
by the conditions below (no characters consumed -> ep == sep, overflow
-> outside the range [0, 128]). But in this case all the numbers
involved are small enough that the extra cognitive load is reasonable.
If it was "lbits > 4294967242" then I would probably include a comment
to the effect that 4294967242 is less than the minimum value of
LONG_MAX and therefore the input overflow check is subsumed.
zw
On Mär 22 2024, Job Snijders wrote:
> The above to me means that it is not enough to just check whether errno
> is set to ERANGE, but one also has to check whether LONG_MAX or LONG_MIN
> were returned. It's an 'AND' operation.
That is irrelevant.
> Then, if errno is NOT set to ERANGE, then we have to additionally check
> whether the value is within contextual bounds.
By checking the range you already know that the value is neither
LONG_MAX nor LONG_MIN, thus the errno value does not matter. This is
simple logic.
> So I don't think it is correct to skimp on checks here, to me idiomatic
> checks do not equate 'bad'.
A source analyzer will likely flag it, which makes it even worse.
Dear Zack,
On Fri, Mar 22, 2024 at 10:24:35AM -0400, Zack Weinberg wrote:
> Please change that to "ep == sep", even if you make no other changes.
>
> Anyway, I would have written this as
>
> lbits = strtol(sep, &ep, 10);
> if (ep == sep || *ep != '\0') {
> __set_errno(ENOENT);
> return -1;
> }
> if (lbits < 0 || lbits > 128) {
> __set_errno(EMSGSIZE);
> return -1;
> }
>
> and I do think this is an improvement over what you had, largely
> because it eliminates the need to save and restore the original value
> of errno.
Thank you for your response, I've adjusted the proposal according to
your notes.
Kind regards,
Job
Signed-off: Job Snijders <job@fastly.com>
---
resolv/inet_net_pton.c | 69 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
index 63a47b7394..93753cb37f 100644
--- a/resolv/inet_net_pton.c
+++ b/resolv/inet_net_pton.c
@@ -1,4 +1,6 @@
/*
+ * Copyright (c) 2024 Job Snijders <job@fastly.com>
+ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
* Copyright (c) 1996,1999 by Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
@@ -19,6 +21,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <resolv/resolv-internal.h>
#include <assert.h>
#include <ctype.h>
@@ -35,13 +38,16 @@
static int inet_net_pton_ipv4 (const char *src, u_char *dst,
size_t size) __THROW;
+static int inet_net_pton_ipv6 (const char *src, u_char *dst,
+ size_t size) __THROW;
/*
- * static int
+ * int
* inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * Convert network number from presentation format to network format.
+ * If "af" is set to AF_INET, accept various formats like hex octets,
+ * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
+ * IPv6 addresses. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
@@ -55,6 +61,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
switch (af) {
case AF_INET:
return (inet_net_pton_ipv4(src, dst, size));
+ case AF_INET6:
+ return (inet_net_pton_ipv6(src, dst, size));
default:
__set_errno (EAFNOSUPPORT);
return (-1);
@@ -196,3 +204,56 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
__set_errno (EMSGSIZE);
return (-1);
}
+
+
+/*
+ * Convert an IPv6 prefix from presentation format to network format.
+ * Return the number of bits specified, or -1 as error (check errno).
+ */
+static int
+inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
+{
+ struct in6_addr in6;
+ int bits;
+ long lbits;
+ size_t bytes;
+ char *ep, *sep;
+
+ sep = strchr(src, '/');
+
+ if (__inet_pton_length(AF_INET6, src, sep ? sep - src : strlen(src),
+ &in6) != 1) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ if (sep == NULL) {
+ bits = 128;
+ goto out;
+ }
+
+ if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ lbits = strtol(sep, &ep, 10);
+ if (ep == sep || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if (lbits < 0 || lbits > 128) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ bits = lbits;
+
+ out:
+ bytes = (bits + 7) / 8;
+ if (bytes > size) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ memcpy(dst, &in6.s6_addr, bytes);
+ return (bits);
+}
Dear all,
Are there any other comments, or can this be considered for merging? :-)
Kind regards,
Job
On Mon, Mar 25, 2024 at 09:04:52AM +0000, Job Snijders wrote:
> Dear Zack,
>
> On Fri, Mar 22, 2024 at 10:24:35AM -0400, Zack Weinberg wrote:
> > Please change that to "ep == sep", even if you make no other changes.
> >
> > Anyway, I would have written this as
> >
> > lbits = strtol(sep, &ep, 10);
> > if (ep == sep || *ep != '\0') {
> > __set_errno(ENOENT);
> > return -1;
> > }
> > if (lbits < 0 || lbits > 128) {
> > __set_errno(EMSGSIZE);
> > return -1;
> > }
> >
> > and I do think this is an improvement over what you had, largely
> > because it eliminates the need to save and restore the original value
> > of errno.
>
> Thank you for your response, I've adjusted the proposal according to
> your notes.
>
> Kind regards,
>
> Job
>
> Signed-off: Job Snijders <job@fastly.com>
> ---
> resolv/inet_net_pton.c | 69 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
> index 63a47b7394..93753cb37f 100644
> --- a/resolv/inet_net_pton.c
> +++ b/resolv/inet_net_pton.c
> @@ -1,4 +1,6 @@
> /*
> + * Copyright (c) 2024 Job Snijders <job@fastly.com>
> + * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
> * Copyright (c) 1996,1999 by Internet Software Consortium.
> *
> * Permission to use, copy, modify, and distribute this software for any
> @@ -19,6 +21,7 @@
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> +#include <resolv/resolv-internal.h>
>
> #include <assert.h>
> #include <ctype.h>
> @@ -35,13 +38,16 @@
>
> static int inet_net_pton_ipv4 (const char *src, u_char *dst,
> size_t size) __THROW;
> +static int inet_net_pton_ipv6 (const char *src, u_char *dst,
> + size_t size) __THROW;
>
> /*
> - * static int
> + * int
> * inet_net_pton(af, src, dst, size)
> - * convert network number from presentation to network format.
> - * accepts hex octets, hex strings, decimal octets, and /CIDR.
> - * "size" is in bytes and describes "dst".
> + * Convert network number from presentation format to network format.
> + * If "af" is set to AF_INET, accept various formats like hex octets,
> + * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
> + * IPv6 addresses. "size" is in bytes and describes "dst".
> * return:
> * number of bits, either imputed classfully or specified with /CIDR,
> * or -1 if some failure occurred (check errno). ENOENT means it was
> @@ -55,6 +61,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
> switch (af) {
> case AF_INET:
> return (inet_net_pton_ipv4(src, dst, size));
> + case AF_INET6:
> + return (inet_net_pton_ipv6(src, dst, size));
> default:
> __set_errno (EAFNOSUPPORT);
> return (-1);
> @@ -196,3 +204,56 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
> __set_errno (EMSGSIZE);
> return (-1);
> }
> +
> +
> +/*
> + * Convert an IPv6 prefix from presentation format to network format.
> + * Return the number of bits specified, or -1 as error (check errno).
> + */
> +static int
> +inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
> +{
> + struct in6_addr in6;
> + int bits;
> + long lbits;
> + size_t bytes;
> + char *ep, *sep;
> +
> + sep = strchr(src, '/');
> +
> + if (__inet_pton_length(AF_INET6, src, sep ? sep - src : strlen(src),
> + &in6) != 1) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> +
> + if (sep == NULL) {
> + bits = 128;
> + goto out;
> + }
> +
> + if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> +
> + lbits = strtol(sep, &ep, 10);
> + if (ep == sep || *ep != '\0') {
> + __set_errno (ENOENT);
> + return (-1);
> + }
> + if (lbits < 0 || lbits > 128) {
> + __set_errno (EMSGSIZE);
> + return (-1);
> + }
> + bits = lbits;
> +
> + out:
> + bytes = (bits + 7) / 8;
> + if (bytes > size) {
> + __set_errno (EMSGSIZE);
> + return (-1);
> + }
> + memcpy(dst, &in6.s6_addr, bytes);
> + return (bits);
> +}
On Sun, 2024-04-14 at 16:56 +0200, Job Snijders wrote:
> Dear all,
>
> Are there any other comments, or can this be considered for merging?
Send the updated patch as [PATCH v2] instead of just a follow up. Or
patchwork won't track the updated version and people are unlikely to
ever notice it.
@@ -1,4 +1,6 @@
/*
+ * Copyright (c) 2022 Job Snijders <job@fastly.com>
+ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
* Copyright (c) 1996,1999 by Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
@@ -35,13 +37,16 @@
static int inet_net_pton_ipv4 (const char *src, u_char *dst,
size_t size) __THROW;
+static int inet_net_pton_ipv6 (const char *src, u_char *dst,
+ size_t size) __THROW;
/*
- * static int
+ * int
* inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * Convert network number from presentation format to network format.
+ * If "af" is set to AF_INET, accept various formats like hex octets,
+ * hex strings, or decimal octets. If "af" is set to AF_INET6, accept
+ * IPv6 addresses. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
@@ -55,6 +60,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
switch (af) {
case AF_INET:
return (inet_net_pton_ipv4(src, dst, size));
+ case AF_INET6:
+ return (inet_net_pton_ipv6(src, dst, size));
default:
__set_errno (EAFNOSUPPORT);
return (-1);
@@ -196,3 +203,64 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
__set_errno (EMSGSIZE);
return (-1);
}
+
+
+/*
+ * Convert an IPv6 prefix from presentation format to network format.
+ * Return the number of bits specified, or -1 as error (check errno).
+ */
+static int
+inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
+{
+ struct in6_addr in6;
+ int bits;
+ long lbits;
+ size_t bytes;
+ char buf[INET6_ADDRSTRLEN + sizeof("/128")];
+ char *ep, *sep;
+
+ strncpy(buf, src, sizeof(buf) - 1);
+ buf[sizeof(buf) - 1] = '\0';
+
+ sep = strchr(buf, '/');
+ if (sep != NULL)
+ *sep++ = '\0';
+
+ if (inet_pton(AF_INET6, buf, &in6) != 1) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ if (sep == NULL) {
+ bits = 128;
+ goto out;
+ }
+
+ if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+
+ errno = 0;
+ lbits = strtol(sep, &ep, 10);
+ if (sep[0] == '\0' || *ep != '\0') {
+ __set_errno (ENOENT);
+ return (-1);
+ }
+ if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
+ || (lbits > 128 || lbits < 0)) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+ bits = lbits;
+
+ out:
+ bytes = (bits + 7) / 8;
+ if (bytes > size) {
+ __set_errno (EMSGSIZE);
+ return (-1);
+ }
+
+ memcpy(dst, &in6.s6_addr, bytes);
+ return (bits);
+}