mailing list of musl libc
 help / color / mirror / code / Atom feed
* Re: inet_pton
       [not found] <CAKHv7pi-VhjkzOpUuz3RBachSPfPOHXN09G7rhM9uFtw4ri2-A@mail.gmail.com>
@ 2013-10-21  2:08 ` Rich Felker
  2013-10-21 19:59   ` inet_pton Paul Schutte
  0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2013-10-21  2:08 UTC (permalink / raw)
  To: musl; +Cc: Paul Schutte

It's not an annoyance at all, and your test cases may be useful for
writing a regression test, so I'm replying on-list. As you may have
noticed, I already fixed the first issue you reported, but the second
issue does remain. nsz has proposed a simple fix: at line 61,

-			if (s[j]!='.') return 0;
+			if (s[j]!='.' || brk<0) return 0;

I have not checked this yet but I suspect it's correct. Please let me
know if it works for you.

Rich


On Sun, Oct 20, 2013 at 09:35:32PM +0200, Paul Schutte wrote:
> Hi Rich,
> 
> I send this directly to you as I think I am starting to annoy the people on
> the list with this.
> 
> I have done some testing on this and here is the result:
> 
> 
> 
> proper
> before  : 0
> 1  :: 1
> 1  ::: 0
> 0  192.168.1.1 0
> 1  :192.168.1.1 0
> 1  ::192.168.1.1 1
> 1  :ffff:192.168.1.1 0
> 1  ::ffff:192.168.1.1 1
> 1  .192.168.1.1 0
> 0  :.192.168.1.1 0
> 0  ffff:c0a8:5e4 0
> 0  :ffff:c0a8:5e4 0
> 1  0:0:0:0:0:ffff:c0a8:5e4 1
> 1  0:0:0:0:ffff:c0a8:5e4 0
> 0  0::ffff:c0a8:5e4 1
> 1  ::0::ffff:c0a8:5e4 0
> 0  c0a8 0
> 0
> The following seems to produce the correct output:
> 
> --- a/musl/src/network/inet_pton.c
> +++ b/musl/src/network/inet_pton.c
> @@ -14,11 +14,11 @@
>         return -1;
>  }
> 
> -int inet_pton(int af, const char *restrict s, void *restrict a0)
> +int inet_pton(int af, const char *restrict s0, void *restrict a0)
>  {
>         uint16_t ip[8];
>         unsigned char *a = a0;
> -       const char *z;
> +       const char *z, *s = s0;
>         unsigned long x;
>         int i, j, v, d, brk=-1, need_v4=0;
> 
> @@ -36,7 +36,13 @@
>                 return -1;
>         }
> 
> -       if (s[0]==':' && s[1]==':') s++;
> +       if (s[0]==':') {
> +               if (s[1]==':') {
> +                       s++;
> +               } else {
> +                       return 0;
> +               }
> +       }
> 
>         for (i=0; ; i++, s+=j+1) {
>                 if (s[0]==':' && brk<0) {
> @@ -73,6 +79,9 @@
>                 *a++ = ip[j]>>8;
>                 *a++ = ip[j];
>         }
> +
> +       if (s==s0) return 0;
> +
>         if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0;
>         return 1;
> }
> 
> 
> I used the following to test:
> 
> #include <ctype.h>
> #include <netdb.h>
> #include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/socket.h>
> #include <sys/un.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> 
> void test(char *s) {
>         char buf[256];
>         int musl;
>         musl=inet_pton(AF_INET6, s, (void*)buf);
>         printf("%s=%d\n",s,musl);
> }
> 
> int main() {
> 
>         test(":");
>         test("::");
>         test(":::");
>         test("192.168.1.1");
>         test(":192.168.1.1");
>         test("::192.168.1.1");
>         test(":ffff:192.168.1.1");
>         test("::ffff:192.168.1.1");
>         test(".192.168.1.1");
>         test(":.192.168.1.1");
>         test("ffff:c0a8:5e4");
>         test(":ffff:c0a8:5e4");
>         test("0:0:0:0:0:ffff:c0a8:5e4");
>         test("0:0:0:0:ffff:c0a8:5e4");
>         test("0::ffff:c0a8:5e4");
>         test("::0::ffff:c0a8:5e4");
>         test("c0a8");
> 
>         return 0;
> }
> 
> The output is as follows which seems to be the desired outcome:
> :=0
> ::=1
> :::=0
> 192.168.1.1=0
> :192.168.1.1=0
> ::192.168.1.1=1
> :ffff:192.168.1.1=0
> ::ffff:192.168.1.1=1
> ..192.168.1.1=0
> :.192.168.1.1=0
> ffff:c0a8:5e4=0
> :ffff:c0a8:5e4=0
> 0:0:0:0:0:ffff:c0a8:5e4=1
> 0:0:0:0:ffff:c0a8:5e4=0
> 0::ffff:c0a8:5e4=1
> ::0::ffff:c0a8:5e4=0
> c0a8=0
> 
> 
> Hope I you find this use full.
> 
> Regards
> Paul


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: inet_pton
  2013-10-21  2:08 ` inet_pton Rich Felker
@ 2013-10-21 19:59   ` Paul Schutte
  2013-10-21 22:25     ` Szabolcs Nagy
  2013-10-25 15:44     ` Rich Felker
  0 siblings, 2 replies; 6+ messages in thread
