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.

7 files changed, 31 insertions, 56 deletions

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));