aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorGarrett D'Amore <garrett@damore.org>2017-08-14 11:58:38 -0700
committerGarrett D'Amore <garrett@damore.org>2017-08-14 13:43:01 -0700
commit4fb81f024e5f32a186cd5538574f8e5796980e36 (patch)
treee8fedc5f01c9f381e590f3de8986515093d63699 /src
parent827aed05d352c470c8b4b8a7e232e21e1cd19313 (diff)
downloadnng-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.)
Diffstat (limited to 'src')
-rw-r--r--src/protocol/reqrep/rep.c21
1 files changed, 14 insertions, 7 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