diff options
| author | Garrett D'Amore <garrett@damore.org> | 2017-08-14 11:58:38 -0700 |
|---|---|---|
| committer | Garrett D'Amore <garrett@damore.org> | 2017-08-14 13:43:01 -0700 |
| commit | 4fb81f024e5f32a186cd5538574f8e5796980e36 (patch) | |
| tree | e8fedc5f01c9f381e590f3de8986515093d63699 | |
| parent | 827aed05d352c470c8b4b8a7e232e21e1cd19313 (diff) | |
| download | nng-4fb81f024e5f32a186cd5538574f8e5796980e36.tar.gz nng-4fb81f024e5f32a186cd5538574f8e5796980e36.tar.bz2 nng-4fb81f024e5f32a186cd5538574f8e5796980e36.zip | |
REP drops peers a little too aggressively.
We noticed that certain failure modes were exposed in tests that were
caused by us closing the underlying pipe when certain messaging errors
occurred. Discarding the pipe is the wrong answer; instead we should
discard the message and keep the pipe open (unless the message is so
malformed that the remote party cannot be trusted.)
| -rw-r--r-- | src/protocol/reqrep/rep.c | 21 | ||||
| -rw-r--r-- | tests/compat_reqttl.c | 10 |
2 files changed, 19 insertions, 12 deletions
diff --git a/src/protocol/reqrep/rep.c b/src/protocol/reqrep/rep.c index 49bcdce2..4319cbf8 100644 --- a/src/protocol/reqrep/rep.c +++ b/src/protocol/reqrep/rep.c @@ -302,7 +302,8 @@ nni_rep_pipe_recv_cb(void *arg) // Store the pipe id in the header, first thing. rv = nni_msg_header_append_u32(msg, nni_pipe_id(rp->pipe)); if (rv != 0) { - goto malformed; + // Failure here causes us to drop the message. + goto drop; } // Move backtrace from body to header @@ -310,10 +311,17 @@ nni_rep_pipe_recv_cb(void *arg) for (;;) { int end = 0; if (hops >= rep->ttl) { - goto malformed; + // This isn't malformed, but it has gone through + // too many hops. Do not disconnect, because we + // can legitimately receive messages with too many + // hops from devices, etc. + goto drop; } if (nni_msg_len(msg) < 4) { - goto malformed; + // Peer is speaking garbage. Kick it. + nni_msg_free(msg); + nni_pipe_stop(rp->pipe); + return; } body = nni_msg_body(msg); end = (body[0] & 0x80) ? 1 : 0; @@ -323,7 +331,7 @@ nni_rep_pipe_recv_cb(void *arg) // We could just discard and try again, but we // just toss the connection for now. Given the // out of memory situation, this is not unreasonable. - goto malformed; + goto drop; } nni_msg_trim(msg, 4); if (end) { @@ -336,10 +344,9 @@ nni_rep_pipe_recv_cb(void *arg) nni_msgq_aio_put(rp->rep->urq, &rp->aio_putq); return; -malformed: - // Failures here are bad enough to warrant to dropping the conn. +drop: nni_msg_free(msg); - nni_pipe_stop(rp->pipe); + nni_pipe_recv(rp->pipe, &rp->aio_recv); } static void diff --git a/tests/compat_reqttl.c b/tests/compat_reqttl.c index ad9ec71a..c758f92e 100644 --- a/tests/compat_reqttl.c +++ b/tests/compat_reqttl.c @@ -57,8 +57,8 @@ int main (int argc, const char *argv[]) int port = get_test_port(argc, argv); - test_addr_from(socket_address_a, "inproc", "127.0.0.1", port); - test_addr_from(socket_address_b, "inproc", "127.0.0.1", port + 1); + test_addr_from(socket_address_a, "tcp", "127.0.0.1", port); + test_addr_from(socket_address_b, "tcp", "127.0.0.1", port + 1); /* Intialise the device sockets. */ dev0 = test_socket (AF_SP_RAW, NN_REP); @@ -78,13 +78,13 @@ int main (int argc, const char *argv[]) test_connect (end1, socket_address_b); /* Wait for TCP to establish. */ - nn_sleep (200); + nn_sleep (100); /* Pass a message between endpoints. */ /* Set up max receive timeout. */ - timeo = 500; + timeo = 100; test_setsockopt (end0, NN_SOL_SOCKET, NN_RCVTIMEO, &timeo, sizeof (timeo)); - timeo = 500; + timeo = 100; test_setsockopt (end1, NN_SOL_SOCKET, NN_RCVTIMEO, &timeo, sizeof (timeo)); /* Test default TTL is 8. */ |