[asterisk-commits] mmichelson: branch group/dns_naptr r433885 - in /team/group/dns_naptr: includ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Apr 1 09:35:08 CDT 2015


Author: mmichelson
Date: Wed Apr  1 09:35:03 2015
New Revision: 433885

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=433885
Log:
Addressed review feedback from Matt and Kevin.

* Added Doxygen comments for NAPTR data and DNS record data_ptr
* Added safety checks to return NULL if NAPTR record not found in DNS result.
* Macrotized the check if we are past the end of the NAPTR record when parsing.
* Short-circuit the sort operation if no NAPTR records were returned
* Removed "ast_" prefix from internal DNS routines.
* Error messages for nominal tests are more descriptive.


Modified:
    team/group/dns_naptr/include/asterisk/dns_internal.h
    team/group/dns_naptr/main/dns_core.c
    team/group/dns_naptr/main/dns_naptr.c
    team/group/dns_naptr/res/res_resolver_unbound.c
    team/group/dns_naptr/tests/test_dns_naptr.c

Modified: team/group/dns_naptr/include/asterisk/dns_internal.h
URL: http://svnview.digium.com/svn/asterisk/team/group/dns_naptr/include/asterisk/dns_internal.h?view=diff&rev=433885&r1=433884&r2=433885
==============================================================================
--- team/group/dns_naptr/include/asterisk/dns_internal.h (original)
+++ team/group/dns_naptr/include/asterisk/dns_internal.h Wed Apr  1 09:35:03 2015
@@ -35,6 +35,14 @@
 	size_t data_len;
 	/*! \brief Linked list information */
 	AST_LIST_ENTRY(ast_dns_record) list;
+	/*! \brief pointer to record-specific data.
+	 *
+	 * For certain "subclasses" of DNS records, the
+	 * location of the raw DNS data will differ from
+	 * the generic case. This pointer will reliably
+	 * be set to point to the raw DNS data, no matter
+	 * where in the structure it may lie.
+	 */
 	char *data_ptr;
 	/*! \brief The raw DNS record */
 	char data[0];
@@ -70,6 +78,12 @@
 	unsigned short order;
 	/*! \brief The preference of the NAPTR record */
 	unsigned short preference;
+	/*! \brief Buffer for NAPTR-specific data
+	 *
+	 * This includes the raw NAPTR record, as well as
+	 * the area where the flags, service, regexp, and
+	 * replacement strings are stored.
+	 */
 	char data[0];
 };
 
@@ -158,11 +172,11 @@
  * \retval non-NULL success
  * \retval NULL failure
  */
-struct ast_dns_record *ast_dns_naptr_alloc(struct ast_dns_query *query, const char *data, const size_t size);
+struct ast_dns_record *dns_naptr_alloc(struct ast_dns_query *query, const char *data, const size_t size);
 
 /*!
  * \brief Sort the NAPTR records on a result
  *
  * \param result The DNS result
  */
-void ast_dns_naptr_sort(struct ast_dns_result *result);
+void dns_naptr_sort(struct ast_dns_result *result);

Modified: team/group/dns_naptr/main/dns_core.c
URL: http://svnview.digium.com/svn/asterisk/team/group/dns_naptr/main/dns_core.c?view=diff&rev=433885&r1=433884&r2=433885
==============================================================================
--- team/group/dns_naptr/main/dns_core.c (original)
+++ team/group/dns_naptr/main/dns_core.c Wed Apr  1 09:35:03 2015
@@ -461,7 +461,7 @@
 	}
 
 	if (rr_type == ns_t_naptr) {
-		record = ast_dns_naptr_alloc(query, data, size);
+		record = dns_naptr_alloc(query, data, size);
 	} else {
 		record = generic_record_alloc(query, data, size);
 	}
@@ -484,7 +484,7 @@
 void ast_dns_resolver_completed(struct ast_dns_query *query)
 {
 	if (ast_dns_query_get_rr_type(query) == ns_t_naptr) {
-		ast_dns_naptr_sort(query->result);
+		dns_naptr_sort(query->result);
 	}
 	query->callback(query);
 }