From: Paul Schutte @ 2013-10-21 19:59 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]

Hi,

I have tested the suggested fix and this is the result:

:    1 <--
::    1
:::    0
192.168.1.1    0
:192.168.1.1    1 <--
::192.168.1.1    1
:ffff:192.168.1.1    1 <--
::ffff:192.168.1.1    1
.192.168.1.1    0
:.192.168.1.1    0
ffff:c0a8:5e4    0
:ffff:c0a8:5e4    1 <--
0:0:0:0:0:ffff:c0a8:5e4    1
0:0:0:0:ffff:c0a8:5e4    0
0::ffff:c0a8:5e4    1
::0::ffff:c0a8:5e4    0
c0a8    0



Those marked with the <-- are still incorrect.

Regards
Paul


On Mon, Oct 21, 2013 at 4:08 AM, Rich Felker <dalias@aerifal.cx> wrote:

> It's not an annoyance at all, and your test cases may be useful for
> writing a regression test, so I'm replying on-list. As you may have
> noticed, I already fixed the first issue you reported, but the second
> issue does remain. nsz has proposed a simple fix: at line 61,
>
> -                       if (s[j]!='.') return 0;
> +                       if (s[j]!='.' || brk<0) return 0;
>
> I have not checked this yet but I suspect it's correct. Please let me
> know if it works for you.
>
> Rich
>
>
> On Sun, Oct 20, 2013 at 09:35:32PM +0200, Paul Schutte wrote:
> > Hi Rich,
> >
> > I send this directly to you as I think I am starting to annoy the people
> on
> > the list with this.
> >
> > I have done some testing on this and here is the result:
> >
> >
> >
> > proper
> > before  : 0
> > 1  :: 1
> > 1  ::: 0
> > 0  192.168.1.1 0
> > 1  :192.168.1.1 0
> > 1  ::192.168.1.1 1
> > 1  :ffff:192.168.1.1 0
> > 1  ::ffff:192.168.1.1 1
> > 1  .192.168.1.1 0
> > 0  :.192.168.1.1 0
> > 0  ffff:c0a8:5e4 0
> > 0  :ffff:c0a8:5e4 0
> > 1  0:0:0:0:0:ffff:c0a8:5e4 1
> > 1  0:0:0:0:ffff:c0a8:5e4 0
> > 0  0::ffff:c0a8:5e4 1
> > 1  ::0::ffff:c0a8:5e4 0
> > 0  c0a8 0
> > 0
> > The following seems to produce the correct output:
> >
> > --- a/musl/src/network/inet_pton.c
> > +++ b/musl/src/network/inet_pton.c
> > @@ -14,11 +14,11 @@
> >         return -1;
> >  }
> >
> > -int inet_pton(int af, const char *restrict s, void *restrict a0)
> > +int inet_pton(int af, const char *restrict s0, void *restrict a0)
> >  {
> >         uint16_t ip[8];
> >         unsigned char *a = a0;
> > -       const char *z;
> > +       const char *z, *s = s0;
> >         unsigned long x;
> >         int i, j, v, d, brk=-1, need_v4=0;
> >
> > @@ -36,7 +36,13 @@
> >                 return -1;
> >         }
> >
> > -       if (s[0]==':' && s[1]==':') s++;
> > +       if (s[0]==':') {
> > +               if (s[1]==':') {
> > +                       s++;
> > +               } else {
> > +                       return 0;
> > +               }
> > +       }
> >
> >         for (i=0; ; i++, s+=j+1) {
> >                 if (s[0]==':' && brk<0) {
> > @@ -73,6 +79,9 @@
> >                 *a++ = ip[j]>>8;
> >                 *a++ = ip[j];
> >         }
> > +
> > +       if (s==s0) return 0;
> > +
> >         if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0;
> >         return 1;
> > }
> >
> >
> > I used the following to test:
> >
> > #include <ctype.h>
> > #include <netdb.h>
> > #include <stdarg.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/socket.h>
> > #include <sys/un.h>
> > #include <netinet/in.h>
> > #include <arpa/inet.h>
> >
> > void test(char *s) {
> >         char buf[256];
> >         int musl;
> >         musl=inet_pton(AF_INET6, s, (void*)buf);
> >         printf("%s=%d\n",s,musl);
> > }
> >
> > int main() {
> >
> >         test(":");
> >         test("::");
> >         test(":::");
> >         test("192.168.1.1");
> >         test(":192.168.1.1");
> >         test("::192.168.1.1");
> >         test(":ffff:192.168.1.1");
> >         test("::ffff:192.168.1.1");
> >         test(".192.168.1.1");
> >         test(":.192.168.1.1");
> >         test("ffff:c0a8:5e4");
> >         test(":ffff:c0a8:5e4");
> >         test("0:0:0:0:0:ffff:c0a8:5e4");
> >         test("0:0:0:0:ffff:c0a8:5e4");
> >         test("0::ffff:c0a8:5e4");
> >         test("::0::ffff:c0a8:5e4");
> >         test("c0a8");
> >
> >         return 0;
> > }
> >
> > The output is as follows which seems to be the desired outcome:
> > :=0
> > ::=1
> > :::=0
> > 192.168.1.1=0
> > :192.168.1.1=0
> > ::192.168.1.1=1
> > :ffff:192.168.1.1=0
> > ::ffff:192.168.1.1=1
> > ..192.168.1.1=0
> > :.192.168.1.1=0
> > ffff:c0a8:5e4=0
> > :ffff:c0a8:5e4=0
> > 0:0:0:0:0:ffff:c0a8:5e4=1
> > 0:0:0:0:ffff:c0a8:5e4=0
> > 0::ffff:c0a8:5e4=1
> > ::0::ffff:c0a8:5e4=0
> > c0a8=0
> >
> >
> > Hope I you find this use full.
> >
> > Regards
> > Paul
>

