Output the attribute which failed the filter

src/include/libradius.h

@@ -618,8 +618,9 @@ typedef int8_t (*fr_pair_cmp_t)(VALUE_PAIR const *a, VALUE_PAIR const *b);
 int8_t		attrcmp(VALUE_PAIR const *a, VALUE_PAIR const *b);
 int8_t		attrtagcmp(VALUE_PAIR const *a, VALUE_PAIR const *b);
 void		pairsort(VALUE_PAIR **vps, fr_pair_cmp_t cmp);
-bool		pairvalidate(VALUE_PAIR *filter, VALUE_PAIR *list);
-bool 		pairvalidate_relaxed(VALUE_PAIR *filter, VALUE_PAIR *list);
+void		pairvalidate_debug(TALLOC_CTX *ctx, VALUE_PAIR const *failed[2]);
+bool		pairvalidate(VALUE_PAIR const *failed[2], VALUE_PAIR *filter, VALUE_PAIR *list);
+bool 		pairvalidate_relaxed(VALUE_PAIR const *failed[2], VALUE_PAIR *filter, VALUE_PAIR *list);
 VALUE_PAIR	*paircopyvp(TALLOC_CTX *ctx, VALUE_PAIR const *vp);
 VALUE_PAIR	*paircopyvpdata(TALLOC_CTX *ctx, DICT_ATTR const *da, VALUE_PAIR const *vp);
 VALUE_PAIR	*paircopy(TALLOC_CTX *ctx, VALUE_PAIR *from);

src/lib/valuepair.c

@@ -482,12 +482,65 @@ void pairsort(VALUE_PAIR **vps, fr_pair_cmp_t cmp)
 	*vps = pairsort_merge(a, b, cmp);
 }
 
+/** Write an error to the library errorbuff detailing the mismatch
+ *
+ * Retrieve output with fr_strerror();
+ *
+ * @todo add thread specific talloc contexts.
+ *
+ * @param ctx a hack until we have thread specific talloc contexts.
+ * @param failed pair of attributes which didn't match.
+ */
+void pairvalidate_debug(TALLOC_CTX *ctx, VALUE_PAIR const *failed[2])
+{
+	VALUE_PAIR const *filter = failed[0];
+	VALUE_PAIR const *list = failed[1];
+
+	char *value, *pair;
+
+	(void) fr_strerror();	/* Clear any existing messages */
+
+	if (!filter && !list)
+
+	if (!fr_assert(!(!filter && !list))) return;
+
+	if (!list) {
+		fr_strerror_printf("Attribute \"%s\" not found in list", filter->da->name);
+		return;
+	}
+
+	if (!filter || (filter->da != list->da)) {
+		fr_strerror_printf("Attribute \"%s\" not found in filter", list->da->name);
+		return;
+	}
+
+	if (filter->tag != list->tag) {
+		fr_strerror_printf("Attribute \"%s\" tag \"%i\" didn't match filter tag \"%i\"",
+				   list->da->name, list->tag, list->tag);
+		return;
+	}
+
+	pair = vp_aprint(ctx, filter);
+	value = vp_aprints(ctx, list);
+
+	fr_strerror_printf("Attribute value \"%s\" didn't match filter \"%s\"", value, pair);
+
+	talloc_free(pair);
+	talloc_free(value);
+
+	return;
+}
+
 /** Uses paircmp to verify all VALUE_PAIRs in list match the filter defined by check
  *
+ * @note will sort both filter and list in place.
+ *
+ * @param failed pointer to an array to write the pointers of the filter/list attributes that didn't match.
+ *	  May be NULL.
  * @param filter attributes to check list against.
  * @param list attributes, probably a request or reply
  */
-bool pairvalidate(VALUE_PAIR *filter, VALUE_PAIR *list)
+bool pairvalidate(VALUE_PAIR const *failed[2], VALUE_PAIR *filter, VALUE_PAIR *list)
 {
 	vp_cursor_t filter_cursor;
 	vp_cursor_t list_cursor;
@@ -507,18 +560,16 @@ bool pairvalidate(VALUE_PAIR *filter, VALUE_PAIR *list)
 	pairsort(&filter, attrtagcmp);
 	pairsort(&list, attrtagcmp);
 
-	match = fr_cursor_init(&list_cursor, &list);
 	check = fr_cursor_init(&filter_cursor, &filter);
+	match = fr_cursor_init(&list_cursor, &list);
 
 	while (true) {
 		/*
 		 *	The lists are sorted, so if the first
 		 *	attributes aren't of the same type, then we're
 		 *	done.
 		 */
-		if (!attribute_eq(check, match)) {
-			return false;
-		}
+		if (!attribute_eq(check, match)) goto mismatch;
 
 		/*
 		 *	They're of the same type, but don't have the
@@ -527,33 +578,37 @@ bool pairvalidate(VALUE_PAIR *filter, VALUE_PAIR *list)
 		 *	Note that the RFCs say that for attributes of
 		 *	the same type, order is important.
 		 */
-		if (!paircmp(check, match)) {
-			return false;
-		}
+		if (!paircmp(check, match)) goto mismatch;
 
-		match = fr_cursor_next(&list_cursor);
 		check = fr_cursor_next(&filter_cursor);
-
-		if (!match && !check) break;
+		match = fr_cursor_next(&list_cursor);
+		if (!match && !check) goto mismatch;
 
 		/*
 		 *	One list ended earlier than the others, they
 		 *	didn't match.
 		 */
-		if (!match || !check) {
-			return false;
-		}
+		if (!match || !check) goto mismatch;
 	}
 
 	return true;
+
+mismatch:
+	if (failed) {
+		failed[0] = check;
+		failed[1] = match;
+	}
+	return false;
 }
 
 /** Uses paircmp to verify all VALUE_PAIRs in list match the filter defined by check
+ *
+ * @note will sort both filter and list in place.
  *
  * @param filter attributes to check list against.
  * @param list attributes, probably a request or reply
  */