Modified: team/group/dns_naptr/main/dns_naptr.c
URL: http://svnview.digium.com/svn/asterisk/team/group/dns_naptr/main/dns_naptr.c?view=diff&rev=433885&r1=433884&r2=433885
==============================================================================
--- team/group/dns_naptr/main/dns_naptr.c (original)
+++ team/group/dns_naptr/main/dns_naptr.c Wed Apr  1 09:35:03 2015
@@ -376,7 +376,9 @@
 	return 0;
 }
 
-struct ast_dns_record *ast_dns_naptr_alloc(struct ast_dns_query *query, const char *data, const size_t size)
+#define PAST_END_OF_RECORD ptr >= end_of_record
+
+struct ast_dns_record *dns_naptr_alloc(struct ast_dns_query *query, const char *data, const size_t size)
 {
 	struct ast_dns_naptr_record *naptr;
 	char *ptr = NULL;
@@ -420,6 +422,14 @@
 		ast_assert(naptr_offset != NULL);
 		ast_assert(naptr_search_base + remaining_size - naptr_offset >= size);
 
+		/* ... but just to be on the safe side, let's be sure we can break
+		 * out if the assertion doesn't hold
+		 */
+		if (!naptr_offset || naptr_search_base + remaining_size - naptr_offset < size) {
+			ast_log(LOG_ERROR, "Failed to locate NAPTR record within DNS result\n");
+			return NULL;
+		}
+
 		if (!memcmp(naptr_offset, data, size)) {
 			/* BAM! FOUND IT! */
 			ptr = naptr_offset;
@@ -444,7 +454,7 @@
 	order = ((unsigned char)(ptr[1]) << 0) | ((unsigned char)(ptr[0]) << 8);
 	ptr += 2;
 
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 
@@ -452,43 +462,43 @@
 	preference = ((unsigned char) (ptr[1]) << 0) | ((unsigned char)(ptr[0]) << 8);
 	ptr += 2;
 
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 
 	/* FLAGS */
 	flags_size = *ptr;
 	++ptr;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 	flags = ptr;
 	ptr += flags_size;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 
 	/* SERVICES */
 	services_size = *ptr;
 	++ptr;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 	services = ptr;
 	ptr += services_size;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 
 	/* REGEXP */
 	regexp_size = *ptr;
 	++ptr;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 	regexp = ptr;
 	ptr += regexp_size;
-	if (ptr >= end_of_record) {
+	if (PAST_END_OF_RECORD) {
 		return NULL;
 	}
 
@@ -595,7 +605,7 @@
 	}
 }
 
-void ast_dns_naptr_sort(struct ast_dns_result *result)
+void dns_naptr_sort(struct ast_dns_result *result)
 {
 	struct ast_dns_record *current;
 	size_t num_records = 0;
@@ -607,6 +617,11 @@
 	/* Determine the number of records */
 	AST_LIST_TRAVERSE(&result->records, current, list) {
 		++num_records;
+	}
+
+	/* No point in continuing if there are no records */
+	if (num_records == 0) {
+		return;
 	}
 
 	/* Allocate an array with that number of records */

Modified: team/group/dns_naptr/res/res_resolver_unbound.c
URL: http://svnview.digium.com/svn/asterisk/team/group/dns_naptr/res/res_resolver_unbound.c?view=diff&rev=433885&r1=433884&r2=433885
==============================================================================
--- team/group/dns_naptr/res/res_resolver_unbound.c (original)
+++ team/group/dns_naptr/res/res_resolver_unbound.c Wed Apr  1 09:35:03 2015
@@ -1251,27 +1251,33 @@
 	i = 0;
 	for (record = ast_dns_result_get_records(result); record; record = ast_dns_record_get_next(record)) {
 		if (ast_dns_naptr_get_order(record) != records[i].order) {
-			ast_test_status_update(test, "Unexpected order in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected order %hu, got order %hu from NAPTR record\n",
+					records[i].order, ast_dns_naptr_get_order(record));
 			res = AST_TEST_FAIL;
 		}
 		if (ast_dns_naptr_get_preference(record) != records[i].preference) {
-			ast_test_status_update(test, "Unexpected preference in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected preference %hu, got preference %hu from NAPTR record\n",
+					records[i].preference, ast_dns_naptr_get_preference(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_flags(record), records[i].flags)) {
-			ast_test_status_update(test, "Unexpected flags in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected flags %s, got flags %s from NAPTR record\n",
+					records[i].flags, ast_dns_naptr_get_flags(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_service(record), records[i].services)) {
-			ast_test_status_update(test, "Unexpected services in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected services %s, got services %s from NAPTR record\n",
+					records[i].services, ast_dns_naptr_get_service(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_regexp(record), records[i].regexp)) {
-			ast_test_status_update(test, "Unexpected regexp in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected regexp %s, got regexp %s from NAPTR record\n",
+					records[i].regexp, ast_dns_naptr_get_regexp(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_replacement(record), records[i].replacement)) {
-			ast_test_status_update(test, "Unexpected replacement in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected replacement %s, got replacement %s from NAPTR record\n",
+					records[i].replacement, ast_dns_naptr_get_replacement(record));
 			res = AST_TEST_FAIL;
 		}
 		records[i].visited = 1;

Modified: team/group/dns_naptr/tests/test_dns_naptr.c
URL: http://svnview.digium.com/svn/asterisk/team/group/dns_naptr/tests/test_dns_naptr.c?view=diff&rev=433885&r1=433884&r2=433885
==============================================================================
--- team/group/dns_naptr/tests/test_dns_naptr.c (original)
+++ team/group/dns_naptr/tests/test_dns_naptr.c Wed Apr  1 09:35:03 2015
@@ -437,27 +437,33 @@
 	i = 0;
 	for (record = ast_dns_result_get_records(result); record; record = ast_dns_record_get_next(record)) {
 		if (ast_dns_naptr_get_order(record) != records[naptr_record_order[i]].order) {
-			ast_test_status_update(test, "Unexpected order in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected order %hu, got order %hu from NAPTR record\n",
+					records[naptr_record_order[i]].order, ast_dns_naptr_get_order(record));
 			res = AST_TEST_FAIL;
 		}
 		if (ast_dns_naptr_get_preference(record) != records[naptr_record_order[i]].preference) {
-			ast_test_status_update(test, "Unexpected preference in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected preference %hu, got preference %hu from NAPTR record\n",
+					records[naptr_record_order[i]].preference, ast_dns_naptr_get_preference(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_flags(record), records[naptr_record_order[i]].flags.val)) {
-			ast_test_status_update(test, "Unexpected flags in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected flags %s, got flags %s from NAPTR record\n",
+					records[naptr_record_order[i]].flags.val, ast_dns_naptr_get_flags(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_service(record), records[naptr_record_order[i]].services.val)) {
-			ast_test_status_update(test, "Unexpected services in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected services %s, got services %s from NAPTR record\n",
+					records[naptr_record_order[i]].services.val, ast_dns_naptr_get_service(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_regexp(record), records[naptr_record_order[i]].regexp.val)) {
-			ast_test_status_update(test, "Unexpected regexp in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected regexp %s, got regexp %s from NAPTR record\n",
+					records[naptr_record_order[i]].regexp.val, ast_dns_naptr_get_regexp(record));
 			res = AST_TEST_FAIL;
 		}
 		if (strcmp(ast_dns_naptr_get_replacement(record), records[naptr_record_order[i]].replacement)) {
-			ast_test_status_update(test, "Unexpected replacement in returned NAPTR record\n");
+			ast_test_status_update(test, "Expected replacement %s, got replacement %s from NAPTR record\n",
+					records[naptr_record_order[i]].replacement, ast_dns_naptr_get_replacement(record));
 			res = AST_TEST_FAIL;
 		}
 		++i;




More information about the asterisk-commits mailing list