# HG changeset patch
# User Sebastien Decugis <sdecugis@nict.go.jp>
# Date 1260258918 -32400
# Node ID 5b3868944e2bb3cf770324b2aed739548e497d28
# Parent  2cf9f62cdcc7bbd5c23b6a488a28568bee46fb54
Reporting errors in parse_dict function

diff -r 2cf9f62cdcc7 -r 5b3868944e2b freeDiameter/messages.c
--- a/freeDiameter/messages.c	Tue Dec 08 15:38:09 2009 +0900
+++ b/freeDiameter/messages.c	Tue Dec 08 16:55:18 2009 +0900
@@ -292,10 +292,11 @@
 	
 	/* Parse the message against our dictionary */
 	ret = fd_msg_parse_rules ( m, fd_g_config->cnf_dict, &pei);
-	if (ret != EBADMSG)
+	if 	((ret != EBADMSG) 	/* Parsing grouped AVP failed / Conflicting rule found */
+		&& (ret != ENOTSUP))	/* Command is not supported / Mandatory AVP is not supported */
 		return ret;
 	
-	fd_log_debug("The following message does not comply to the dictionary and rules (%s):\n", pei.pei_errcode);
+	fd_log_debug("The following message does not comply to the dictionary and/or rules (%s):\n", pei.pei_errcode);
 	fd_msg_dump_walk(NONE, m);
 	
 	/* Now create an answer error if the message is a query */
diff -r 2cf9f62cdcc7 -r 5b3868944e2b freeDiameter/routing.c
--- a/freeDiameter/routing.c	Tue Dec 08 15:38:09 2009 +0900
+++ b/freeDiameter/routing.c	Tue Dec 08 16:55:18 2009 +0900
@@ -384,7 +384,7 @@
 					switch (ahdr->avp_code) {
 						case AC_DESTINATION_HOST:
 							/* Parse this AVP */
-							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ), goto fatal_error );
+							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ), goto fatal_error );
 							ASSERT( ahdr->avp_value );
 							/* Compare the Destination-Host AVP of the message with our identity */
 							if (ahdr->avp_value->os.len != fd_g_config->cnf_diamid_len) {
@@ -397,7 +397,7 @@
 							
 						case AC_DESTINATION_REALM:
 							/* Parse this AVP */
-							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ), goto fatal_error );
+							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ), goto fatal_error );
 							ASSERT( ahdr->avp_value );
 							dr_val = ahdr->avp_value;
 							/* Compare the Destination-Realm AVP of the message with our identity */
@@ -411,7 +411,7 @@
 						
 						case AC_USER_NAME:
 							/* Parse this AVP */
-							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ), goto fatal_error );
+							CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ), goto fatal_error );
 							ASSERT( ahdr->avp_value );
 							un = avp;
 							un_val = ahdr->avp_value;
@@ -655,7 +655,7 @@
 				
 				if ((ahdr->avp_code == AC_ROUTE_RECORD) && (! (ahdr->avp_flags & AVP_FLAG_VENDOR)) ) {
 					/* Parse this AVP */
-					CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ), goto fatal_error );
+					CHECK_FCT_DO( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ), goto fatal_error );
 					ASSERT( ahdr->avp_value );
 					/* Remove this value from the list */
 					fd_rtd_candidate_del(rtd, (char *)ahdr->avp_value->os.data, ahdr->avp_value->os.len);
@@ -795,14 +795,14 @@
 			switch (ahdr->avp_code) {
 				case AC_DESTINATION_HOST:
 					/* Parse this AVP */
-					CHECK_FCT( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ) );
+					CHECK_FCT( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ) );
 					ASSERT( ahdr->avp_value );
 					dh = ahdr->avp_value;
 					break;
 
 				case AC_DESTINATION_REALM:
 					/* Parse this AVP */
-					CHECK_FCT( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict ) );
+					CHECK_FCT( fd_msg_parse_dict ( avp, fd_g_config->cnf_dict, NULL ) );
 					ASSERT( ahdr->avp_value );
 					dr = ahdr->avp_value;
 					break;