-bool pairvalidate_relaxed(VALUE_PAIR *filter, VALUE_PAIR *list)
+bool pairvalidate_relaxed(VALUE_PAIR const *failed[2], VALUE_PAIR *filter, VALUE_PAIR *list)
 {
 	vp_cursor_t filter_cursor;
 	vp_cursor_t list_cursor;
@@ -587,10 +642,8 @@ bool pairvalidate_relaxed(VALUE_PAIR *filter, VALUE_PAIR *list)
 			 */
 			last_match = fr_cursor_next_by_da(&list_cursor, check->da, check->tag);
 			if (!last_match) {
-				if (check->op == T_OP_CMP_FALSE) {
-					continue;
-				}
-				return false;
+				if (check->op == T_OP_CMP_FALSE) continue;
+				goto mismatch;
 			}
 
 			fr_cursor_init(&list_cursor, &last_match);
@@ -606,13 +659,18 @@ bool pairvalidate_relaxed(VALUE_PAIR *filter, VALUE_PAIR *list)
 			/*
 			 *	This attribute passed the filter
 			 */
-			if (!paircmp(check, match)) {
-				return false;
-			}
+			if (!paircmp(check, match)) goto mismatch;
 		}
 	}
 
 	return true;
+
+mismatch:
+	if (failed) {
+		failed[0] = check;
+		failed[1] = match;
+	}
+	return false;
 }
 
 /** Copy a single valuepair

src/main/radclient.c

@@ -299,6 +299,7 @@ static int radclient_init(TALLOC_CTX *ctx, rc_file_pair_t *files)
 		 *	Read the request VP's.
 		 */
 		if (readvp2(&request->packet->vps, request->packet, packets, &packets_done) < 0) {
+			fr_perror("radclient: Error parsing \"%s\"", files->filters);
 			goto error;
 		}
 
@@ -319,6 +320,7 @@ static int radclient_init(TALLOC_CTX *ctx, rc_file_pair_t *files)
 			bool filters_done;
 
 			if (readvp2(&request->filter, request, filters, &filters_done) < 0) {
+				fr_perror("radclient: Error parsing \"%s\"", files->filters);
 				goto error;
 			}
 
@@ -1054,12 +1056,15 @@ static int recv_one_packet(int wait_time)
 	} else if (!request->filter) {
 		stats.passed++;
 	} else {
+		VALUE_PAIR const *failed[2];
+
 		pairsort(&request->reply->vps, attrtagcmp);
-		if (pairvalidate(request->filter, request->reply->vps)) {
+		if (pairvalidate(failed, request->filter, request->reply->vps)) {
 			printf("Packet passed filter\n");
 			stats.passed++;
 		} else {
-			printf("Packet failed filter\n");
+			pairvalidate_debug(request, failed);
+			fr_perror("Packet failed filter");
 			stats.failed++;
 		}
 	}
@@ -1068,7 +1073,7 @@ static int recv_one_packet(int wait_time)
 		request->done = true;
 	}
 
- packet_done:
+packet_done:
 	rad_free(&request->reply);
 	rad_free(&reply);	/* may be NULL */
 

src/main/radsniff.c

@@ -1112,7 +1112,7 @@ static void rs_packet_process(uint64_t count, rs_event_t *event, struct pcap_pkt
 			 */
 			if (conf->filter_response_vps) {
 				pairsort(&current->vps, attrtagcmp);
-				if (!pairvalidate_relaxed(conf->filter_response_vps, current->vps)) {
+				if (!pairvalidate_relaxed(NULL, conf->filter_response_vps, current->vps)) {
 					goto drop_response;
 				}
 			}
@@ -1281,7 +1281,7 @@ static void rs_packet_process(uint64_t count, rs_event_t *event, struct pcap_pkt
 		 *	Now verify the packet passes the attribute filter
 		 */
 		if (conf->filter_request_vps) {
-			if (!pairvalidate_relaxed(conf->filter_request_vps, current->vps)) {
+			if (!pairvalidate_relaxed(NULL, conf->filter_request_vps, current->vps)) {
 				goto drop_request;
 			}
 		}

src/main/unittest.c

@@ -639,10 +639,15 @@ int main(int argc, char *argv[])
 			       PW_RESPONSE_PACKET_TYPE, 0);
 	vp->vp_integer = request->reply->code;
 
-	if (filter_vps && !pairvalidate(filter_vps, request->reply->vps)) {
-		fprintf(stderr, "Output file %s does not match attributes in filter %s\n",
-			output_file ? output_file : input_file, filter_file);
-		exit(EXIT_FAILURE);
+	{
+		VALUE_PAIR const *failed[2];
+
+		if (filter_vps && !pairvalidate(failed, filter_vps, request->reply->vps)) {
+			pairvalidate_debug(request, failed);
+			fr_perror("Output file %s does not match attributes in filter %s",
+				  output_file ? output_file : input_file, filter_file);
+			exit(EXIT_FAILURE);
+		}
 	}
 
 	talloc_free(request);