t/io/socket.t failing on hurd: peer from recv() should be empty or the remote name #14052

p5pRT · 2014-08-31T00:17:18Z

Migrated from rt.perl.org#122657 (status was 'resolved')

Searchable as RT122657$

p5pRT · 2014-08-31T00:17:18Z

From @jmdh

As reported in <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=758718> this test (introduced as part of [perl #118843]) fails on Hurd. The comment from the Hurd porting team is:

"The test seems fishy to me: it is making sure that the name as returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason."

p5pRT · 2014-09-23T00:32:11Z

From @tonycoz

On Sat Aug 30 17:17:18 2014, dom wrote:

As reported in <https://bugs.debian.org/cgi-
bin/bugreport.cgi?bug=758718> this test (introduced as part of [perl
#118843]) fails on Hurd. The comment from the Hurd porting team is:

"The test seems fishy to me: it is making sure that the name as
returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the
actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason."

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

Tony

p5pRT · 2014-09-23T00:32:11Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2014-09-23T13:22:14Z

From svante.signell@gmail.com

On Mon, 2014-09-22 at 17:32 -0700, Tony Cook via RT wrote:

On Sat Aug 30 17:17:18 2014, dom wrote:

As reported in <https://bugs.debian.org/cgi-
bin/bugreport.cgi?bug=758718> this test (introduced as part of [perl
#118843]) fails on Hurd. The comment from the Hurd porting team is:

"The test seems fishy to me: it is making sure that the name as
returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the
actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason."

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

I think there is something wrong with this test, even comparing with
getpeername():
elsif (defined $pid) {
curr_test(curr_test()+2);
#sleep 1;
# child
ok_child(close($serv), "close server socket in child");
ok_child(socket(my $child, PF_INET, SOCK_STREAM, $tcp),
"make child tcp socket");

ok_child(connect($child, $bind_name), "connect() works")
or diag "connect error: $!";

my $buf;
my $recv_peer = recv($child, $buf, 1000, 0);
# [perl #118843]
ok_child($recv_peer eq '' || $recv_peer eq $bind_name,
"peer from recv() should be empty or the remote name");

The return value of recv() should be the number of bytes returned?? I've
tested on both Linux and Hurd and the received value in both cases is
zero. (and the return value of $bind_name is zero too!)

From the man page of recv():

RETURN VALUE

These calls return the number of bytes received, or -1 if an error
occurred. In the event of an error, errno is set to indicate the error.

When a stream socket peer has performed an orderly shutdown, the return
value will be 0 (the traditional "end-of-file" return).

Datagram sockets in various domains (e.g., the UNIX and Internet
domains) permit zero-length datagrams. When such a datagram is
received, the return value is 0.

The value 0 may also be returned if the requested number of bytes to
receive from a stream socket was 0.

p5pRT · 2014-09-23T23:39:02Z

From @tonycoz

On Tue Sep 23 06:22:14 2014, svante.signell@gmail.com wrote:

On Mon, 2014-09-22 at 17:32 -0700, Tony Cook via RT wrote:

On Sat Aug 30 17:17:18 2014, dom wrote:

As reported in <https://bugs.debian.org/cgi-
bin/bugreport.cgi?bug=758718> this test (introduced as part of [perl
#118843]) fails on Hurd. The comment from the Hurd porting team is:

"The test seems fishy to me: it is making sure that the name as
returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the
actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason."

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

I think there is something wrong with this test, even comparing with
getpeername():
elsif (defined $pid) {
curr_test(curr_test()+2);
#sleep 1;
# child
ok_child(close($serv), "close server socket in child");
ok_child(socket(my $child, PF_INET, SOCK_STREAM, $tcp),
"make child tcp socket");
        ok\_child$connect\($child\, $bind\_name$\, "connect works"\)
            or diag "connect error&#8203;: $\!";

        my $buf;
        my $recv\_peer = recv$$child\, $buf\, 1000\, 0$;
        \# \[perl \#118843\]
        ok\_child$$recv\_peer eq '' || $recv\_peer eq $bind\_name\,
           "peer from recv\($ should be empty or the remote name"\);
The return value of recv() should be the number of bytes returned?? I've
tested on both Linux and Hurd and the received value in both cases is
zero. (and the return value of $bind_name is zero too!)

From the man page of recv():

From the perl documentation for recv():

Returns the address of the sender if
SOCKET's protocol supports this; returns an empty string
otherwise. If there's an error, returns the undefined value.
This call is actually implemented in terms of recvfrom(2)
system call.

Tony

p5pRT · 2014-09-24T18:04:27Z

From svante.signell@gmail.com

On Tue, 2014-09-23 at 16:39 -0700, Tony Cook via RT wrote:

On Tue Sep 23 06:22:14 2014, svante.signell@gmail.com wrote:

On Mon, 2014-09-22 at 17:32 -0700, Tony Cook via RT wrote:

On Sat Aug 30 17:17:18 2014, dom wrote:

As reported in <https://bugs.debian.org/cgi-
bin/bugreport.cgi?bug=758718> this test (introduced as part of [perl
#118843]) fails on Hurd. The comment from the Hurd porting team is:

"The test seems fishy to me: it is making sure that the name as
returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the
actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason."

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

From the perl documentation for recv():

                              Returns the address of the sender if
           SOCKET's protocol supports this; returns an empty string
           otherwise\.  If there's an error\, returns the undefined value\.
           This call is actually implemented in terms of recvfrom\(2\)
           system call\.

Did not find that when searching the web, and thought the return value
was as the system call. Found it now:
http://perldoc.perl.org/functions/recv.html

Attached is a patch modified Debian patch which works in both GNU/Linux
and GNU/Hurd. Of course you can find a better solution, and the print
statements should be removed. Comparing $recv_peer eq my $peer_name does
not work.

BTW: What's the difference between $var and my $var?

Thanks!

p5pRT · 2014-09-24T18:04:27Z

From svante.signell@gmail.com

From 9256a6f74e501b42d4a820728327763a651cfa4f Mon Sep 17 00:00:00 2001
From: Dominic Hargreaves <dom@earth.li>
Date: Sun, 31 Aug 2014 00:42:47 +0000
Subject: Disable failing GNU/Hurd test in t/io/socket.t

According to Samuel Thibault this test is questionable and should be
discussed with upstream:

"The test seems fishy to me: it is making sure that the name as returned
by recv is *exactly* the same as what the server socket is bound to,
which is 0.0.0.0:some_port but I would expect recv to return the actual
IP address used in the socket, not 0.0.0.0. It happens that Linux
doesn't return anything at all so it goes fine there, but that's not a
reason. The test should most probably be discussed with upstream."

Bug-Debian: http://bugs.debian.org/758718
Bug: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=122657
Patch-Name: fixes/hurd_test_todo_socket.t

t/io/socket.t | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Index: perl-5.20.1/t/io/socket.t

--- perl-5.20.1.orig/t/io/socket.t
+++ perl-5.20.1/t/io/socket.t
@@ -102,8 +102,20 @@ SKIP: {
my $buf;
my $recv_peer = recv($child, $buf, 1000, 0);
# [perl #118843]
- ok_child($recv_peer eq '' || $recv_peer eq $bind_name,
- "peer from recv() should be empty or the remote name");
+ if ($recv_peer eq '') {
+ my $recv_addr = '';
+ my $peer_addr = '';
+ }
+ else {
+ my ($recv_port, $recv_addr) = sockaddr_in($recv_peer);
+ my $peer_name = getpeername($child);
+ my ($peer_port, $peer_addr) = sockaddr_in($peer_name);
+ printf("# recv_addr %s\n", inet_ntoa($recv_addr));
+ printf("# peer_addr %s\n", inet_ntoa($peer_addr));
+ }
+ ok_child($recv_peer eq '' || my $recv_addr eq my $peer_addr,
+ "peer from recv() should be empty or the peer address");
+
while(defined recv($child, my $tmp, 1000, 0)) {
last if length $tmp == 0;
$buf .= $tmp;

p5pRT · 2014-09-24T20:47:33Z

From @Leont

On Wed, Sep 24, 2014 at 10:48 AM, Svante Signell <svante.signell@gmail.com>
wrote:

Did not find that when searching the web, and thought the return value
was as the system call. Found it now:
http://perldoc.perl.org/functions/recv.html

Attached is a patch modified Debian patch which works in both GNU/Linux
and GNU/Hurd. Of course you can find a better solution, and the print
statements should be removed. Comparing $recv_peer eq my $peer_name does
not work.

Your patch is nonsensical.

BTW: What's the difference between $var and my $var?

And that question explains why. The my keyword created a new lexical
variable with that name (so you're redeclaring $recv_addr and $peer_addr).

Leon

p5pRT · 2014-09-24T21:32:09Z

From @Leont

On Tue, Sep 23, 2014 at 2:32 AM, Tony Cook via RT <perlbug-followup@perl.org

wrote:

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

Yes, that would be a more sensible test. Patch to that effect attached.

Leon

p5pRT · 2014-09-24T21:32:09Z

From @Leont

0001-Compare-recv-return-value-to-peername-in-socket-test.patch

From 34faf418933fa03c23374d777313a39238fea375 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <fawaka@gmail.com>
Date: Wed, 24 Sep 2014 23:17:21 +0200
Subject: [PATCH] Compare recv return value to peername in socket test

---
 t/io/socket.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/io/socket.t b/t/io/socket.t
index b723e3c..345d196 100644
--- a/t/io/socket.t
+++ b/t/io/socket.t
@@ -102,7 +102,7 @@ SKIP: {
 	    my $buf;
 	    my $recv_peer = recv($child, $buf, 1000, 0);
 	    # [perl #118843]
-	    ok_child($recv_peer eq '' || $recv_peer eq $bind_name,
+	    ok_child($recv_peer eq '' || $recv_peer eq getpeername $child,
 	       "peer from recv() should be empty or the remote name");
 	    while(defined recv($child, my $tmp, 1000, 0)) {
 		last if length $tmp == 0;
-- 
2.1.1-391-g7a54a76

p5pRT · 2014-09-25T13:14:15Z

From svante.signell@gmail.com

On Thu, 2014-09-25 at 10:51 +0200, Svante Signell wrote:

On Wed, 2014-09-24 at 14:32 -0700, Leon Timmermans via RT wrote:

On Tue, Sep 23, 2014 at 2:32 AM, Tony Cook via RT <perlbug-followup@perl.org

wrote:

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

Yes, that would be a more sensible test. Patch to that effect attached.

FYI: With your patch the test still fails on GNU/Hurd :(

The attached patch make the test work on GNU/Hurd
(but not on GNU/Linux since in that case $recv_peer is empty, and
my ($recv_port, $recv_addr) = sockaddr_in($recv_peer);
fails.)

maybe
if ($^O eq 'linux') {
...
} else {
...
}

p5pRT · 2014-09-25T13:14:15Z

From svante.signell@gmail.com

socket.t.patch

--- ./t_io_socket.t.orig	2014-09-25 11:10:40.000000000 +0200
+++ perl-5.20.1/t/io/socket.t	2014-09-25 11:08:29.000000000 +0200
@@ -102,8 +102,13 @@
 	    my $buf;
 	    my $recv_peer = recv($child, $buf, 1000, 0);
 	    # [perl #118843]
-	    ok_child($recv_peer eq '' || $recv_peer eq $bind_name,
-		     "peer from recv() should be empty or the remote name");
+	    my ($recv_port, $recv_addr) = sockaddr_in($recv_peer);
+	    my $peer_name = getpeername($child);
+	    my ($peer_port, $peer_addr) = sockaddr_in($peer_name);
+	    printf("# peer_port %d\n", $peer_port);
+	    printf("# peer_addr %s\n", inet_ntoa($peer_addr));
+	    ok_child($recv_peer eq '' || $recv_addr eq $peer_addr,
+		     "peer from recv() should be empty or the peer address");
 	    while(defined recv($child, my $tmp, 1000, 0)) {
 		last if length $tmp == 0;
 		$buf .= $tmp;

p5pRT · 2014-09-25T13:14:38Z

From svante.signell@gmail.com

On Wed, 2014-09-24 at 14:32 -0700, Leon Timmermans via RT wrote:

On Tue, Sep 23, 2014 at 2:32 AM, Tony Cook via RT <perlbug-followup@perl.org

wrote:

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

Yes, that would be a more sensible test. Patch to that effect attached.

FYI: With your patch the test still fails on GNU/Hurd :(

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

p5pRT · 2014-09-25T13:14:50Z

From svante.signell@gmail.com

On Wed, 2014-09-24 at 13:47 -0700, Leon Timmermans via RT wrote:

On Wed, Sep 24, 2014 at 10:48 AM, Svante Signell <svante.signell@gmail.com>
wrote:

Did not find that when searching the web, and thought the return value
was as the system call. Found it now:
http://perldoc.perl.org/functions/recv.html

Attached is a patch modified Debian patch which works in both GNU/Linux
and GNU/Hurd. Of course you can find a better solution, and the print
statements should be removed. Comparing $recv_peer eq my $peer_name does
not work.

Your patch is nonsensical.

See below!

BTW: What's the difference between $var and my $var?

And that question explains why. The my keyword created a new lexical
variable with that name (so you're redeclaring $recv_addr and $peer_addr).

I would really appreciate comments to people not even knowing the famous
"perl" language, by experts to be a little more nice when solving
problems caused by upstream authors.

I think you have just created a non-fan of "perl", not a student willing
to learn more :(

Thanks!

p5pRT · 2014-09-25T13:15:25Z

From svante.signell@gmail.com

On Thu, 2014-09-25 at 11:28 +0200, Svante Signell wrote:

On Thu, 2014-09-25 at 10:51 +0200, Svante Signell wrote:

On Wed, 2014-09-24 at 14:32 -0700, Leon Timmermans via RT wrote:

On Tue, Sep 23, 2014 at 2:32 AM, Tony Cook via RT <perlbug-followup@perl.org

wrote:

If it compared against getpeername(), would that pass?

As you say, comparing against the bind address is just plain wrong.

I don't have ready access to Hurd.

Yes, that would be a more sensible test. Patch to that effect attached.

FYI: With your patch the test still fails on GNU/Hurd :(

The attached patch make the test work on GNU/Hurd
(but not on GNU/Linux since in that case $recv_peer is empty, and
my ($recv_port, $recv_addr) = sockaddr_in($recv_peer);
fails.)

Attached is a patch that works on Linux, kFreeBSD and Hurd:
Linux and kFreeBSD returns the empty string, Hurd does not.

OK?

p5pRT · 2014-09-25T13:15:25Z

From svante.signell@gmail.com

socket.t.patch

--- t_io_socket.t.orig	2014-09-25 11:10:40.000000000 +0200
+++ t_io_socket.t	2014-09-25 13:41:23.000000000 +0200
@@ -102,8 +102,18 @@
 	    my $buf;
 	    my $recv_peer = recv($child, $buf, 1000, 0);
 	    # [perl #118843]
-	    ok_child($recv_peer eq '' || $recv_peer eq $bind_name,
-		     "peer from recv() should be empty or the remote name");
+	    if ($recv_peer ne '')
+	    {
+		my $peer_name = getpeername($child);
+		my ($peer_port, $peer_addr) = sockaddr_in($peer_name);
+		my ($recv_port, $recv_addr) = sockaddr_in($recv_peer);
+		ok_child($recv_addr eq $peer_addr,
+		     "peer from recv() should be empty or the peer address");
+	    } else {
+		ok_child($recv_peer eq '',
+		     "peer from recv() should be empty or the peer address");
+	    }
+
 	    while(defined recv($child, my $tmp, 1000, 0)) {
 		last if length $tmp == 0;
 		$buf .= $tmp;

p5pRT · 2014-09-25T15:22:16Z

From @Leont

On Thu, Sep 25, 2014 at 12:17 AM, Svante Signell <svante.signell@gmail.com>
wrote:

And that question explains why. The my keyword created a new lexical
variable with that name (so you're redeclaring $recv_addr and
$peer_addr).

I would really appreciate comments to people not even knowing the famous
"perl" language, by experts to be a little more nice when solving
problems caused by upstream authors.

I think you have just created a non-fan of "perl", not a student willing
to learn more :(

My apologies, I had no intention of offending you. My choice of words may
be been a bit stronger than I intended. I was commenting on the quality of
the patch, not on your effort. If I didn't care about the latter I wouldn't
have bothered to explain why the patch wasn't good.

Leon

p5pRT · 2014-09-25T16:04:09Z

From @Leont

On Thu, Sep 25, 2014 at 10:51 AM, Svante Signell <svante.signell@gmail.com>
wrote:

FYI: With your patch the test still fails on GNU/Hurd :(

That is unexpected.

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

printf "%s\n", unpack "H*", $recv_peer;
printf "%s\n", unpack "H*", getpeername $child,

That should give a hex-dump, which should give some information on why they
aren't identical.

Leon

p5pRT · 2014-09-26T17:38:43Z

From svante.signell@gmail.com

On Thu, 2014-09-25 at 09:04 -0700, Leon Timmermans via RT wrote:

On Thu, Sep 25, 2014 at 10:51 AM, Svante Signell <svante.signell@gmail.com>
wrote:

FYI: With your patch the test still fails on GNU/Hurd :(

That is unexpected.

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

printf "%s\n", unpack "H*", $recv_peer;
printf "%s\n", unpack "H*", getpeername $child,

That should give a hex-dump, which should give some information on why they
aren't identical.

100289c37f0000011f0000001f000000
100289c37f000001c091070100000000

p5pRT · 2014-09-26T18:13:46Z

From @Leont

On Fri, Sep 26, 2014 at 7:38 PM, Svante Signell <svante.signell@gmail.com>
wrote:

On Thu, 2014-09-25 at 09:04 -0700, Leon Timmermans via RT wrote:

On Thu, Sep 25, 2014 at 10:51 AM, Svante Signell <
svante.signell@gmail.com>
wrote:

FYI: With your patch the test still fails on GNU/Hurd :(

That is unexpected.

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

printf "%s\n", unpack "H*", $recv_peer;
printf "%s\n", unpack "H*", getpeername $child,

That should give a hex-dump, which should give some information on why
they
aren't identical.

100289c37f0000011f0000001f000000
100289c37f000001c091070100000000

Actually, I'd be curious about the $bind_addr too, but in the big picture
that shouldn't matter.

The 7f000001 is localhost, 89c3 must be the port number (35267). 1002 must
be the address family. The rest of the data looks like garbage to me. It
seems you're leaking it in the padding of the sockaddr_in. Comparing
addresses isn't well-defined, but I'd argue that comparing sockaddr's for
binary equivalence is a perfectly reasonable way to compare them.

That looks to me like a bug in the Hurd, regardless of how perl should
compare addresses.

Leon

p5pRT · 2014-09-26T19:14:24Z

From svante.signell@gmail.com

On Fri, 2014-09-26 at 11:13 -0700, Leon Timmermans via RT wrote:

On Fri, Sep 26, 2014 at 7:38 PM, Svante Signell <svante.signell@gmail.com>
wrote:

On Thu, 2014-09-25 at 09:04 -0700, Leon Timmermans via RT wrote:

On Thu, Sep 25, 2014 at 10:51 AM, Svante Signell <
svante.signell@gmail.com>
wrote:

FYI: With your patch the test still fails on GNU/Hurd :(

That is unexpected.

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

printf "%s\n", unpack "H*", $recv_peer;
printf "%s\n", unpack "H*", getpeername $child,

That should give a hex-dump, which should give some information on why
they
aren't identical.

100289c37f0000011f0000001f000000
100289c37f000001c091070100000000

Actually, I'd be curious about the $bind_addr too, but in the big picture
that shouldn't matter.

$bind_name: 100289c57f000001ffffffff01000000

The 7f000001 is localhost, 89c3 must be the port number (35267). 1002 must
be the address family. The rest of the data looks like garbage to me. It
seems you're leaking it in the padding of the sockaddr_in. Comparing
addresses isn't well-defined, but I'd argue that comparing sockaddr's for
binary equivalence is a perfectly reasonable way to compare them.

That looks to me like a bug in the Hurd, regardless of how perl should
compare addresses.

I'll forward this to the Hurd developers. Are you planning to change
this test, either way?

p5pRT · 2014-09-27T09:25:19Z

From svante.signell@gmail.com

On Fri, 2014-09-26 at 21:14 +0200, Svante Signell wrote:

On Fri, 2014-09-26 at 11:13 -0700, Leon Timmermans via RT wrote:

On Fri, Sep 26, 2014 at 7:38 PM, Svante Signell <svante.signell@gmail.com>
wrote:

On Thu, 2014-09-25 at 09:04 -0700, Leon Timmermans via RT wrote:

On Thu, Sep 25, 2014 at 10:51 AM, Svante Signell <
svante.signell@gmail.com>
wrote:

FYI: With your patch the test still fails on GNU/Hurd :(

That is unexpected.

Please: How to add (correct) print statements to see what is returned,
you are the "perl" gurus??

printf "%s\n", unpack "H*", $recv_peer;
printf "%s\n", unpack "H*", getpeername $child,

That should give a hex-dump, which should give some information on why
they
aren't identical.

100289c37f0000011f0000001f000000
100289c37f000001c091070100000000

Actually, I'd be curious about the $bind_addr too, but in the big picture
that shouldn't matter.

$bind_name: 100289c57f000001ffffffff01000000

The 7f000001 is localhost, 89c3 must be the port number (35267). 1002 must
be the address family. The rest of the data looks like garbage to me. It
seems you're leaking it in the padding of the sockaddr_in. Comparing
addresses isn't well-defined, but I'd argue that comparing sockaddr's for
binary equivalence is a perfectly reasonable way to compare them.

That looks to me like a bug in the Hurd, regardless of how perl should
compare addresses.

I'll forward this to the Hurd developers. Are you planning to change
this test, either way?

Hi, below are some comments from Samuel Thibault on IRC:
(12:39:34 AM) youpi: gnu_srs1: 1002 is not only the address family. 10
is sin_len, thus 16, and 02 is the address family, thus AF_INET
(12:40:15 AM) youpi: I don't know how $recv_peer happened to contain so
many bytes, but it's not supposed to contain so many, but just 16, as
advertised by sin_len
(12:40:26 AM) youpi: (but also returned by getpeername)
(12:40:53 AM) youpi: (which is where perl should be fetching the size,
since sin_len is not really portable)
(12:51:26 AM) youpi: gnu_srs1: btw, « I'd argue that comparing
sockaddr's for binary equivalence is a perfectly reasonable way to
compare them. », I don't agree
(12:51:46 AM) youpi: I don't think the standard asserts that padding
bytes due to alignment have any defined value

As seen above you are comparing padding bytes, and that content is not
defined by any standard. You should decode sin_len from getpeername and
then compare only that many bytes as given by it.

p5pRT · 2014-09-27T14:02:16Z

From @Leont

On Sat, Sep 27, 2014 at 11:24 AM, Svante Signell <svante.signell@gmail.com>
wrote:

Hi, below are some comments from Samuel Thibault on IRC:
(12:39:34 AM) youpi: gnu_srs1: 1002 is not only the address family. 10
is sin_len, thus 16, and 02 is the address family, thus AF_INET
(12:40:15 AM) youpi: I don't know how $recv_peer happened to contain so
many bytes, but it's not supposed to contain so many, but just 16, as
advertised by sin_len
(12:40:26 AM) youpi: (but also returned by getpeername)
(12:40:53 AM) youpi: (which is where perl should be fetching the size,
since sin_len is not really portable)

$recv_peer is containing exactly 16 bytes (or 32 hex characters). 1 for the
length, 1 for the address family, 2 for the port number, 4 for the IP
address and 8 without apparent function. I assume this is because struct
sockaddr is 16 bytes.

(12:51:26 AM) youpi: gnu_srs1: btw, « I'd argue that comparing
sockaddr's for binary equivalence is a perfectly reasonable way to
compare them. », I don't agree
(12:51:46 AM) youpi: I don't think the standard asserts that padding
bytes due to alignment have any defined value

The standard is rather silent on this matter, hence I don't believe that is
a matter of right or wrong but that this is more a matter of more or less
useful.

The sensible thing to me is to either return the correct amount of relevant
bytes (in this case 8), or zero the additional bytes. I'm sure the Hurd is
already doing the former for AF_UNIX addresses, it's not much of a stretch
to also expect something similar in AF_INET.

And quite frankly, I'd worry more about what data Hurd is leaking…

Also, if one ever wants to add fields to the sockaddr_in (not that that's
very likely), you need to be zeroing it now.

As seen above you are comparing padding bytes, and that content is not
defined by any standard. You should decode sin_len from getpeername and
then compare only that many bytes as given by it.

That's exactly what we're doing right now.

Leon

p5pRT · 2014-09-28T20:52:26Z

From svante.signell@gmail.com

On Sat, 2014-09-27 at 07:02 -0700, Leon Timmermans via RT wrote:

On Sat, Sep 27, 2014 at 11:24 AM, Svante Signell <svante.signell@gmail.com>
wrote:

As seen above you are comparing padding bytes, and that content is not
defined by any standard. You should decode sin_len from getpeername and
then compare only that many bytes as given by it.

That's exactly what we're doing right now.

You were right, there was a bug in the Linux code borrowed for the
network, see below. Samuel had problems delivering this message
therefore I'm replying. In case it got through, just ignore this
message.

Thank you for your time,
"the middle man"

Hello,

Samuel Thibault wrote:

I don't know how $recv_peer happened to contain so many bytes, but
it's not
supposed to contain so many, but just 16, as advertised by sin_len

Oops, sorry. I was in a hurry when answering that, and miscounted the
printed
bytes.

Father Chrysostomos wrote:

And quite frankly, I'd worry more about what data Hurd is leaking…

Indeed. It seems it's a bug which was present in the Linux code that
we have
borrowed into the Hurd stack. I have thus backported the fix, and the
padding
is now zero instead of stack leak. It has actually let me find a
potential
issue in the Linux source code in the SO_PEERNAME code :)

Samuel

p5pRT · 2014-09-28T23:35:30Z

From samuel.thibault@ens-lyon.org

Hello,

Samuel Thibault wrote:

I don't know how $recv_peer happened to contain so many bytes, but it's not
supposed to contain so many, but just 16, as advertised by sin_len

Oops, sorry. I was in a hurry when answering that, and miscounted the printed
bytes.

Father Chrysostomos wrote:

And quite frankly, I'd worry more about what data Hurd is leaking…

Indeed. It seems it's a bug which was present in the Linux code that we have
borrowed into the Hurd stack. I have thus backported the fix, and the padding
is now zero instead of stack leak. It has actually let me find a potential
issue in the Linux source code in the SO_PEERNAME code :)

Samuel

p5pRT · 2014-10-01T23:56:26Z

From @tonycoz

On Fri Sep 26 12:14:24 2014, svante.signell@gmail.com wrote:

I'll forward this to the Hurd developers. Are you planning to change
this test, either way?

I plan to apply Leon's fix.

Do you have any idea when a new hurd kernel might be released with the fixes?

If it won't be fixed soon I'll make that test a TODO test so it won't fail the build.

Tony

p5pRT · 2014-10-02T09:03:03Z

From @Leont

On Thu, Oct 2, 2014 at 1:56 AM, Tony Cook via RT <perlbug-followup@perl.org>
wrote:

I plan to apply Leon's fix.

Do you have any idea when a new hurd kernel might be released with the
fixes?

If it won't be fixed soon I'll make that test a TODO test so it won't fail
the build.

I was planning to do just that, but hadn't come around to it. Waiting for a
new kernel to trickle down is likely to take too long, we should just skip
it for now.

Leon

p5pRT · 2014-10-03T04:41:35Z

From @tonycoz

On Thu Oct 02 02:03:03 2014, LeonT wrote:

On Thu, Oct 2, 2014 at 1:56 AM, Tony Cook via RT <perlbug-followup@perl.org>
wrote:

I plan to apply Leon's fix.

Do you have any idea when a new hurd kernel might be released with the
fixes?

If it won't be fixed soon I'll make that test a TODO test so it won't fail
the build.

I was planning to do just that, but hadn't come around to it. Waiting for a
new kernel to trickle down is likely to take too long, we should just skip
it for now.

I've pushed your fix, and made the the test TODO on hurd.

I used TODO instead of skip so that once it's fixed there's some visible change in behaviour so we can remove the TODO or make it release specific.

Closing this ticket.

Tony

p5pRT · 2014-10-03T04:41:36Z

@tonycoz - Status changed from 'open' to 'resolved'

p5pRT closed this as completed Oct 3, 2014

p5pRT added Severity Low distro-gnu labels Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

t/io/socket.t failing on hurd: peer from recv() should be empty or the remote name #14052

t/io/socket.t failing on hurd: peer from recv() should be empty or the remote name #14052

p5pRT commented Aug 31, 2014

p5pRT commented Aug 31, 2014

p5pRT commented Sep 23, 2014

p5pRT commented Sep 23, 2014

p5pRT commented Sep 23, 2014

p5pRT commented Sep 23, 2014

p5pRT commented Sep 24, 2014

p5pRT commented Sep 24, 2014

p5pRT commented Sep 24, 2014

p5pRT commented Sep 24, 2014

p5pRT commented Sep 24, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 25, 2014

p5pRT commented Sep 26, 2014

p5pRT commented Sep 26, 2014

p5pRT commented Sep 26, 2014

p5pRT commented Sep 27, 2014

p5pRT commented Sep 27, 2014

p5pRT commented Sep 28, 2014

p5pRT commented Sep 28, 2014

p5pRT commented Oct 1, 2014

p5pRT commented Oct 2, 2014

p5pRT commented Oct 3, 2014

p5pRT commented Oct 3, 2014

t/io/socket.t failing on hurd: peer from recv() should be empty or the remote name #14052

t/io/socket.t failing on hurd: peer from recv() should be empty or the remote name #14052

Comments

p5pRT commented Aug 31, 2014

p5pRT commented Aug 31, 2014

From @jmdh

p5pRT commented Sep 23, 2014

From @tonycoz

p5pRT commented Sep 23, 2014

p5pRT commented Sep 23, 2014

From svante.signell@gmail.com

p5pRT commented Sep 23, 2014

From @tonycoz

p5pRT commented Sep 24, 2014

From svante.signell@gmail.com

p5pRT commented Sep 24, 2014

From svante.signell@gmail.com

Index​: perl-5.20.1/t/io/socket.t

p5pRT commented Sep 24, 2014

From @Leont

p5pRT commented Sep 24, 2014

From @Leont

p5pRT commented Sep 24, 2014

From @Leont

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From svante.signell@gmail.com

p5pRT commented Sep 25, 2014

From @Leont

p5pRT commented Sep 25, 2014

From @Leont

p5pRT commented Sep 26, 2014

From svante.signell@gmail.com

p5pRT commented Sep 26, 2014

From @Leont

p5pRT commented Sep 26, 2014

From svante.signell@gmail.com

p5pRT commented Sep 27, 2014

From svante.signell@gmail.com

p5pRT commented Sep 27, 2014

From @Leont

p5pRT commented Sep 28, 2014

From svante.signell@gmail.com

p5pRT commented Sep 28, 2014

From samuel.thibault@ens-lyon.org

p5pRT commented Oct 1, 2014

From @tonycoz

p5pRT commented Oct 2, 2014

From @Leont

p5pRT commented Oct 3, 2014

From @tonycoz

p5pRT commented Oct 3, 2014

Index: perl-5.20.1/t/io/socket.t