diff -r 2cf9f62cdcc7 -r 5b3868944e2b freeDiameter/tests/testmesg.c
--- a/freeDiameter/tests/testmesg.c	Tue Dec 08 15:38:09 2009 +0900
+++ b/freeDiameter/tests/testmesg.c	Tue Dec 08 16:55:18 2009 +0900
@@ -686,7 +686,7 @@
 				/* Change the command-code */
 				buf_cpy[5] = 0x11;
 				CHECK( 0, fd_msg_parse_buffer( &buf_cpy, 344, &msg) );
-				CHECK( ENOTSUP, fd_msg_parse_dict( msg, fd_g_config->cnf_dict ) );
+				CHECK( ENOTSUP, fd_msg_parse_dict( msg, fd_g_config->cnf_dict, NULL ) );
 				
 				/* reset */
 				CHECK( 0, fd_msg_free ( msg ) );
@@ -701,7 +701,7 @@
 				
 				/* Check that we cannot support this message now */
 				CHECK( 0, fd_msg_parse_buffer( &buf_cpy, 344, &msg) );
-				CHECK( ENOTSUP, fd_msg_parse_dict( msg, fd_g_config->cnf_dict ) );
+				CHECK( ENOTSUP, fd_msg_parse_dict( msg, fd_g_config->cnf_dict, NULL ) );
 				
 				/* reset */
 				CHECK( 0, fd_msg_free ( msg ) );
@@ -715,7 +715,7 @@
 				
 				/* Check that we can support this message now */
 				CHECK( 0, fd_msg_parse_buffer( &buf_cpy, 344, &msg) );
-				CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict ) );
+				CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict, NULL ) );
 				
 				#if 0
 				fd_msg_dump_walk(0, msg);
@@ -726,7 +726,7 @@
 			}
 			
 			CHECK( 0, fd_msg_parse_buffer( &buf, 344, &msg) );
-			CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict ) );
+			CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict, NULL ) );
 			#if 0
 			fd_msg_dump_walk(0, msg);
 			#endif
@@ -963,7 +963,7 @@
 		
 		/* Ok, now let's recreate the message */
 		CHECK( 0, fd_msg_parse_buffer( &buf, 64, &cer) );
-		CHECK( 0, fd_msg_parse_dict( cer, fd_g_config->cnf_dict ) );
+		CHECK( 0, fd_msg_parse_dict( cer, fd_g_config->cnf_dict, NULL ) );
 		
 		/* Get the pointers to the first and last AVP */
 		CHECK( 0, fd_msg_browse( cer, MSG_BRW_FIRST_CHILD, &avp4, NULL) );
@@ -1193,7 +1193,7 @@
 			CHECK( 0, fd_msg_free( msg ) );
 			
 			CHECK( 0, fd_msg_parse_buffer( &buf, 148, &msg) );
-			CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict ) );
+			CHECK( 0, fd_msg_parse_dict( msg, fd_g_config->cnf_dict, NULL ) );
 			#if 0
 			fd_msg_dump_walk(0, msg);
 			#endif
diff -r 2cf9f62cdcc7 -r 5b3868944e2b include/freeDiameter/libfreeDiameter.h
--- a/include/freeDiameter/libfreeDiameter.h	Tue Dec 08 15:38:09 2009 +0900
+++ b/include/freeDiameter/libfreeDiameter.h	Tue Dec 08 16:55:18 2009 +0900
@@ -2179,12 +2179,21 @@
  */
 int fd_msg_parse_buffer ( unsigned char ** buffer, size_t buflen, struct msg ** msg );
 
+/* Parsing Error Information structure */
+struct fd_pei {
+	char *		pei_errcode;	/* name of the error code to use */
+	struct avp *	pei_avp;	/* pointer to invalid or missing AVP (to be freed) */
+	char *		pei_message;	/* Overwrite default message if needed */
+	int		pei_protoerr; 	/* do we set the 'E' bit in the error message ? */
+};
+
 /*
  * FUNCTION:	fd_msg_parse_dict
  *
  * PARAMETERS:
  *  object	: A msg or AVP object as returned by fd_msg_parse_buffer.
  *  dict	: the dictionary containing the objects definitions to use for resolving all AVPs.
+ *  error_info	: If not NULL, will contain the detail about error upon return. May be used to generate an error reply.
  *
  * DESCRIPTION: 
  *   This function looks up for the command and each children AVP definitions in the dictionary.
@@ -2202,15 +2211,7 @@
  *  ENOMEM	: Unable to allocate enough memory to complete the operation.
  *  ENOTSUP	: No dictionary definition for the command or one of the mandatory AVP was found.
  */
