Address review comments for #759:

- Decrease allocations for "" EDE strings when loading the cachedump.
- Check for existence of EDE code before attaching.

diff --git a/daemon/cachedump.c b/daemon/cachedump.c
index a90f04f2a106f121e6815e79ab2b1e21127dbfc7..61ee1d29133fa24e94e80e90d1af1ba41ea6acf1 100644 (file)
--- a/daemon/cachedump.c
+++ b/daemon/cachedump.c
@@ -651,14 +651,15 @@ load_msg(RES* ssl, sldns_buffer* buf, struct worker* worker)
        }
 
        /* read remainder of line */
-       if(sscanf(s, " %u %u " ARG_LL "d %u %u %u %u %d%n", &flags, &qdcount, &ttl,
+       /* note the last space before any possible EDE text */
+       if(sscanf(s, " %u %u " ARG_LL "d %u %u %u %u %d %n", &flags, &qdcount, &ttl,
                &security, &an, &ns, &ar, &ede, &consumed) != 8) {
                log_warn("error cannot parse numbers: %s", s);
                return 0;
        }
        /* there may be EDE text after the numbers */
        if(consumed > 0 && (size_t)consumed < strlen(s))
-               ede_str = s + consumed + 1 /* space */;
+               ede_str = s + consumed;
        memset(&rep, 0, sizeof(rep));
        rep.flags = (uint16_t)flags;
        rep.qdcount = (uint16_t)qdcount;

diff --git a/daemon/worker.c b/daemon/worker.c
index 529b22ae4141976c699cf7322e478086469e1fb6..1b988ed36c404174c441ff0fa7ab3afeb668be53 100644 (file)
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -508,7 +508,8 @@ answer_norec_from_cache(struct worker* worker, struct query_info* qinfo,
                                worker->env.now_tv))
                                        return 0;
                        /* Attach the cached EDE (RFC8914) */
-                       if(worker->env.cfg->ede) {
+                       if(worker->env.cfg->ede &&
+                               msg->rep->reason_bogus != LDNS_EDE_NONE) {
                                edns_opt_list_append_ede(&edns->opt_list_out,
                                        worker->scratchpad, msg->rep->reason_bogus,
                                        msg->rep->reason_bogus_str);
@@ -694,7 +695,7 @@ answer_from_cache(struct worker* worker, struct query_info* qinfo,
                        worker->env.now_tv))
                        goto bail_out;
                /* Attach the cached EDE (RFC8914) */
-               if(worker->env.cfg->ede) {
+               if(worker->env.cfg->ede && rep->reason_bogus != LDNS_EDE_NONE) {
                        edns_opt_list_append_ede(&edns->opt_list_out,
                                        worker->scratchpad, rep->reason_bogus,
                                        rep->reason_bogus_str);

diff --git a/services/mesh.c b/services/mesh.c
index 683c76407db1f7ab088af29528b7497633084b5f..5fa467d18ca81c978a5819279b7c3f77f6096a30 100644 (file)
--- a/services/mesh.c
+++ b/services/mesh.c
@@ -1253,7 +1253,7 @@ mesh_find_and_attach_ede_and_reason(struct mesh_state* m,
         * but a compelling reason to do otherwise is just as valid
         * NEW note:
         * The compelling reason is that with caching support, the value
-        * in the * reply_info is cached.
+        * in the reply_info is cached.
         * The reason members of the reply_info struct should be
         * updated as they are already cached. No reason to
         * try and find the EDE information in errinf anymore.