- Fix that if there are reply callbacks for the given rcode, those

are called per reply and a new message created if that was modified
  by the call.
- Pass the comm_reply information to the inplace_cb_reply* functions
  during the mesh state and update the documentation on that.

doc/Changelog

@@ -1,3 +1,10 @@
+15 October 2020: George
+	- Fix that if there are reply callbacks for the given rcode, those
+	  are called per reply and a new message created if that was modified
+	  by the call.
+	- Pass the comm_reply information to the inplace_cb_reply* functions
+	  during the mesh state and update the documentation on that.
+
 15 October 2020: Wouter
 	- Merge PR #326 from netblue30: DoH: implement content-length
 	  header field

pythonmod/doc/examples/example6.rst

@@ -60,7 +60,6 @@ The callback function's prototype is the following:
         :param **kwargs: Dictionary that may contain parameters added in a future
                          release. Current parameters:
             ``repinfo``: Reply information for a communication point (comm_reply).
-                         It is None when the callback happens in the mesh states.
 
         :return: True on success, False on failure.
 
@@ -105,8 +104,6 @@ The callback function's prototype is the following:
         :param **kwargs: Dictionary that may contain parameters added in a future
                          release. Current parameters:
             ``repinfo``: Reply information for a communication point (comm_reply).
-                         It is None when the callback happens in the mesh
-                         states(modules).
 
         :return: True on success, False on failure.
 
@@ -154,8 +151,6 @@ The callback function's prototype is the following:
         :param **kwargs: Dictionary that may contain parameters added in a future
                          release. Current parameters:
             ``repinfo``: Reply information for a communication point (comm_reply).
-                         It is None when the callback happens in the mesh
-                         states(modules).
 
         :return: True on success, False on failure.
 
@@ -201,8 +196,6 @@ The callback function's prototype is the following:
         :param **kwargs: Dictionary that may contain parameters added in a future
                          release. Current parameters:
             ``repinfo``: Reply information for a communication point (comm_reply).
-                         It is None when the callback happens in the mesh
-                         states(modules).
 
         :return: True on success, False on failure.
 

pythonmod/doc/modules/functions.rst

@@ -89,7 +89,7 @@ EDNS options
 Inplace callbacks
 -----------------
 
-.. function:: inplace_cb_reply(qinfo, qstate, rep, rcode, edns, opt_list_out, region)
+.. function:: inplace_cb_reply(qinfo, qstate, rep, rcode, edns, opt_list_out, region, \*\*kwargs)
 
     Function prototype for callback functions used in
     `register_inplace_cb_reply`_, `register_inplace_cb_reply_cache`_,
@@ -102,6 +102,9 @@ Inplace callbacks
     :param edns: :class:`edns_data`
     :param opt_list_out: :class:`edns_option`. EDNS option list to append options to.
     :param region: :class:`regional`
+    :param \*\*kwargs: Dictionary that may contain parameters added in a future
+                     release. Current parameters:
+        ``repinfo``: :class:`comm_reply`. Reply information for a communication point.
 
 .. function:: inplace_cb_query(qinfo, flags, qstate, addr, zone, region)
 

pythonmod/examples/inplace_callbacks.py

@@ -43,7 +43,7 @@
 #       This query returns SERVFAIL as the txt record of bogus.nlnetlabs.nl is
 #       intentionally bogus. The reply will contain an empty EDNS option
 #       with option code 65003.
-#       Unbound will also log the source address(es) of the client(s) that made
+#       Unbound will also log the source address of the client that made
 #       the request.
 #       (unbound needs to be validating for this example to work)
 