-int fd_msg_parse_dict ( msg_or_avp * object, struct dictionary * dict );
-
-/* Parsing Error Information structure */
-struct fd_pei {
-	char *		pei_errcode;	/* name of the error code to use */
-	struct avp *	pei_avp;	/* pointer to invalid or missing AVP (to be freed) */
-	char *		pei_message;	/* Overwrite default message if needed */
-	int		pei_protoerr; 	/* do we set the 'E' bit in the error message ? */
-};
+int fd_msg_parse_dict ( msg_or_avp * object, struct dictionary * dict, struct fd_pei *error_info );
 
 /*
  * FUNCTION:	fd_msg_parse_rules
diff -r 2cf9f62cdcc7 -r 5b3868944e2b libfreeDiameter/messages.c
--- a/libfreeDiameter/messages.c	Tue Dec 08 15:38:09 2009 +0900
+++ b/libfreeDiameter/messages.c	Tue Dec 08 16:55:18 2009 +0900
@@ -156,7 +156,7 @@
 #define GETINITIALSIZE( _type, _vend ) (avp_value_sizes[ CHECK_BASETYPE(_type) ? (_type) : 0] + GETAVPHDRSZ(_vend))
 
 /* Forward declaration */
-static int parsedict_do_msg(struct dictionary * dict, struct msg * msg, int only_hdr);
+static int parsedict_do_msg(struct dictionary * dict, struct msg * msg, int only_hdr, struct fd_pei *error_info);
 
 /***************************************************************************************************************/
 /* Creating objects */
@@ -306,7 +306,7 @@
 		CHECK_FCT( fd_dict_get_error_cmd(dict, &model) );
 	} else {
 		/* The model is the answer corresponding to the query. It supposes that these are defined in the dictionary */
-		CHECK_FCT_DO(  parsedict_do_msg( dict, qry, 1), /* continue */  );
+		CHECK_FCT_DO(  parsedict_do_msg( dict, qry, 1, NULL), /* continue */  );
 		if (qry->msg_model) {
 			CHECK_FCT(  fd_dict_search ( dict, DICT_COMMAND, CMD_ANSWER, qry->msg_model, &model, EINVAL )  );
 		}
@@ -550,7 +550,7 @@
 	if (avp && nextavp) {
 		struct dictionary * dict;
 		CHECK_FCT( fd_dict_getdict( what, &dict) );
-		CHECK_FCT_DO( fd_msg_parse_dict( nextavp, dict ), /* nothing */ );
+		CHECK_FCT_DO( fd_msg_parse_dict( nextavp, dict, NULL ), /* nothing */ );
 	}
 	
 	if (avp || nextavp)
@@ -1192,7 +1192,7 @@
 	}
 	
 	if (!avp->avp_model) {
-		CHECK_FCT( fd_msg_parse_dict ( avp, dict ) );
+		CHECK_FCT( fd_msg_parse_dict ( avp, dict, NULL ) );
 	}
 	
 	ASSERT( avp->avp_public.avp_value );
@@ -1695,14 +1695,14 @@
  * For command, if the dictionary model is not found, an error is returned.
  */
 
-static int parsedict_do_chain(struct dictionary * dict, struct fd_list * head, int mandatory);
+static int parsedict_do_chain(struct dictionary * dict, struct fd_list * head, int mandatory, struct fd_pei *error_info);
 
 /* Process an AVP. If we are not in recheck, the avp_source must be set. */
