fixes #1169 survey and xsurvey could use message cloning

fixes #1160 Consider limiting maximum hop count to 15
fixes #1098 Maximum maxTTL should be compile time defined

This doesn't expose the max-MaxTTL in the CMakeList.txt -- there
is really no reason anyone should be changing it.  This does not
yet inline the message header into the nni_msg_t, but it is my
intention to do so soon, and eliminate most of the conditional cases
for failure on inserting into the header.

18 files changed, 68 insertions, 77 deletions

diff --git a/src/core/defs.h b/src/core/defs.h
index 9e320e1d..eaba7428 100644
--- a/src/core/defs.h
+++ b/src/core/defs.h
@@ -1,5 +1,5 @@
 //
-// Copyright 2019 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
 // Copyright 2018 Capitar IT Group BV <info@capitoar.com>
 //
 // This software is supplied under the terms of the MIT License, a
@@ -161,4 +161,18 @@ typedef enum {
 
 typedef nni_type nni_opt_type;
 
+// NNI_MAX_MAX_TTL is the maximum value that MAX_TTL can be set to -
+// i.e. the number of nng_device boundaries that a message can traverse.
+// This value drives the size of pre-allocated headers and back-trace
+// buffers -- we need 4 bytes for each hop, plus 4 bytes for the request
+// identifier.  Thus, it is recommended not to set this value too large.
+// (It is possible to scale out to inconceivably large networks with
+// only a few hops - we have yet to see more than 4 in practice.)
+#ifndef NNI_MAX_MAX_TTL
+#define NNI_MAX_MAX_TTL 15
+#endif
+
+// NNI_MAX_HEADER_SIZE is our header size.
+#define NNI_MAX_HEADER_SIZE ((NNI_MAX_MAX_TTL + 1) * sizeof(uint32_t))
+
 #endif // CORE_DEFS_H
diff --git a/src/core/message.c b/src/core/message.c
index 888585ad..00b23a55 100644
--- a/src/core/message.c
+++ b/src/core/message.c
@@ -470,9 +470,9 @@ nni_msg_alloc(nni_msg **mp, size_t sz)
 		return (NNG_ENOMEM);
 	}
 
-	// 64-bytes of header, including room for 32 bytes
-	// of headroom and 32 bytes of trailer.
-	if ((rv = nni_chunk_grow(&m->m_header, 32, 32)) != 0) {
+	// Header size is strictly limited.  We need max hops plus one for
+	// the request or survey ID.  TODO: Inline the header (no chunk).
+	if ((rv = nni_chunk_grow(&m->m_header, NNI_MAX_HEADER_SIZE, 0)) != 0) {
 		NNI_FREE_STRUCT(m);
 		return (rv);
 	}
diff --git a/src/protocol/pair1/pair.c b/src/protocol/pair1/pair.c
index a99a0d17..a3e87bf3 100644
--- a/src/protocol/pair1/pair.c
+++ b/src/protocol/pair1/pair.c
@@ -454,7 +454,7 @@ pair1_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	int         rv;
 	int         ttl;
 
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 
diff --git a/src/protocol/pair1/pair1_test.c b/src/protocol/pair1/pair1_test.c
index 08f808a5..7011606a 100644
--- a/src/protocol/pair1/pair1_test.c
+++ b/src/protocol/pair1/pair1_test.c
@@ -575,22 +575,22 @@ test_ttl(void)
 	nng_msg_free(msg);
 
 	// Large TTL passes
-	TEST_CHECK(nng_setopt_int(s1, NNG_OPT_MAXTTL, 0xff) == 0);
+	TEST_CHECK(nng_setopt_int(s1, NNG_OPT_MAXTTL, 15) == 0);
 	TEST_CHECK(nng_msg_alloc(&msg, 0) == 0);
 	TEST_CHECK(nng_msg_append_u32(msg, 1234) == 0);
-	TEST_CHECK(nng_msg_header_append_u32(msg, 0xfe) == 0);
+	TEST_CHECK(nng_msg_header_append_u32(msg, 14) == 0);
 	TEST_CHECK(nng_sendmsg(c1, msg, 0) == 0);
 	TEST_CHECK(nng_recvmsg(s1, &msg, 0) == 0);
 	TEST_CHECK(nng_msg_trim_u32(msg, &val) == 0);
 	TEST_CHECK(val == 1234);
 	TEST_CHECK(nng_msg_header_trim_u32(msg, &val) == 0);
-	TEST_CHECK(val == 0xff);
+	TEST_CHECK(val == 15);
 	nng_msg_free(msg);
 
 	// Max TTL fails
-	TEST_CHECK(nng_setopt_int(s1, NNG_OPT_MAXTTL, 0xff) == 0);
+	TEST_CHECK(nng_setopt_int(s1, NNG_OPT_MAXTTL, 15) == 0);
 	TEST_CHECK(nng_msg_alloc(&msg, 0) == 0);
-	TEST_CHECK(nng_msg_header_append_u32(msg, 0xff) == 0);
+	TEST_CHECK(nng_msg_header_append_u32(msg, 15) == 0);
 	TEST_CHECK(nng_sendmsg(c1, msg, 0) == 0);
 	TEST_CHECK(nng_recvmsg(s1, &msg, 0) == NNG_ETIMEDOUT);
 
diff --git a/src/protocol/reqrep0/rep.c b/src/protocol/reqrep0/rep.c
index ef3f548a..d0cc0d55 100644
--- a/src/protocol/reqrep0/rep.c
+++ b/src/protocol/reqrep0/rep.c
@@ -34,7 +34,7 @@ struct rep0_ctx {
 	nni_list_node sqnode;
 	nni_list_node rqnode;
 	size_t        btrace_len;
-	uint32_t      btrace[256]; // backtrace buffer
+	uint32_t      btrace[NNI_MAX_MAX_TTL + 1];
 };
 
 // rep0_sock is our per-socket protocol private structure.
@@ -573,7 +573,7 @@ rep0_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	int        ttl;
 	int        rv;
 
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/reqrep0/req.c b/src/protocol/reqrep0/req.c
index b8ca498d..fea95725 100644
--- a/src/protocol/reqrep0/req.c
+++ b/src/protocol/reqrep0/req.c
@@ -666,12 +666,9 @@ req0_ctx_send(void *arg, nni_aio *aio)
 		return;
 	}
 	ctx->request_id = (uint32_t) id;
