From 6752479a6aa18ce972a3b3997d0c55ae5bfb1550 Mon Sep 17 00:00:00 2001
From: Jan Kadlec <jan.kadlec@nic.cz>
Date: Mon, 24 Feb 2014 11:07:46 +0100
Subject: [PATCH] Moved TTL checks into semantic checks.

- RR insertion function now returns zone RRSet.
- Removed reduntant zone contents / node API functions
- Cleanup.
---
 src/knot/zone/node.c           | 35 ++++------------
 src/knot/zone/node.h           |  4 +-
 src/knot/zone/semantic-check.c | 64 +++++++++++++++++++++++++----
 src/knot/zone/semantic-check.h | 37 ++++++++++++++---
 src/knot/zone/zone-contents.c  | 75 ++++------------------------------
 src/knot/zone/zone-contents.h  | 15 +------
 src/knot/zone/zone-create.c    |  9 +++-
 src/libknot/rrset-dump.c       |  2 +-
 8 files changed, 115 insertions(+), 126 deletions(-)

diff --git a/src/knot/zone/node.c b/src/knot/zone/node.c
index b6b06e49b..9191261ed 100644
--- a/src/knot/zone/node.c
+++ b/src/knot/zone/node.c
@@ -68,31 +68,6 @@ static inline void knot_node_flags_clear(knot_node_t *node, uint8_t flag)
 	node->flags &= ~flag;
 }
 
-/*----------------------------------------------------------------------------*/
-/*!
- * \brief Logs a warning if merging RRs with different TTLs.
- *
- * \param ttl_first TTL of the first RR in the RRSet.
- * \param ttl_new TTL to be inserted.
- * \param rr RRSet we're adding into.
- * \param zname Zone name for logging.
- */
-static void ttl_check(uint32_t ttl_first, uint32_t ttl_new,
-                      knot_rrset_t *rr, const knot_dname_t *zname)
-{
-	if (rr->type != KNOT_RRTYPE_RRSIG && (ttl_first != ttl_new) && zname) {
-		char *zstr = knot_dname_to_str(zname);
-		char *rrstr = knot_dname_to_str(rr->owner);
-		char typestr[32];
-		knot_rrtype_to_string(rr->type, typestr, 32);
-		log_zone_warning("Zone %s: Record %s, type %s: "
-		                 "Inserting different TTL into RRSet.\n",
-		                 zstr, rrstr, typestr);
-		free(zstr);
-		free(rrstr);
-	}
-}
-
 /*----------------------------------------------------------------------------*/
 /* API functions                                                              */
 /*----------------------------------------------------------------------------*/