-static int parsedict_do_avp(struct dictionary * dict, struct avp * avp, int mandatory)
+static int parsedict_do_avp(struct dictionary * dict, struct avp * avp, int mandatory, struct fd_pei *error_info)
 {
 	struct dict_avp_data dictdata;
 	
-	TRACE_ENTRY("%p %p %d", dict, avp, mandatory);
+	TRACE_ENTRY("%p %p %d %p", dict, avp, mandatory, error_info);
 	
 	/* First check we received an AVP as input */
 	CHECK_PARAMS(  CHECK_AVP(avp) );
@@ -1714,7 +1714,7 @@
 
 		if ( avp->avp_public.avp_code == dictdata.avp_code  ) {
 			/* Ok then just process the children if any */
-			return parsedict_do_chain(dict, &avp->avp_chain.children, mandatory && (avp->avp_public.avp_flags & AVP_FLAG_MANDATORY));
+			return parsedict_do_chain(dict, &avp->avp_chain.children, mandatory && (avp->avp_public.avp_flags & AVP_FLAG_MANDATORY), error_info);
 		} else {
 			/* We just erase the old model */
 			avp->avp_model = NULL;
@@ -1738,6 +1738,10 @@
 		if (mandatory && (avp->avp_public.avp_flags & AVP_FLAG_MANDATORY)) {
 			TRACE_DEBUG(INFO, "Unsupported mandatory AVP found:");
 			msg_dump_intern(INFO, avp, 2);
+			if (error_info) {
+				error_info->pei_errcode = "DIAMETER_AVP_UNSUPPORTED";
+				error_info->pei_avp = avp;
+			}
 			return ENOTSUP;
 		}
 		
@@ -1773,21 +1777,42 @@
 	/* Check the size is valid */
 	if ((avp_value_sizes[dictdata.avp_basetype] != 0) &&
 	    (avp->avp_public.avp_len - GETAVPHDRSZ( avp->avp_public.avp_flags ) != avp_value_sizes[dictdata.avp_basetype])) {
-		TRACE_DEBUG(INFO, "The AVP size is not suitable for the type. EBADMSG.");
+		TRACE_DEBUG(INFO, "The AVP size is not suitable for the type.");
+		if (error_info) {
+			error_info->pei_errcode = "DIAMETER_INVALID_AVP_LENGTH";
+			error_info->pei_avp = avp;
+		}
 		return EBADMSG;
 	}
 	
 	/* Now get the value inside */
 	switch (dictdata.avp_basetype) {
-		case AVP_TYPE_GROUPED:
+		case AVP_TYPE_GROUPED: {
+			int ret;
+			
 			/* This is a grouped AVP, so let's parse the list of AVPs inside */
-			CHECK_FCT(  parsebuf_list(avp->avp_source, avp->avp_public.avp_len - GETAVPHDRSZ( avp->avp_public.avp_flags ), &avp->avp_chain.children)  );
+			CHECK_FCT_DO(  ret = parsebuf_list(avp->avp_source, avp->avp_public.avp_len - GETAVPHDRSZ( avp->avp_public.avp_flags ), &avp->avp_chain.children),
+				{
+					if ((ret == EBADMSG) && (error_info)) {
+						error_info->pei_errcode = "DIAMETER_INVALID_AVP_VALUE";
+						error_info->pei_avp = avp;
+					}
+					return ret;
+				}  );
 			
-			return parsedict_do_chain(dict, &avp->avp_chain.children, mandatory && (avp->avp_public.avp_flags & AVP_FLAG_MANDATORY));
+			return parsedict_do_chain(dict, &avp->avp_chain.children, mandatory && (avp->avp_public.avp_flags & AVP_FLAG_MANDATORY), error_info);
+		}
 			
 		case AVP_TYPE_OCTETSTRING:
 			/* We just have to copy the string into the storage area */
-			CHECK_PARAMS( avp->avp_public.avp_len > GETAVPHDRSZ( avp->avp_public.avp_flags ) );
+			CHECK_PARAMS_DO( avp->avp_public.avp_len > GETAVPHDRSZ( avp->avp_public.avp_flags ),
+				{
+					if (error_info) {
+						error_info->pei_errcode = "DIAMETER_INVALID_AVP_LENGTH";
+						error_info->pei_avp = avp;
+					}
+					return EBADMSG;
+				} );
 			avp->avp_storage.os.len = avp->avp_public.avp_len - GETAVPHDRSZ( avp->avp_public.avp_flags );
 			CHECK_MALLOC(  avp->avp_storage.os.data = malloc(avp->avp_storage.os.len)  );
 			avp->avp_mustfreeos = 1;
@@ -1820,18 +1845,18 @@
 }
 
 /* Process a list of AVPs */
-static int parsedict_do_chain(struct dictionary * dict, struct fd_list * head, int mandatory)
+static int parsedict_do_chain(struct dictionary * dict, struct fd_list * head, int mandatory, struct fd_pei *error_info)
 {
 	struct fd_list * avpch;
 	
-	TRACE_ENTRY("%p %p %d", dict, head, mandatory);
+	TRACE_ENTRY("%p %p %d %p", dict, head, mandatory, error_info);
 	
 	/* Sanity check */
 	ASSERT ( head == head->head );
 	
 	/* Now process the list */
 	for (avpch=head->next; avpch != head; avpch = avpch->next) {
-		CHECK_FCT(  parsedict_do_avp(dict, _A(avpch->o), mandatory)  );
+		CHECK_FCT(  parsedict_do_avp(dict, _A(avpch->o), mandatory, error_info)  );
 	}
 	
 	/* Done */
@@ -1839,23 +1864,30 @@
 }
 
 /* Process a msg header. */
-static int parsedict_do_msg(struct dictionary * dict, struct msg * msg, int only_hdr)
+static int parsedict_do_msg(struct dictionary * dict, struct msg * msg, int only_hdr, struct fd_pei *error_info)
 {
 	int ret = 0;
 	
-	TRACE_ENTRY("%p %p %d", dict, msg, only_hdr);
+	TRACE_ENTRY("%p %p %d %p", dict, msg, only_hdr, error_info);
 	
 	CHECK_PARAMS(  CHECK_MSG(msg)  );
 	
 	/* Look for the model from the header */
-	CHECK_FCT( fd_dict_search ( dict, DICT_COMMAND, 
+	CHECK_FCT_DO( ret = fd_dict_search ( dict, DICT_COMMAND, 
 			(msg->msg_public.msg_flags & CMD_FLAG_REQUEST) ? CMD_BY_CODE_R : CMD_BY_CODE_A,
 			&msg->msg_public.msg_code,
-			&msg->msg_model, ENOTSUP) );
+			&msg->msg_model, ENOTSUP),
+		{
+			if ((ret == ENOTSUP) && (error_info)) {
+				error_info->pei_errcode = "DIAMETER_COMMAND_UNSUPPORTED";
+				error_info->pei_protoerr = 1;
+			}
+			return ret;
+		} );
 	
 	if (!only_hdr) {
 		/* Then process the children */
-		ret = parsedict_do_chain(dict, &msg->msg_chain.children, 1);
+		ret = parsedict_do_chain(dict, &msg->msg_chain.children, 1, error_info);
 
 		/* Free the raw buffer if any */
 		if ((ret == 0) && (msg->msg_rawbuffer != NULL)) {
@@ -1867,18 +1899,21 @@
 	return ret;
 }
 
-int fd_msg_parse_dict ( msg_or_avp * object, struct dictionary * dict )
+int fd_msg_parse_dict ( msg_or_avp * object, struct dictionary * dict, struct fd_pei *error_info )
 {
-	TRACE_ENTRY("%p %p", dict, object);
+	TRACE_ENTRY("%p %p %p", dict, object, error_info);
 	
 	CHECK_PARAMS(  VALIDATE_OBJ(object)  );
 	
+	if (error_info)
+		memset(error_info, 0, sizeof(struct fd_pei));
+	
 	switch (_C(object)->type) {
 		case MSG_MSG:
-			return parsedict_do_msg(dict, _M(object), 0);
+			return parsedict_do_msg(dict, _M(object), 0, error_info);
 		
 		case MSG_AVP:
-			return parsedict_do_avp(dict, _A(object), 0);
+			return parsedict_do_avp(dict, _A(object), 0, error_info);
 		
 		default:
 			ASSERT(0);
@@ -2156,7 +2191,7 @@
 		memset(error_info, 0, sizeof(struct fd_pei));
 	
 	/* Resolve the dictionary objects when missing. This also validates the object. */
-	CHECK_FCT(  fd_msg_parse_dict ( object, dict )  );
+	CHECK_FCT(  fd_msg_parse_dict ( object, dict, error_info )  );
 	
 	/* Call the recursive function */
 	return parserules_do ( dict, object, error_info, 1 ) ;