From 9f9c4102474ac1e5c847b1f9fefb461133fd9336 Mon Sep 17 00:00:00 2001
From: Jan Kadlec <jan.kadlec@nic.cz>
Date: Sat, 20 Jul 2013 18:22:01 +0200
Subject: [PATCH] Out-of-zone data handling for [A,I]XFR, DDNS.

- Added an error handling function
- Changed some static functions a bit - they now return error codes
  instead of NULL

Refs #102
---
 src/libknot/updates/ddns.c   |  42 ++++++++------
 src/libknot/updates/xfr-in.c | 106 +++++++++++++++++++++++++----------
 src/libknot/updates/xfr-in.h |   2 +
 3 files changed, 104 insertions(+), 46 deletions(-)

diff --git a/src/libknot/updates/ddns.c b/src/libknot/updates/ddns.c
index 7f4f14fc5..fb1415754 100644
--- a/src/libknot/updates/ddns.c
+++ b/src/libknot/updates/ddns.c
@@ -916,16 +916,17 @@ static int knot_ddns_rr_is_nsec3(const knot_rrset_t *rr)
 
 /*----------------------------------------------------------------------------*/
 /*! \note Copied from xfrin_add_new_node(). */
-static knot_node_t *knot_ddns_add_new_node(knot_zone_contents_t *zone,
-                                           knot_dname_t *owner, int is_nsec3)
+static  int knot_ddns_add_new_node(knot_zone_contents_t *zone,
+                                   knot_dname_t *owner,
+                                   knot_node_t **node, int is_nsec3)
 {
 	assert(zone != NULL);
 	assert(owner != NULL);
 
-	knot_node_t *node = knot_node_new(owner, NULL, 0);
-	if (node == NULL) {
+	*node = knot_node_new(owner, NULL, 0);
+	if (*node == NULL) {
 		dbg_xfrin("Failed to create a new node.\n");
-		return NULL;
+		return KNOT_ERROR;
 	}
 
 	int ret = 0;
@@ -933,14 +934,14 @@ static knot_node_t *knot_ddns_add_new_node(knot_zone_contents_t *zone,
 	// insert the node into zone structures and create parents if
 	// necessary
 	if (is_nsec3) {
-		ret = knot_zone_contents_add_nsec3_node(zone, node, 1, 0);
+		ret = knot_zone_contents_add_nsec3_node(zone, *node, 1, 0);
 	} else {
-		ret = knot_zone_contents_add_node(zone, node, 1, 0);
+		ret = knot_zone_contents_add_node(zone, *node, 1, 0);
 	}
 	if (ret != KNOT_EOK) {
 		dbg_xfrin("Failed to add new node to zone contents.\n");
-		knot_node_free(&node);
-		return NULL;
+		knot_node_free(node);
+		return ret;
 	}
 
 	/*!
@@ -949,9 +950,9 @@ static knot_node_t *knot_ddns_add_new_node(knot_zone_contents_t *zone,
 	 */
 	assert(zone->zone != NULL);
 	//knot_node_set_zone(node, zone->zone);
-	assert(node->zone == zone->zone);
+	assert((*node)->zone == zone->zone);
 
-	return node;
+	return ret;
 }
 
 /*----------------------------------------------------------------------------*/
@@ -1548,14 +1549,21 @@ static int knot_ddns_process_add(const knot_rrset_t *rr,
 	dbg_ddns_verb("Adding RR.\n");
 
 	if (node == NULL) {
-		// create new node, connect it properly to the
-		// zone nodes
+		// create new node, connect it to the zone nodes
 		dbg_ddns_detail("Node not found. Creating new.\n");
-		node = knot_ddns_add_new_node(zone, knot_rrset_get_owner(rr),
-		                              knot_ddns_rr_is_nsec3(rr));
-		if (node == NULL) {
+		int ret = knot_ddns_add_new_node(zone, knot_rrset_get_owner(rr),
+		                                 &node, knot_ddns_rr_is_nsec3(rr));
+		if (ret != KNOT_EOK) {
 			dbg_xfrin("Failed to create new node in zone.\n");
-			return KNOT_ERROR;
+			ret = xfrin_handle_error(zone->apex ? zone->apex->owner : NULL,
+		                             rr->owner, ret);
+			if (ret == KNOT_EOK) {
+		    	// Recoverable error, continue
+		    	return KNOT_EOK;
+			} else {
+            	// Fatal error, rollback update
+				return ret;
+			}
 		}
 	}
 
diff --git a/src/libknot/updates/xfr-in.c b/src/libknot/updates/xfr-in.c
index 61739ad82..28e272ae5 100644
--- a/src/libknot/updates/xfr-in.c
+++ b/src/libknot/updates/xfr-in.c
@@ -321,6 +321,31 @@ static int xfrin_insert_rrset_dnames_to_table(knot_rrset_t *rrset,
 	return KNOT_EOK;
 }
 
+int xfrin_handle_error(const knot_dname_t *zone_owner,
+                       const knot_dname_t *rr_owner,
+                       int ret)
+{
+	char *zonename = knot_dname_to_str(zone_owner);
+        if (ret == KNOT_EBADZONE) {
+		// Out-of-zone data, ignore
+		char *rrname = knot_dname_to_str(rr_owner);
+		log_zone_warning("Zone %s: Ignoring "
+                                 "out-of-zone RR owned by %s\n",
+                                 zonename, rrname);
+		free(zonename);
+	        free(rrname);
+		return KNOT_EOK;
+        } else {
+	        log_zone_error("Zone %s: Failed to process "
+	                       "incoming RR, transfer "
+	                       "is probably malformed. (Reason: %s)\n",
+	                        knot_strerror(ret));
+	        free(zonename);
+	        return KNOT_ERROR;
+	}
+}
+
+
 void xfrin_free_orphan_rrsigs(xfrin_orphan_rrsig_t **rrsigs)
 {
 	xfrin_orphan_rrsig_t *r = *rrsigs;
@@ -799,14 +824,20 @@ dbg_xfrin_exec_verb(
 			ret = add_node(zone, node, 1, 0);
 			assert(node != NULL);
 			if (ret != KNOT_EOK) {
-				dbg_xfrin("Failed to add node to zone (%s).\n",
-					  knot_strerror(ret));
-				knot_packet_free(&packet);
-				knot_node_free(&node); // ???
-				knot_rrset_deep_free(&rr, 1, 1);
-				return KNOT_ERROR;
+				ret = xfrin_handle_error(xfr->zone->name,
+				                         rr->owner, ret);
+				if (ret == KNOT_EOK) {
+					// Recoverable error, do cleanup
+					knot_node_free_rrsets(node, 1);
+					knot_node_free(&node);
+				} else {
+					// Fatal error, free packet
+					knot_packet_free(&packet);
+					knot_node_free(&node);
+					knot_rrset_deep_free(&rr, 1, 1);
+					return KNOT_ERROR;
+				}
 			}
-
 			in_zone = 1;
 		} else {
 			assert(in_zone);
@@ -822,7 +853,6 @@ dbg_xfrin_exec_verb(
 				// merged, free the RRSet
 				knot_rrset_deep_free(&rr, 1, 0);
 			}
-
 		}
 
 		rr = NULL;
@@ -856,9 +886,19 @@ dbg_xfrin_exec_verb(
 		if (ret != KNOT_EOK) {
 			dbg_xfrin("Failed to add last node into zone (%s).\n",
 				  knot_strerror(ret));
-			knot_packet_free(&packet);
-			knot_node_free(&node);
-			return KNOT_ERROR;	/*! \todo Other error */
+			ret = xfrin_handle_error(xfr->zone->name, rr->owner,
+			                         ret);
+                        if (ret == KNOT_EOK) {
+				// Recoverable error, do cleanup
+				knot_node_free_rrsets(node, 1);
+				knot_node_free(&node);
+                        } else {
+				// Fatal error, free packet
+				knot_packet_free(&packet);
+				knot_node_free(&node);
+				knot_rrset_deep_free(&rr, 1, 1);
+				return KNOT_ERROR;
+                        }
 		}
 	}
 
@@ -1833,28 +1873,28 @@ dbg_xfrin_exec_verb(
 
 /*----------------------------------------------------------------------------*/
 
-static knot_node_t *xfrin_add_new_node(knot_zone_contents_t *contents,
-                                       knot_rrset_t *rrset, int is_nsec3)
+static int xfrin_add_new_node(knot_zone_contents_t *contents,
+                              knot_rrset_t *rrset, knot_node_t **node,
+                              int is_nsec3)
 {
-	knot_node_t *node = knot_node_new(knot_rrset_get_owner(rrset),
-					  NULL, 0);
-	if (node == NULL) {
+	*node = knot_node_new(knot_rrset_get_owner(rrset), NULL, 0);
+	if (*node == NULL) {
 		dbg_xfrin("Failed to create a new node.\n");
-		return NULL;
+		return KNOT_ERROR;
 	}
 
-	int ret = 0;
-
 	// insert the node into zone structures and create parents if
 	// necessary
+	int ret = 0;
 	if (is_nsec3) {
-		ret = knot_zone_contents_add_nsec3_node(contents, node, 1, 0);
+		ret = knot_zone_contents_add_nsec3_node(contents, *node, 1, 0);
 	} else {
-		ret = knot_zone_contents_add_node(contents, node, 1, 0);
+		ret = knot_zone_contents_add_node(contents, *node, 1, 0);
 	}
 	if (ret != KNOT_EOK) {
 		dbg_xfrin("Failed to add new node to zone contents.\n");
-		return NULL;
+		knot_node_free(node);
+		return ret;
 	}
 
 	/*!
@@ -1863,9 +1903,9 @@ static knot_node_t *xfrin_add_new_node(knot_zone_contents_t *contents,
 	 */
 
 	assert(contents->zone != NULL);
-	knot_node_set_zone(node, contents->zone);
+	knot_node_set_zone(*node, contents->zone);
 
-	return node;
+	return KNOT_EOK;
 }
 
 /*----------------------------------------------------------------------------*/
@@ -2699,13 +2739,21 @@ dbg_xfrin_exec_detail(
 				// zone nodes
 				dbg_xfrin_detail("Node not found. Creating new."
 						 "\n");
-				node = xfrin_add_new_node(contents,
-							  chset->add[i],
-							  is_nsec3);
-				if (node == NULL) {
+				ret = xfrin_add_new_node(contents,
+							 chset->add[i], &node,
+							 is_nsec3);
+				if (ret != KNOT_EOK) {
 					dbg_xfrin("Failed to create new node "
 						  "in zone.\n");
-					return KNOT_ERROR;
+					ret =
+					xfrin_handle_error(contents->apex ? 
+					                   contents->apex->owner :
+					                   NULL,
+					                   chset->add[i]->owner,
+					                   ret);
+					if (ret != KNOT_EOK) {
+						return ret;
+					}
 				}
 			}
 		}
diff --git a/src/libknot/updates/xfr-in.h b/src/libknot/updates/xfr-in.h
index 54a22c554..802ff0941 100644
--- a/src/libknot/updates/xfr-in.h
+++ b/src/libknot/updates/xfr-in.h
@@ -214,6 +214,8 @@ int xfrin_replace_rrset_in_node(knot_node_t *node,
                                 knot_changes_t *changes,
                                 knot_zone_contents_t *contents);
 
+int xfrin_handle_error(const knot_dname_t *zone_owner, const knot_dname_t *rr_owner, int ret);
+
 #endif /* _KNOTXFR_IN_H_ */
 
 /*! @} */
-- 
GitLab