Bug#774534: neon27: Racy redirect test

[Samuel Thibault]

Source: neon27
Version: 0.30.1-1
Severity: important

Hello,

I have been having issues with running the neon27 testsuite, more
precisely the redirect test. I'm getting this:

no_redirect........... FAIL (did not get NE_REDIRECT)

This was originally on hurd-i386, but debugging the issues shows that
it can potentially happen on other OSes, it just depends on process
scheduling getting lucky or not. This can be triggered by applying the
attached "testcase" patch, which puts a couple of sleeps at the right
places.

Basically, what redirect.c's no_redirect is doing is the attached
test.c:

1) the client sends a first request
2) the server gets the request
3) the server sends two response
4) the client gets the first server response and checks it
5) the server calls shutdown(SHUT_RD)
6) the server calls recv() in loop to flush the socket
7) the client sends a second request
8) the server closes the socket
9) the client gets the second server response and checks it

And what fails is the latest step. This happens only when scheduling is
unlucky enough that step 7 happens just between steps 6 and 8, but that
*can* happen. In that case the server close() at step 6 will see that
some data was received, and thus send a RST, which then turns into an
ECONNRESET during step 9.

One could have thought that shutdown(SHUT_RD) would stop data from
getting in, and the recv() loop would flush them all without fearing
that new data comes before close(), but it seems at least Linux does
not do that, and thus we have a race.  I haven't seen POSIX specifying
the details of what happens at shutdown() more precisely than “The
shutdown() function disables subsequent send and/or receive operations
on a socket”...

A safer way would of course to make the server in the test properly wait
and consume the second request before emitting the second answer and
shutting down the socket.

Samuel

-- System Information:
Debian Release: 8.0
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'buildd-unstable'), (500, 'unstable'), 
(500, 'stable'), (500, 'oldstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.18.0 (SMP w/8 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)

--- a/test/common/child.c
+++ b/test/common/child.c
@@ -154,6 +154,7 @@ static int close_socket(ne_socket *sock)
     
     shutdown(fd, 0);
     while (ne_sock_read(sock, buf, sizeof buf) > 0);
+    sleep(2);
 #endif
     return ne_sock_close(sock);
 }
--- a/test/redirect.c
+++ b/test/redirect.c
@@ -181,6 +181,7 @@ static int no_redirect(void)
 
     ONN("redirect non-NULL after non-redir req", ne_redirect_location(sess));
 
+    sleep(1);
     CALL(process_redir(sess, "/foo", &loc));
     CALL(await_server());
 
@@ -189,11 +190,13 @@ static int no_redirect(void)
 }
 
 ne_test tests[] = {
+#if 0
     T(lookup_localhost),
     T(simple),
     T(non_absolute),
     T(relative_1),
     T(relative_2),
+#endif
     T(no_redirect),
     T(NULL) 
 };

#include <sys/socket.h>
#include <netinet/in.h>
#include <unistd.h>
#include <assert.h>

int main(int argc, char *argv[]) {
        int list;
        int server, client;
        char buf[32];
        struct sockaddr_in addr;
        int yes = 1;

        list = socket(AF_INET, SOCK_STREAM, 0);
        assert(list >= 0);
        addr.sin_family = AF_INET;
        addr.sin_addr.s_addr = htonl((in_addr_t) INADDR_LOOPBACK);
        addr.sin_port = htons(4000);
        setsockopt(list, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes));
        assert(bind(list, (struct sockaddr *)&addr, sizeof(addr)) == 0);
        assert(listen(list, 1) == 0);

        client = socket(AF_INET, SOCK_STREAM, 0);
        assert(client >= 0);
        assert(connect(client, (struct sockaddr *)&addr, sizeof(addr)) == 0);
        server = accept(list, NULL, 0);
        assert(server);

        // 1)
        assert(write(client, "GET1\n", 5) == 5);
        // 2)
        assert(read(server, buf, 5) == 5);
        // 3)
        assert(write(server, "OK1\nOK2\n", 8) == 8);

        // 4)
        assert(read(client, buf, 4) == 4);

        // 5)
        shutdown(server, SHUT_RD);
        // 6)
        while (read(server, buf, sizeof(buf)))
                ;

        // 7)
        assert(write(client, "GET2\n", 5) == 5);

        // 8)
        assert(close(server) == 0);

        // 9)
        assert(read(client, buf, 4) == 4);
        assert(read(client, buf, 4) == 0);

        return 0;
}