[-- Attachment #2: Type: text/html, Size: 6178 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: inet_pton
  2013-10-21 19:59   ` inet_pton Paul Schutte
@ 2013-10-21 22:25     ` Szabolcs Nagy
  2013-10-21 23:20       ` Szabolcs Nagy
  2013-10-25 15:44     ` Rich Felker
  1 sibling, 1 reply; 6+ messages in thread
From: Szabolcs Nagy @ 2013-10-21 22:25 UTC (permalink / raw)
  To: musl

* Paul Schutte <sjpschutte@gmail.com> [2013-10-21 21:59:16 +0200]:
> I have tested the suggested fix and this is the result:

i've added your test cases to libc-test and some more
which revealed further issues

src/functional/inet_pton.c:59: inet_pton(AF_INET, "1.2.3.", addr) returned 1, want 0
src/functional/inet_pton.c:60: inet_pton(AF_INET, "1.2.3.4.5", addr) returned 1, want 0
src/functional/inet_pton.c:62: inet_pton(AF_INET, "1.2.03.4", addr) returned 1, want 0
src/functional/inet_pton.c:68: inet_pton(AF_INET6, ":", addr) returned 1, want 0
src/functional/inet_pton.c:73: inet_pton(AF_INET6, ":192.168.1.1", addr) returned 1, want 0
src/functional/inet_pton.c:75: inet_pton(AF_INET6, ":ffff:192.168.1.1", addr) returned 1, want 0
src/functional/inet_pton.c:81: inet_pton(AF_INET6, ":ffff:c0a8:5e4", addr) returned 1, want 0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: inet_pton
  2013-10-21 22:25     ` Szabolcs Nagy
@ 2013-10-21 23:20       ` Szabolcs Nagy
  0 siblings, 0 replies; 6+ messages in thread
From: Szabolcs Nagy @ 2013-10-21 23:20 UTC (permalink / raw)
  To: musl

* Szabolcs Nagy <nsz@port70.net> [2013-10-22 00:25:17 +0200]:

> * Paul Schutte <sjpschutte@gmail.com> [2013-10-21 21:59:16 +0200]:
> > I have tested the suggested fix and this is the result:
> 
> i've added your test cases to libc-test and some more
> which revealed further issues
> 

these two changes fix the v6 bugs, but leading zeros of the v4 part
are still swallowed incorrectly (even if v4 parsing gets fixed)

-       if (s[0]==':' && s[1]==':') s++;
+       if (s[0]==':' && *++s!=':') return 0;

-                       if (s[j]!='.') return 0;
+                       if (s[j]!='.' || (i<6 && brk<0)) return 0;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: inet_pton
  2013-10-21 19:59   ` inet_pton Paul Schutte
  2013-10-21 22:25     ` Szabolcs Nagy
@ 2013-10-25 15:44     ` Rich Felker
  2013-10-26 13:22       ` Paul Schutte
  1 sibling, 1 reply; 6+ messages in thread
From: Rich Felker @ 2013-10-25 15:44 UTC (permalink / raw)
  To: musl

On Mon, Oct 21, 2013 at 09:59:16PM +0200, Paul Schutte wrote:
> Hi,
> 
> I have tested the suggested fix and this is the result:
> 
> :    1 <--
> ::    1
> :::    0
> 192.168.1.1    0
> :192.168.1.1    1 <--
> ::192.168.1.1    1
> :ffff:192.168.1.1    1 <--
> ::ffff:192.168.1.1    1
> ..192.168.1.1    0
> :.192.168.1.1    0
> ffff:c0a8:5e4    0
> :ffff:c0a8:5e4    1 <--
> 0:0:0:0:0:ffff:c0a8:5e4    1
> 0:0:0:0:ffff:c0a8:5e4    0
> 0::ffff:c0a8:5e4    1
> ::0::ffff:c0a8:5e4    0
> c0a8    0
> 
> 
> 
> Those marked with the <-- are still incorrect.

I've merged nsz's fixes and they seem to be handled correctly now. nsz
added your test cases to libc-test too. Please let me know if you're
still experiencing problems.

Rich


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: inet_pton
  2013-10-25 15:44     ` Rich Felker
@ 2013-10-26 13:22       ` Paul Schutte
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Schutte @ 2013-10-26 13:22 UTC (permalink / raw)
  To: musl

Thanks you very much Nsz and Rich !

It is working perfectly now.

Big thanks
Paul

On 10/25/13, Rich Felker <dalias@aerifal.cx> wrote:
> On Mon, Oct 21, 2013 at 09:59:16PM +0200, Paul Schutte wrote:
>> Hi,
>>
>> I have tested the suggested fix and this is the result:
>>
>> :    1 <--
>> ::    1
>> :::    0
>> 192.168.1.1    0
>> :192.168.1.1    1 <--
>> ::192.168.1.1    1
>> :ffff:192.168.1.1    1 <--
>> ::ffff:192.168.1.1    1
>> ..192.168.1.1    0
>> :.192.168.1.1    0
>> ffff:c0a8:5e4    0
>> :ffff:c0a8:5e4    1 <--
>> 0:0:0:0:0:ffff:c0a8:5e4    1
>> 0:0:0:0:ffff:c0a8:5e4    0
>> 0::ffff:c0a8:5e4    1
>> ::0::ffff:c0a8:5e4    0
>> c0a8    0
>>
>>
>>
>> Those marked with the <-- are still incorrect.
>
> I've merged nsz's fixes and they seem to be handled correctly now. nsz
> added your test cases to libc-test too. Please let me know if you're
> still experiencing problems.
>
> Rich
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-26 13:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAKHv7pi-VhjkzOpUuz3RBachSPfPOHXN09G7rhM9uFtw4ri2-A@mail.gmail.com>
2013-10-21  2:08 ` inet_pton Rich Felker
2013-10-21 19:59   ` inet_pton Paul Schutte
2013-10-21 22:25     ` Szabolcs Nagy
2013-10-21 23:20       ` Szabolcs Nagy
2013-10-25 15:44     ` Rich Felker
2013-10-26 13:22       ` Paul Schutte

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).