- 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.

diff --git a/doc/Changelog b/doc/Changelog
index 2d8c69e37c2110afbf5b2a832f21d20c162795ce..c97b24ec098307315f5415ee87cd6aeda2d1a5dc 100644 (file)
--- a/doc/Changelog
+++ b/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

diff --git a/pythonmod/doc/examples/example6.rst b/pythonmod/doc/examples/example6.rst
index d294fb8be618ef747c5e8d70acc665805844ab1d..fd6caf74d549261236f21017ef7870508eef91fc 100644 (file)
--- a/pythonmod/doc/examples/example6.rst
+++ b/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.
 

diff --git a/pythonmod/doc/modules/functions.rst b/pythonmod/doc/modules/functions.rst
index 43c66eb380dd702bef05dc5008f8c8ec457969bd..6ded3bf7a73a649870db457629591678413769ec 100644 (file)
--- a/pythonmod/doc/modules/functions.rst
+++ b/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)
 

diff --git a/pythonmod/examples/inplace_callbacks.py b/pythonmod/examples/inplace_callbacks.py
index 768c2d0138c543a9bcb47b196ddb01db1f606938..de375b4e12fc1cd0f72edbc52a4d6c390b05fbd7 100644 (file)
--- a/pythonmod/examples/inplace_callbacks.py
+++ b/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
 

diff --git a/services/mesh.c b/services/mesh.c
index 810865933aecff09578f2481b9a4c0f5c167674f..cd90509366f29249336d93bec64053a590252425 100644 (file)
--- a/services/mesh.c
+++ b/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.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);

diff --git a/util/data/msgreply.h b/util/data/msgreply.h
index 8d75f9b12f3ae5170fba9eca7c57e218aa958370..385780268a3ca28e6fe0c15e0aed4a83dac81647 100644 (file)
--- a/util/data/msgreply.h
+++ b/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).
  */