-	if ((rv = nni_msg_header_append_u32(msg, ctx->request_id)) != 0) {
-		nni_idhash_remove(s->requests, id);
-		nni_mtx_unlock(&s->mtx);
-		nni_aio_finish_error(aio, rv);
-		return;
-	}
+	nni_msg_header_clear(msg);
+	nni_msg_header_must_append_u32(msg, ctx->request_id);
+
 	// If no pipes are ready, and the request was a poll (no background
 	// schedule), then fail it.  Should be NNG_ETIMEDOUT.
 	rv = nni_aio_schedule(aio, req0_ctx_cancel_send, ctx);
@@ -713,7 +710,7 @@ req0_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	req0_sock *s = arg;
 	int        ttl;
 	int        rv;
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/reqrep0/req_test.c b/src/protocol/reqrep0/req_test.c
index 2524dfd2..6cb06f62 100644
--- a/src/protocol/reqrep0/req_test.c
+++ b/src/protocol/reqrep0/req_test.c
@@ -52,6 +52,9 @@ test_req_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(req, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(req, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(req, opt, -1), NNG_EINVAL);
+	// This test will fail if the NNI_MAX_MAX_TTL is changed from the
+	// builtin default of 15.
+	TEST_NNG_FAIL(nng_setopt_int(req, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(req, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(req, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(req, opt, &v));
diff --git a/src/protocol/reqrep0/xrep.c b/src/protocol/reqrep0/xrep.c
index e63cfe83..901cecc4 100644
--- a/src/protocol/reqrep0/xrep.c
+++ b/src/protocol/reqrep0/xrep.c
@@ -355,7 +355,7 @@ xrep0_sock_set_maxttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	xrep0_sock *s = arg;
 	int         ttl;
 	int         rv;
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/reqrep0/xrep_test.c b/src/protocol/reqrep0/xrep_test.c
index 33d834df..8fc36964 100644
--- a/src/protocol/reqrep0/xrep_test.c
+++ b/src/protocol/reqrep0/xrep_test.c
@@ -337,6 +337,7 @@ test_xrep_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(rep, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, -1), NNG_EINVAL);
+	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(rep, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(rep, opt, &v));
diff --git a/src/protocol/reqrep0/xreq.c b/src/protocol/reqrep0/xreq.c
index 4900fb06..bcb218bf 100644
--- a/src/protocol/reqrep0/xreq.c
+++ b/src/protocol/reqrep0/xreq.c
@@ -257,7 +257,7 @@ xreq0_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	xreq0_sock *s = arg;
 	int         ttl;
 	int         rv;
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/reqrep0/xreq_test.c b/src/protocol/reqrep0/xreq_test.c
index 57ad7f0a..68a7c7f5 100644
--- a/src/protocol/reqrep0/xreq_test.c
+++ b/src/protocol/reqrep0/xreq_test.c
@@ -337,6 +337,7 @@ test_xreq_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(rep, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, -1), NNG_EINVAL);
+	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(rep, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(rep, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(rep, opt, &v));
diff --git a/src/protocol/survey0/respond.c b/src/protocol/survey0/respond.c
index 19dedef0..580aa743 100644
--- a/src/protocol/survey0/respond.c
+++ b/src/protocol/survey0/respond.c
@@ -43,7 +43,7 @@ struct resp0_ctx {
 	nni_list_node sqnode;
 	nni_list_node rqnode;
 	size_t        btrace_len;
-	uint32_t      btrace[256];
+	uint32_t      btrace[NNI_MAX_MAX_TTL + 1];
 };
 
 // resp0_sock is our per-socket protocol private structure.
@@ -565,7 +565,7 @@ resp0_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	int         ttl;
 	int         rv;
 
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/survey0/respond_test.c b/src/protocol/survey0/respond_test.c
index 3c211843..2801222c 100644
--- a/src/protocol/survey0/respond_test.c
+++ b/src/protocol/survey0/respond_test.c
@@ -507,6 +507,7 @@ test_resp_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(resp, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, -1), NNG_EINVAL);
+	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(resp, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(resp, opt, &v));
diff --git a/src/protocol/survey0/survey.c b/src/protocol/survey0/survey.c
index 9746ff45..dcf92e07 100644
--- a/src/protocol/survey0/survey.c
+++ b/src/protocol/survey0/survey.c
@@ -39,11 +39,11 @@ struct surv0_ctx {
 
 // surv0_sock is our per-socket protocol private structure.
 struct surv0_sock {
-	int           ttl;
-	nni_list      pipes;
-	nni_mtx       mtx;
-	surv0_ctx     ctx;
-	nni_idhash *  surveys;
+	int          ttl;
+	nni_list     pipes;
+	nni_mtx      mtx;
+	surv0_ctx    ctx;
+	nni_idhash * surveys;
 	nni_pollable writable;
 };
 
@@ -161,34 +161,18 @@ surv0_ctx_send(void *arg, nni_aio *aio)
 		nni_aio_finish_error(aio, rv);
 		return;
 	}
-	// Insert it into the message.  We report an error if one occurs,
-	// although arguably at this point we could just discard silently.
-	if ((rv = nni_msg_header_append_u32(msg, (uint32_t) ctx->survid)) !=
-	    0) {
-		nni_idhash_remove(sock->surveys, ctx->survid);
-		ctx->survid = 0;
-		nni_mtx_unlock(&sock->mtx);
-		nni_aio_finish_error(aio, rv);
-		return;
-	}
+	nni_msg_header_clear(msg);
+	nni_msg_header_must_append_u32(msg, (uint32_t) ctx->survid);
 
 	// From this point, we're committed to success.  Note that we send
 	// regardless of whether there are any pipes or not.  If no pipes,
 	// then it just gets discarded.
 	nni_aio_set_msg(aio, NULL);
 	NNI_LIST_FOREACH (&sock->pipes, pipe) {
-		nni_msg *dmsg;
-
-		if (nni_list_next(&sock->pipes, pipe) != NULL) {
-			if (nni_msg_dup(&dmsg, msg) != 0) {
-				continue;
-			}
-		} else {
-			dmsg = msg;
-			msg  = NULL;
-		}
-		if (nni_msgq_tryput(pipe->sendq, dmsg) != 0) {
-			nni_msg_free(dmsg);
+
+		nni_msg_clone(msg);
+		if (nni_msgq_tryput(pipe->sendq, msg) != 0) {
+			nni_msg_free(msg);
 		}
 	}
 
@@ -199,9 +183,7 @@ surv0_ctx_send(void *arg, nni_aio *aio)
 	nni_msgq_set_get_error(ctx->rq, 0);
 
 	nni_mtx_unlock(&sock->mtx);
-	if (msg != NULL) {
-		nni_msg_free(msg);
-	}
+	nni_msg_free(msg);
 
 	nni_aio_finish(aio, 0, len);
 }
@@ -441,7 +423,7 @@ static int
 surv0_sock_set_maxttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 {
 	surv0_sock *s = arg;
-	return (nni_copyin_int(&s->ttl, buf, sz, 1, 255, t));
+	return (nni_copyin_int(&s->ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t));
 }
 
 static int
diff --git a/src/protocol/survey0/xrespond.c b/src/protocol/survey0/xrespond.c
index 73e541d3..c664f009 100644
--- a/src/protocol/survey0/xrespond.c
+++ b/src/protocol/survey0/xrespond.c
@@ -284,9 +284,7 @@ xresp0_recv_cb(void *arg)
 	nni_msg_set_pipe(msg, p->id);
 
 	// Store the pipe id in the header, first thing.
-	if (nni_msg_header_append_u32(msg, p->id) != 0) {
-		goto drop;
-	}
+	nni_msg_header_must_append_u32(msg, p->id);
 
 	// Move backtrace from body to header
 	hops = 1;
@@ -346,7 +344,7 @@ xresp0_sock_set_maxttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	xresp0_sock *s = arg;
 	int ttl;
 	int rv;
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
diff --git a/src/protocol/survey0/xrespond_test.c b/src/protocol/survey0/xrespond_test.c
index eec5c4a6..342c8a94 100644
--- a/src/protocol/survey0/xrespond_test.c
+++ b/src/protocol/survey0/xrespond_test.c
@@ -337,6 +337,7 @@ test_xresp_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(resp, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, -1), NNG_EINVAL);
+	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(resp, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(resp, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(resp, opt, &v));
@@ -428,8 +429,10 @@ TEST_LIST = {
 	{ "xrespond poll readable", test_xresp_poll_readable },
 	{ "xrespond poll writable", test_xresp_poll_writeable },
 	{ "xrespond validate peer", test_xresp_validate_peer },
-	{ "xrespond close pipe before send", test_xresp_close_pipe_before_send },
-	{ "xrespond close pipe during send", test_xresp_close_pipe_during_send },
+	{ "xrespond close pipe before send",
+	    test_xresp_close_pipe_before_send },
+	{ "xrespond close pipe during send",
+	    test_xresp_close_pipe_during_send },
 	{ "xrespond close during recv", test_xresp_close_during_recv },
 	{ "xrespond recv aio stopped", test_xresp_recv_aio_stopped },
 	{ "xrespond send no header", test_xresp_send_no_header },
diff --git a/src/protocol/survey0/xsurvey.c b/src/protocol/survey0/xsurvey.c
index 7a5a5c1b..2a198662 100644
--- a/src/protocol/survey0/xsurvey.c
+++ b/src/protocol/survey0/xsurvey.c
@@ -273,7 +273,7 @@ xsurv0_sock_set_max_ttl(void *arg, const void *buf, size_t sz, nni_opt_type t)
 	xsurv0_sock *s = arg;
 	int          ttl;
 	int          rv;
-	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, 255, t)) == 0) {
+	if ((rv = nni_copyin_int(&ttl, buf, sz, 1, NNI_MAX_MAX_TTL, t)) == 0) {
 		nni_atomic_set(&s->ttl, ttl);
 	}
 	return (rv);
@@ -291,8 +291,7 @@ xsurv0_sock_getq_cb(void *arg)
 {
 	xsurv0_sock *s = arg;
 	xsurv0_pipe *p;
-	xsurv0_pipe *last;
-	nni_msg *    msg, *dup;
+	nni_msg *    msg;
 
 	if (nni_aio_result(&s->aio_getq) != 0) {
 		// Should be NNG_ECLOSED.
@@ -302,27 +301,18 @@ xsurv0_sock_getq_cb(void *arg)
 	nni_aio_set_msg(&s->aio_getq, NULL);
 
 	nni_mtx_lock(&s->mtx);
-	last = nni_list_last(&s->pipes);
 	NNI_LIST_FOREACH (&s->pipes, p) {
-		if (p != last) {
-			if (nni_msg_dup(&dup, msg) != 0) {
-				continue;
-			}
-		} else {
-			dup = msg;
-		}
-		if (nni_msgq_tryput(p->sendq, dup) != 0) {
-			nni_msg_free(dup);
+		nni_msg_clone(msg);
+		if (nni_msgq_tryput(p->sendq, msg) != 0) {
+			nni_msg_free(msg);
 		}
 	}
 
 	nni_msgq_aio_get(s->uwq, &s->aio_getq);
 	nni_mtx_unlock(&s->mtx);
 
-	if (last == NULL) {
-		// If there were no pipes to send on, just toss the message.
-		nni_msg_free(msg);
-	}
+	// If there were no pipes to send on, just toss the message.
+	nni_msg_free(msg);
 }
 
 static void
diff --git a/src/protocol/survey0/xsurvey_test.c b/src/protocol/survey0/xsurvey_test.c
index ff096de4..ca7a2dd6 100644
--- a/src/protocol/survey0/xsurvey_test.c
+++ b/src/protocol/survey0/xsurvey_test.c
@@ -340,6 +340,7 @@ test_xsurvey_ttl_option(void)
 	TEST_NNG_PASS(nng_setopt_int(s, opt, 1));
 	TEST_NNG_FAIL(nng_setopt_int(s, opt, 0), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(s, opt, -1), NNG_EINVAL);
+	TEST_NNG_FAIL(nng_setopt_int(s, opt, 16), NNG_EINVAL);
 	TEST_NNG_FAIL(nng_setopt_int(s, opt, 256), NNG_EINVAL);
 	TEST_NNG_PASS(nng_setopt_int(s, opt, 3));
 	TEST_NNG_PASS(nng_getopt_int(s, opt, &v));