@@ -158,7 +133,7 @@ int knot_node_add_rrset_replace(knot_node_t *node, knot_rrset_t *rrset)
 }
 
 int knot_node_add_rrset(knot_node_t *node, knot_rrset_t *rrset,
-                        const knot_dname_t *zname)
+                        knot_rrset_t **out_rrset)
 {
 	if (node == NULL) {
 		return KNOT_EINVAL;
@@ -166,8 +141,9 @@ int knot_node_add_rrset(knot_node_t *node, knot_rrset_t *rrset,
 
 	for (uint16_t i = 0; i < node->rrset_count; ++i) {
 		if (node->rrset_tree[i]->type == rrset->type) {
-			ttl_check(knot_rrset_rr_ttl(node->rrset_tree[i], 0),
-			          knot_rrset_rr_ttl(rrset, 0), rrset, zname);
+			if (out_rrset) {
+				*out_rrset = node->rrset_tree[i];
+			}
 			int merged, deleted_rrs;
 			int ret = knot_rrset_merge_sort(node->rrset_tree[i],
 			                                rrset, &merged,
@@ -183,6 +159,9 @@ int knot_node_add_rrset(knot_node_t *node, knot_rrset_t *rrset,
 	}
 
 	// New RRSet (with one RR)
+	if (out_rrset) {
+		*out_rrset = rrset;
+	}
 	return knot_node_add_rrset_no_merge(node, rrset);
 }
 
diff --git a/src/knot/zone/node.h b/src/knot/zone/node.h
index b70d02757..aeddf2f94 100644
--- a/src/knot/zone/node.h
+++ b/src/knot/zone/node.h
@@ -123,13 +123,13 @@ knot_node_t *knot_node_new(const knot_dname_t *owner, knot_node_t *parent,
  *
  * \param node Node to add the RRSet to.
  * \param rrset RRSet to add.
- * \param zname Zone owner for error logging.
+ * \param rrset Stores destination RRSet in case of merge.
  *
  * \retval KNOT_EOK on success.
  * \retval KNOT_ERROR if the RRSet could not be inserted.
  */
 int knot_node_add_rrset(knot_node_t *node, knot_rrset_t *rrset,
-                        const knot_dname_t *zname);
+                        knot_rrset_t **out_rrset);
 
 int knot_node_add_rrset_replace(knot_node_t *node, knot_rrset_t *rrset);
 
diff --git a/src/knot/zone/semantic-check.c b/src/knot/zone/semantic-check.c
index 90dfe36c2..f3d0b1c5d 100644
--- a/src/knot/zone/semantic-check.c
+++ b/src/knot/zone/semantic-check.c
@@ -39,6 +39,7 @@ static char *error_messages[(-ZC_ERR_UNKNOWN) + 1] = {
 	[-ZC_ERR_MISSING_SOA] = "SOA record missing in zone!",
 	[-ZC_ERR_MISSING_NS_DEL_POINT] = "NS record missing in zone apex or in "
 	                "delegation point!",
+	[-ZC_ERR_TTL_MISMATCH] = "RRSet TTLs mismatched!",
 
 	[-ZC_ERR_RRSIG_RDATA_TYPE_COVERED] =
 	"RRSIG: Type covered RDATA field is wrong!",
@@ -248,6 +249,47 @@ void err_handler_log_all(err_handler_t *handler)
 	}
 }
 
+/* TODO: optimize */
+static bool rrset_ttls_equal(const knot_rrset_t *rrset)
+{
+	uint16_t rr_count = knot_rrset_rr_count(rrset);
+	if (rr_count == 0) {
+		return true;
+	}
+
+	uint32_t prev_ttl = knot_rrset_rr_ttl(rrset, 0);
+	for (uint16_t i = 1; i < rr_count; ++i) {
+		uint32_t cur_ttl = knot_rrset_rr_ttl(rrset, i);
+		if (cur_ttl != prev_ttl) {
+			return false;
+		}
+		prev_ttl = cur_ttl;
+	}
+
+	return true;
+}
+
+/*!
+ * \brief Logs a warning if merging RRs with different TTLs.
+ *
+ * \param ttl_first TTL of the first RR in the RRSet.
+ * \param ttl_new TTL to be inserted.
+ * \param rr RRSet we're adding into.
+ * \param zname Zone name for logging.
+ */
+static void rrset_ttl_check(err_handler_t *handler,
+                            const knot_rrset_t *rr, const knot_node_t *n)
+{
+	if (rr->type != KNOT_RRTYPE_RRSIG && !rrset_ttls_equal(rr)) {
+		/* Prepare additional info string. */
+		char info_str[64] = { '\0' };
+		char type_str[16] = { '\0' };
+		knot_rrtype_to_string(rr->type, type_str, sizeof(type_str));
+		snprintf(info_str, sizeof(info_str), "Record type: %s.", type_str);
+		err_handler_handle_error(handler, n, ZC_ERR_TTL_MISMATCH, info_str);
+	}
+}
+
 /*!
  * \brief Check whether DNSKEY rdata are valid.
  *
@@ -594,8 +636,6 @@ static int rdata_nsec_to_type_array(const knot_rrset_t *rrset, size_t pos,
 	return KNOT_EOK;
 }
 
-/* should write error, not return values !!! */
-
 /*!
  * \brief Semantic check - check node's NSEC node.
  *
@@ -694,7 +734,6 @@ static int check_nsec3_node_in_zone(knot_zone_contents_t *zone,
 	                                                    next_dname_size,
 	                                                    apex->owner);
 	if (next_dname == NULL) {
-		log_zone_warning("Could not create new dname!\n");
 		return KNOT_ERROR;
 	}
 
@@ -721,13 +760,12 @@ static int check_nsec3_node_in_zone(knot_zone_contents_t *zone,
 		/* test for each type's presence */
 		type = array[j];
 		if (type == KNOT_RRTYPE_RRSIG) {
-		       continue;
+			continue;
 		}
 		
-		if (knot_node_rrset(node,
-				      type) == NULL) {
+		if (knot_node_rrset(node, type) == NULL) {
 			err_handler_handle_error(handler, node,
-						 ZC_ERR_NSEC3_RDATA_BITMAP,
+			                         ZC_ERR_NSEC3_RDATA_BITMAP,
 			                         NULL);
 		}
 	}
@@ -907,6 +945,18 @@ int sem_check_node_plain(const knot_zone_contents_t *zone,
 	}
 }
 
+int sem_check_rrset(const knot_node_t *node,
+                    const knot_rrset_t *rrset,
+                    err_handler_t *handler)
+{
+	if (node == NULL || rrset == NULL || handler == NULL) {
+		return KNOT_EINVAL;
+	}
+
+	rrset_ttl_check(handler, rrset, node);
+	return KNOT_EOK;
+}
+
 /*!
  * \brief Run semantic checks for node with DNSSEC-related types.
  *
diff --git a/src/knot/zone/semantic-check.h b/src/knot/zone/semantic-check.h
index 6d2520e1b..649db7600 100644
--- a/src/knot/zone/semantic-check.h
+++ b/src/knot/zone/semantic-check.h
@@ -45,8 +45,9 @@ enum zonechecks_errors {
 
 	ZC_ERR_MISSING_SOA,
 	ZC_ERR_MISSING_NS_DEL_POINT,
+	ZC_ERR_TTL_MISMATCH,
 
-	ZC_ERR_GENERIC_GENERAL_ERROR, /* isn't there a better name? */
+	ZC_ERR_GENERIC_GENERAL_ERROR, /* Generic error delimiter. */
 
 	ZC_ERR_RRSIG_RDATA_TYPE_COVERED,
 	ZC_ERR_RRSIG_RDATA_TTL,
@@ -61,7 +62,7 @@ enum zonechecks_errors {
 	ZC_ERR_RRSIG_CLASS,
 	ZC_ERR_RRSIG_TTL,
 
-	ZC_ERR_RRSIG_GENERAL_ERROR,
+	ZC_ERR_RRSIG_GENERAL_ERROR, /* RRSIG error delimiter. */
 
 	ZC_ERR_NO_NSEC,
 	ZC_ERR_NSEC_RDATA_BITMAP,
@@ -69,7 +70,7 @@ enum zonechecks_errors {
 	ZC_ERR_NSEC_RDATA_CHAIN,
 	ZC_ERR_NSEC_RDATA_CHAIN_NOT_CYCLIC,
 
-	ZC_ERR_NSEC_GENERAL_ERROR,
+	ZC_ERR_NSEC_GENERAL_ERROR, /* NSEC error delimiter. */
 
 	ZC_ERR_NSEC3_UNSECURED_DELEGATION,
 	ZC_ERR_NSEC3_NOT_FOUND,
@@ -79,7 +80,7 @@ enum zonechecks_errors {
 	ZC_ERR_NSEC3_RDATA_BITMAP,
 	ZC_ERR_NSEC3_EXTRA_RECORD,
 
-	ZC_ERR_NSEC3_GENERAL_ERROR,
+	ZC_ERR_NSEC3_GENERAL_ERROR, /* NSEC3 error delimiter. */
 
 	ZC_ERR_CNAME_EXTRA_RECORDS,
 	ZC_ERR_DNAME_CHILDREN,
@@ -89,12 +90,12 @@ enum zonechecks_errors {
 	ZC_ERR_CNAME_WILDCARD_SELF,
 	ZC_ERR_DNAME_WILDCARD_SELF,
 
-	ZC_ERR_CNAME_GENERAL_ERROR,
+	ZC_ERR_CNAME_GENERAL_ERROR, /* CNAME/DNAME error delimiter. */
 
 	ZC_ERR_GLUE_NODE,
 	ZC_ERR_GLUE_RECORD,
 
-	ZC_ERR_GLUE_GENERAL_ERROR,
+	ZC_ERR_GLUE_GENERAL_ERROR, /* GLUE error delimiter. */
 };
 
 /*!
@@ -206,12 +207,36 @@ int zone_do_sem_checks(knot_zone_contents_t *zone, int check_level,
                        err_handler_t *handler, knot_node_t *first_nsec3_node,
                        knot_node_t *last_nsec3_node);
 
+/*!
+ * \brief Does a non-DNSSEC semantic node check. Logs errors via error handler.
+ *
+ * \param zone            Zone containing the node.
+ * \param node            Node to be tested.
+ * \param handler         Error handler.
+ * \param only_mandatory  Mandatory/optional switch.
+ * \param fatal_error     Fatal error out param.
+ *
+ * \return KNOT_E*
+ */
 int sem_check_node_plain(const knot_zone_contents_t *zone,
                          const knot_node_t *node,
                          err_handler_t *handler,
                          bool only_mandatory,
                          bool *fatal_error);
 
+/*!
+ * \brief Checks RRSet for semantic errors. Logs errors via error handler.
+ *
+ * \param node     Node containg the RRSet.
+ * \param rrset    RRSet to be tested.
+ * \param handler  Error handler.
+ *
+ * \return KNOT_E*
+ */
+int sem_check_rrset(const knot_node_t *node,
+                    const knot_rrset_t *rrset,
+                    err_handler_t *handler);
+
 #endif // _KNOT_SEMANTIC_CHECK_H_
 
 /*! @} */
diff --git a/src/knot/zone/zone-contents.c b/src/knot/zone/zone-contents.c
index f8214c21a..ff1ed14ee 100644
--- a/src/knot/zone/zone-contents.c
+++ b/src/knot/zone/zone-contents.c
@@ -476,14 +476,6 @@ int knot_zone_contents_gen_is_new(const knot_zone_contents_t *contents)
 
 /*----------------------------------------------------------------------------*/
 
-int knot_zone_contents_gen_is_finished(const knot_zone_contents_t *contents)
-{
-	return ((contents->flags & KNOT_ZONE_FLAGS_GEN_MASK)
-		== KNOT_ZONE_FLAGS_GEN_FIN);
-}
-
-/*----------------------------------------------------------------------------*/
-
 void knot_zone_contents_set_gen_old(knot_zone_contents_t *contents)
 {
 	contents->flags &= ~KNOT_ZONE_FLAGS_GEN_MASK;
@@ -500,14 +492,6 @@ void knot_zone_contents_set_gen_new(knot_zone_contents_t *contents)
 
 /*----------------------------------------------------------------------------*/
 
-void knot_zone_contents_set_gen_new_finished(knot_zone_contents_t *contents)
-{
-	contents->flags &= ~KNOT_ZONE_FLAGS_GEN_MASK;
-	contents->flags |= KNOT_ZONE_FLAGS_GEN_FIN;
-}
-
-/*----------------------------------------------------------------------------*/
-
 uint16_t knot_zone_contents_class(const knot_zone_contents_t *contents)
 {
 	if (contents == NULL || contents->apex == NULL
@@ -649,9 +633,9 @@ int knot_zone_contents_create_node(knot_zone_contents_t *contents,
 /*----------------------------------------------------------------------------*/
 
 static int insert_rr(knot_zone_contents_t *z, knot_rrset_t *rr, knot_node_t **n,
-                     bool nsec3)
+                     knot_rrset_t **rrset, bool nsec3)
 {
-	if (z == NULL || rr == NULL || n == NULL) {
+	if (z == NULL || rr == NULL || n == NULL || rrset == NULL) {
 		return KNOT_EINVAL;
 	}
 
@@ -679,7 +663,7 @@ static int insert_rr(knot_zone_contents_t *z, knot_rrset_t *rr, knot_node_t **n,
 		}
 	}
 
-	return knot_node_add_rrset(*n, rr, z->apex->owner);
+	return knot_node_add_rrset(*n, rr, rrset);
 }
 
 static bool to_nsec3_tree(const knot_rrset_t *rr)
@@ -688,9 +672,10 @@ static bool to_nsec3_tree(const knot_rrset_t *rr)
 }
 
 int knot_zone_contents_add_rr(knot_zone_contents_t *z,
-                              knot_rrset_t *rr, knot_node_t **n)
+                              knot_rrset_t *rr, knot_node_t **n,
+                              knot_rrset_t **rrset)
 {
-	return insert_rr(z, rr, n, to_nsec3_tree(rr));
+	return insert_rr(z, rr, n, rrset, to_nsec3_tree(rr));
 }
 
 int knot_zone_contents_add_rrset(knot_zone_contents_t *zone,
@@ -728,7 +713,7 @@ dbg_zone_exec_detail(
 
 	/*! \todo REMOVE RRSET */
 	if (dupl == KNOT_RRSET_DUPL_MERGE) {
-		rc = knot_node_add_rrset(*node, rrset, zone->apex->owner);
+		rc = knot_node_add_rrset(*node, rrset, NULL);
 	} else {
 		rc = knot_node_add_rrset_no_merge(*node, rrset);
 	}
@@ -782,52 +767,6 @@ int knot_zone_contents_add_nsec3_node(knot_zone_contents_t *zone,
 
 /*----------------------------------------------------------------------------*/
 
-int knot_zone_contents_add_nsec3_rrset(knot_zone_contents_t *zone,
-                                         knot_rrset_t *rrset,
-                                         knot_node_t **node,
-                                         knot_rrset_dupl_handling_t dupl)
-{
-	if (zone == NULL || rrset == NULL || zone->apex == NULL
-	    || zone->apex->owner == NULL || node == NULL) {
-		return KNOT_EINVAL;
-	}
-
-	// check if the RRSet belongs to the zone
-	if (knot_dname_cmp(knot_rrset_owner(rrset),
-				 zone->apex->owner) != 0
-	    && !knot_dname_is_sub(knot_rrset_owner(rrset),
-					  zone->apex->owner)) {
-		return KNOT_EOUTOFZONE;
-	}
-
-	if ((*node) == NULL
-	    && (*node = knot_zone_contents_get_nsec3_node(
-			      zone, knot_rrset_owner(rrset))) == NULL) {
-		return KNOT_ENONODE;
-	}
-
-	assert(*node != NULL);
-	int rc;
-
-	/*! \todo REMOVE RRSET */
-	if (dupl == KNOT_RRSET_DUPL_MERGE) {
-		rc = knot_node_add_rrset(*node, rrset, zone->apex->owner);
-	} else {
-		rc = knot_node_add_rrset_no_merge(*node, rrset);
-	}
-
-	if (rc < 0) {
-		return rc;
-	}
-
-	int ret = rc;
-
-	dbg_zone_detail("NSEC3 OK\n");
-	return ret;
-}
-
-/*----------------------------------------------------------------------------*/
-
 int knot_zone_contents_remove_node(knot_zone_contents_t *contents,
                                    const knot_dname_t *owner)
 {
diff --git a/src/knot/zone/zone-contents.h b/src/knot/zone/zone-contents.h
index bfba426d9..8dabc44c3 100644
--- a/src/knot/zone/zone-contents.h
+++ b/src/knot/zone/zone-contents.h
@@ -88,11 +88,9 @@ knot_zone_contents_t *knot_zone_contents_new(const knot_dname_t *apex_name);
 
 int knot_zone_contents_gen_is_old(const knot_zone_contents_t *contents);
 int knot_zone_contents_gen_is_new(const knot_zone_contents_t *contents);
-int knot_zone_contents_gen_is_finished(const knot_zone_contents_t *contents);
 
 void knot_zone_contents_set_gen_old(knot_zone_contents_t *contents);
 void knot_zone_contents_set_gen_new(knot_zone_contents_t *contents);
-void knot_zone_contents_set_gen_new_finished(knot_zone_contents_t *contents);
 
 uint16_t knot_zone_contents_class(const knot_zone_contents_t *contents);
 
@@ -131,7 +129,8 @@ int knot_zone_contents_create_node(knot_zone_contents_t *contents,
                                    knot_node_t **node);
 
 int knot_zone_contents_add_rr(knot_zone_contents_t *z,
-                              knot_rrset_t *rr, knot_node_t **n);
+                              knot_rrset_t *rr, knot_node_t **n,
+                              knot_rrset_t **rrset);
 
 /*!
  * \brief Adds a RRSet to the given zone.
@@ -159,11 +158,6 @@ int knot_zone_contents_add_rrset(knot_zone_contents_t *contents,
                                  knot_rrset_t *rrset, knot_node_t **node,
                                  knot_rrset_dupl_handling_t dupl);
 
-int knot_zone_contents_add_rrsigs(knot_zone_contents_t *contents,
-                           knot_rrset_t *rrsigs,
-                           knot_rrset_t **rrset, knot_node_t **node,
-                           knot_rrset_dupl_handling_t dupl);
-
 /*!
  * \brief Adds a node holding NSEC3 records to the given zone.
  *
@@ -183,11 +177,6 @@ int knot_zone_contents_add_nsec3_node(knot_zone_contents_t *contents,
                                         knot_node_t *node, int create_parents,
                                         uint8_t flags);
 
-int knot_zone_contents_add_nsec3_rrset(knot_zone_contents_t *contents,
-                                         knot_rrset_t *rrset,
-                                         knot_node_t **node,
-                                         knot_rrset_dupl_handling_t dupl);
-
 int knot_zone_contents_remove_node(knot_zone_contents_t *contents,
 	const knot_dname_t *owner);
 
diff --git a/src/knot/zone/zone-create.c b/src/knot/zone/zone-create.c
index e48af3e1e..c023078f2 100644
--- a/src/knot/zone/zone-create.c
+++ b/src/knot/zone/zone-create.c
@@ -98,7 +98,8 @@ int zcreator_step(zcreator_t *zc, knot_rrset_t *rr)
 		// Search for node or create a new one
 		n = NULL;
 	}
-	int ret = knot_zone_contents_add_rr(zc->z, rr, &n);
+	knot_rrset_t *zone_rrset = NULL;
+	int ret = knot_zone_contents_add_rr(zc->z, rr, &n, &zone_rrset);
 	if (ret < 0) {
 		if (!handle_err(zc, rr, ret)) {
 			// Fatal error
@@ -111,11 +112,17 @@ int zcreator_step(zcreator_t *zc, knot_rrset_t *rr)
 		knot_rrset_deep_free(&rr, true, NULL);
 	}
 	assert(n);
+	assert(zone_rrset);
 	zc->last_node = n;
 
+	// Do RRSet and node semantic checks
 	bool sem_fatal_error = false;
 	err_handler_t err_handler;
 	err_handler_init(&err_handler);
+	ret = sem_check_rrset(n, zone_rrset, &err_handler);
+	if (ret != KNOT_EOK) {
+		return ret;
+	}
 	ret = sem_check_node_plain(zc->z, n,
 	                           &err_handler, true,
 	                           &sem_fatal_error);
diff --git a/src/libknot/rrset-dump.c b/src/libknot/rrset-dump.c
index c094bb517..93d6c27a8 100644
--- a/src/libknot/rrset-dump.c
+++ b/src/libknot/rrset-dump.c
@@ -2006,7 +2006,7 @@ int knot_rrset_txt_dump(const knot_rrset_t      *rrset,
 
 	// Loop over rdata in rrset.
 	uint16_t rr_count = knot_rrset_rr_count(rrset);
-	for (uint16_t i = 0; i < knot_rrset_rr_count(rrset); i++) {
+	for (uint16_t i = 0; i < rr_count; i++) {
 		// Dump rdata owner, class, ttl and type.
 		ret = knot_rrset_txt_dump_header(rrset,
 		                                 knot_rrset_rr_ttl(rrset, i),
-- 
GitLab