@@ -91,8 +91,6 @@ def inplace_reply_callback(qinfo, qstate, rep, rcode, edns, opt_list_out,
     :param **kwargs: Dictionary that may contain parameters added in a future
                      release. Current parameters:
         ``repinfo``: Reply information for a communication point (comm_reply).
-                     It is None when the callback happens in the mesh
-                     states(modules).
 
     :return: True on success, False on failure.
 
@@ -121,8 +119,6 @@ def inplace_cache_callback(qinfo, qstate, rep, rcode, edns, opt_list_out,
     :param **kwargs: Dictionary that may contain parameters added in a future
                      release. Current parameters:
         ``repinfo``: Reply information for a communication point (comm_reply).
-                     It is None when the callback happens in the mesh
-                     states(modules).
 
     :return: True on success, False on failure.
 
@@ -173,8 +169,6 @@ def inplace_local_callback(qinfo, qstate, rep, rcode, edns, opt_list_out,
     :param **kwargs: Dictionary that may contain parameters added in a future
                      release. Current parameters:
         ``repinfo``: Reply information for a communication point (comm_reply).
-                     It is None when the callback happens in the mesh
-                     states(modules).
 
     :return: True on success, False on failure.
 
@@ -205,13 +199,11 @@ def inplace_servfail_callback(qinfo, qstate, rep, rcode, edns, opt_list_out,
     :param **kwargs: Dictionary that may contain parameters added in a future
                      release. Current parameters:
         ``repinfo``: Reply information for a communication point (comm_reply).
-                     It is None when the callback happens in the mesh
-                     states(modules).
 
     :return: True on success, False on failure.
 
     For demonstration purposes we want to reply with an empty EDNS code '65003'
-    and log the IP address(es) of the client(s).
+    and log the IP address of the client.
 
     """
     log_info("python: called back while servfail.")
@@ -219,30 +211,14 @@ def inplace_servfail_callback(qinfo, qstate, rep, rcode, edns, opt_list_out,
     b = bytearray.fromhex("")
     edns_opt_list_append(opt_list_out, 65003, b, region)
 
-    # Log the client(s) IP address(es)
+    # Log the client's IP address
     comm_reply = kwargs['repinfo']
     if comm_reply:
-        # If it is not None this callback was called before the query reached
-        # the mesh states(modules). There is only one client associated with
-        # this query.
         addr = comm_reply.addr
         port = comm_reply.port
         addr_family = comm_reply.family
         log_info("python: Client IP: {}({}), port: {}"
                  "".format(addr, addr_family, port))
-    else:
-        # If it is not None this callback was called while the query is in the
-        # mesh states(modules). In this case they may be multiple clients
-        # waiting for this query.
-        # The following code is the same as with the resip.py example.
-        rl = qstate.mesh_info.reply_list
-        while (rl):
-            if rl.query_reply:
-                q = rl.query_reply
-                log_info("python: Client IP: {}({}), port: {}"
-                         "".format(q.addr, q.family, q.port))
-            rl = rl.next
-
 
     return True
 

services/mesh.c

@@ -1224,18 +1224,24 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 		r->h2_stream->mesh_state = NULL;
 	}
 	/* send the reply */
-	/* We don't reuse the encoded answer if either the previous or current
-	 * response has a local alias.  We could compare the alias records
-	 * and still reuse the previous answer if they are the same, but that
-	 * would be complicated and error prone for the relatively minor case.
-	 * So we err on the side of safety. */
-	if(prev && prev_buffer && prev->qflags == r->qflags && 
+	/* We don't reuse the encoded answer if:
+	 * - either the previous or current response has a local alias.  We could
+	 *   compare the alias records and still reuse the previous answer if they
+	 *   are the same, but that would be complicated and error prone for the
+	 *   relatively minor case. So we err on the side of safety.
+	 * - there are registered callback functions for the given rcode, as these
+	 *   need to be called for each reply. */
+	if(((rcode != LDNS_RCODE_SERVFAIL &&
+			!m->s.env->inplace_cb_lists[inplace_cb_reply]) ||
+		(rcode == LDNS_RCODE_SERVFAIL &&
+			!m->s.env->inplace_cb_lists[inplace_cb_reply_servfail])) &&
+		prev && prev_buffer && prev->qflags == r->qflags &&
 		!prev->local_alias && !r->local_alias &&
 		prev->edns.edns_present == r->edns.edns_present &&
 		prev->edns.bits == r->edns.bits &&
 		prev->edns.udp_size == r->edns.udp_size &&
 		edns_opt_list_compare(prev->edns.opt_list, r->edns.opt_list)
-		== 0 && !m->s.env->inplace_cb_lists[inplace_cb_reply]) {
+		== 0) {
 		/* if the previous reply is identical to this one, fix ID */
 		if(prev_buffer != r_buffer)
 			sldns_buffer_copy(r_buffer, prev_buffer);
@@ -1250,11 +1256,11 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 		m->s.qinfo.local_alias = r->local_alias;
 		if(rcode == LDNS_RCODE_SERVFAIL) {
 			if(!inplace_cb_reply_servfail_call(m->s.env, &m->s.qinfo, &m->s,
-				rep, rcode, &r->edns, NULL, m->s.region))
+				rep, rcode, &r->edns, &r->query_reply, m->s.region))
 					r->edns.opt_list = NULL;
 		} else { 
 			if(!inplace_cb_reply_call(m->s.env, &m->s.qinfo, &m->s, rep, rcode,
-				&r->edns, NULL, m->s.region))
+				&r->edns, &r->query_reply, m->s.region))
 					r->edns.opt_list = NULL;
 		}
 		error_encode(r_buffer, rcode, &m->s.qinfo, r->qid,
@@ -1271,7 +1277,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 		m->s.qinfo.qname = r->qname;
 		m->s.qinfo.local_alias = r->local_alias;
 		if(!inplace_cb_reply_call(m->s.env, &m->s.qinfo, &m->s, rep,
-			LDNS_RCODE_NOERROR, &r->edns, NULL, m->s.region) ||
+			LDNS_RCODE_NOERROR, &r->edns, &r->query_reply, m->s.region) ||
 			!apply_edns_options(&r->edns, &edns_bak,
 				m->s.env->cfg, r->query_reply.c,
 				m->s.region) ||
@@ -1281,7 +1287,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 			secure)) 
 		{
 			if(!inplace_cb_reply_servfail_call(m->s.env, &m->s.qinfo, &m->s,
-			rep, LDNS_RCODE_SERVFAIL, &r->edns, NULL, m->s.region))
+			rep, LDNS_RCODE_SERVFAIL, &r->edns, &r->query_reply, m->s.region))
 				r->edns.opt_list = NULL;
 			error_encode(r_buffer, LDNS_RCODE_SERVFAIL,
 				&m->s.qinfo, r->qid, r->qflags, &r->edns);

util/data/msgreply.h

@@ -552,7 +552,7 @@ struct edns_option* edns_opt_list_find(struct edns_option* list, uint16_t code);
  * @param rep: Reply info. Could be NULL.
  * @param rcode: return code.
  * @param edns: edns data of the reply.
- * @param repinfo: comm_reply. NULL.
+ * @param repinfo: comm_reply. Reply information for a communication point.
  * @param region: region to store data.
  * @return false on failure (a callback function returned an error).
  */