From: George Thessalonikefs <george@nlnetlabs.nl>
Date: Fri, 10 Feb 2023 15:51:07 +0000 (+0100)
Subject: - Clean up iterator/iterator.c::error_response_cache() and allow for
X-Git-Tag: release-1.19.0rc1~38^2~25^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eb81761b13d1b0078e3452810f3c598a5a54deb8;p=thirdparty%2Funbound.git

- Clean up iterator/iterator.c::error_response_cache() and allow for
  better interaction with serve-expired, prefetch and cached error
  responses.
---

diff --git a/iterator/iterator.c b/iterator/iterator.c
index 2d4c43f58..5f2703f3c 100644
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -302,95 +302,65 @@ error_response(struct module_qstate* qstate, int id, int rcode)
 static int
 error_response_cache(struct module_qstate* qstate, int id, int rcode)
 {
-	if(!qstate->no_cache_store) {
-		/* store in cache */
-		struct reply_info err;
-		if(qstate->prefetch_leeway > NORR_TTL) {
-			verbose(VERB_ALGO, "error response for prefetch in cache");
-			/* attempt to adjust the cache entry prefetch */
-			if(dns_cache_prefetch_adjust(qstate->env, &qstate->qinfo,
-				NORR_TTL, qstate->query_flags))
-				return error_response(qstate, id, rcode);
-			/* if that fails (not in cache), fall through to store err */
+	struct reply_info err;
+	struct msgreply_entry* msg;
+	if(qstate->no_cache_store) {
+		return error_response(qstate, id, rcode);
+	}
+	if(qstate->prefetch_leeway > NORR_TTL) {
+		verbose(VERB_ALGO, "error response for prefetch in cache");
+		/* attempt to adjust the cache entry prefetch */
+		if(dns_cache_prefetch_adjust(qstate->env, &qstate->qinfo,
+			NORR_TTL, qstate->query_flags))
+			return error_response(qstate, id, rcode);
+		/* if that fails (not in cache), fall through to store err */
+	}
+	if((msg=msg_cache_lookup(qstate->env,
+		qstate->qinfo.qname, qstate->qinfo.qname_len,
+		qstate->qinfo.qtype, qstate->qinfo.qclass,
+		qstate->query_flags, 0,
+		qstate->env->cfg->serve_expired_ttl_reset)) != NULL) {
+		struct reply_info* rep = (struct reply_info*)msg->entry.data;
+		if(qstate->env->cfg->serve_expired &&
+			qstate->env->cfg->serve_expired_ttl_reset && rep &&
+			*qstate->env->now + qstate->env->cfg->serve_expired_ttl
+			> rep->serve_expired_ttl) {
+			verbose(VERB_ALGO, "reset serve-expired-ttl for "
+				"response in cache");
+			rep->serve_expired_ttl = *qstate->env->now +
+				qstate->env->cfg->serve_expired_ttl;
 		}
-		if(qstate->env->cfg->serve_expired) {
-			/* if serving expired contents, and such content is
-			 * already available, don't overwrite this servfail */
-			struct msgreply_entry* msg;
-			if((msg=msg_cache_lookup(qstate->env,
-				qstate->qinfo.qname, qstate->qinfo.qname_len,
-				qstate->qinfo.qtype, qstate->qinfo.qclass,
-				qstate->query_flags, 0, 1)) != NULL) {
-				struct reply_info* rep =
-					(struct reply_info*)msg->entry.data;
-				if(qstate->env->cfg->serve_expired_ttl_reset) {
-					if(rep && *qstate->env->now +
-						qstate->env->cfg->serve_expired_ttl  >
-						rep->serve_expired_ttl) {
-						verbose(VERB_ALGO, "reset "
-							"serve-expired-ttl for "
-							"response in cache");
-						rep->serve_expired_ttl =
-							*qstate->env->now +
-							qstate->env->cfg->serve_expired_ttl;
-					}
-				}
-				/* if the expired record is an error response
-				 * refresh for another NORR_TTL */
-				if(rep && *qstate->env->now > rep->ttl &&
-					(FLAGS_GET_RCODE(rep->flags) !=
-					LDNS_RCODE_NOERROR &&
-					FLAGS_GET_RCODE(rep->flags) !=
-					LDNS_RCODE_NXDOMAIN &&
-					FLAGS_GET_RCODE(rep->flags) !=
-					LDNS_RCODE_YXDOMAIN)) {
-					verbose(VERB_ALGO, "refresh TTL for "
-						"error response in cache");
-					rep->ttl = *qstate->env->now + NORR_TTL;
-				}
-				lock_rw_unlock(&msg->entry.lock);
-				return error_response(qstate, id, rcode);
-			}
-			/* serving expired contents, but nothing is cached
-			 * at all, so the servfail cache entry is useful
-			 * (stops waste of time on this servfail NORR_TTL) */
-		} else {
-			/* don't overwrite existing (non-expired) data in
-			 * cache with a servfail */
-			struct msgreply_entry* msg;
-			if((msg=msg_cache_lookup(qstate->env,
-				qstate->qinfo.qname, qstate->qinfo.qname_len,
-				qstate->qinfo.qtype, qstate->qinfo.qclass,
-				qstate->query_flags, *qstate->env->now, 0))
-				!= NULL) {
-				struct reply_info* rep = (struct reply_info*)
-					msg->entry.data;
-				if(FLAGS_GET_RCODE(rep->flags) ==
-					LDNS_RCODE_NOERROR ||
-					FLAGS_GET_RCODE(rep->flags) ==
-					LDNS_RCODE_NXDOMAIN) {
-					/* we have a good entry,
-					 * don't overwrite */
-					lock_rw_unlock(&msg->entry.lock);
-					return error_response(qstate, id, rcode);
-				}
-				lock_rw_unlock(&msg->entry.lock);
-			}
-			
+		if(rep && (FLAGS_GET_RCODE(rep->flags) ==
+			LDNS_RCODE_NOERROR ||
+			FLAGS_GET_RCODE(rep->flags) ==
+			LDNS_RCODE_NXDOMAIN ||
+			FLAGS_GET_RCODE(rep->flags) ==
+			LDNS_RCODE_YXDOMAIN) &&
+			(qstate->env->cfg->serve_expired ||
+			*qstate->env->now <= rep->ttl)) {
+			/* we have a good entry, don't overwrite */
+			lock_rw_unlock(&msg->entry.lock);
+			return error_response(qstate, id, rcode);
 		}
-		memset(&err, 0, sizeof(err));
-		err.flags = (uint16_t)(BIT_QR | BIT_RA);
-		FLAGS_SET_RCODE(err.flags, rcode);
-		err.qdcount = 1;
-		err.ttl = NORR_TTL;
-		err.prefetch_ttl = PREFETCH_TTL_CALC(err.ttl);
-		err.serve_expired_ttl = NORR_TTL;
-		/* do not waste time trying to validate this servfail */
-		err.security = sec_status_indeterminate;
-		verbose(VERB_ALGO, "store error response in message cache");
-		iter_dns_store(qstate->env, &qstate->qinfo, &err, 0, 0, 0, NULL,
-			qstate->query_flags, qstate->qstarttime);
-	}
+		lock_rw_unlock(&msg->entry.lock);
+		/* nothing interesting is cached (already error response or
+		 * expired good record when we don't serve expired), so this
+		 * servfail cache entry is useful (stops waste of time on this
+		 * servfail NORR_TTL) */
+	}
+	/* store in cache */
+	memset(&err, 0, sizeof(err));
+	err.flags = (uint16_t)(BIT_QR | BIT_RA);
+	FLAGS_SET_RCODE(err.flags, rcode);
+	err.qdcount = 1;
+	err.ttl = NORR_TTL;
+	err.prefetch_ttl = PREFETCH_TTL_CALC(err.ttl);
+	err.serve_expired_ttl = NORR_TTL;
+	/* do not waste time trying to validate this servfail */
+	err.security = sec_status_indeterminate;
+	verbose(VERB_ALGO, "store error response in message cache");
+	iter_dns_store(qstate->env, &qstate->qinfo, &err, 0, 0, 0, NULL,
+		qstate->query_flags, qstate->qstarttime);
 	return error_response(qstate, id, rcode);
 }