Fixed multiple AVP error. Missing fix RFC 6733 section 7.5 regarding avps within...
authorEduardo Ramos Testillano <eduardo.ramos.testillano@ericsson.com>
Sat, 21 Mar 2015 18:17:25 +0000 (19:17 +0100)
committerEduardo Ramos Testillano <eduardo.ramos.testillano@ericsson.com>
Sat, 21 Mar 2015 18:17:25 +0000 (19:17 +0100)
example/diameter/launcher/main.cpp
example/diameter/launcher/resources/ft-client/tests/experiment2/ProtocolErrors/BadAARtoServer/case_1.tc
example/diameter/launcher/resources/ft-client/tests/experiment2/go.sh
include/anna/diameter/codec/Avp.hpp
include/anna/diameter/codec/Message.hpp
source/diameter/codec/Avp.cpp
source/diameter/codec/Message.cpp

index 280624c..fdb69d3 100644 (file)
@@ -1466,8 +1466,8 @@ throw(anna::RuntimeException) {
      std::string address;
      int port;
      anna::functions::getAddressAndPortFromSocketLiteral(cl.getValue("httpServer"), address, port);
-     //const anna::comm::Device* device = network.find(Device::asAddress(address)); // aqui hay que proporcionar una IP !
-     const anna::comm::Device* device = *((network.resolve(address)->device_begin())); // Artimaña para resolver (IP o hostname)
+     //const anna::comm::Device* device = network.find(Device::asAddress(address)); // here provide IP
+     const anna::comm::Device* device = *((network.resolve(address)->device_begin())); // trick to solve 
      a_httpServerSocket = new anna::comm::ServerSocket(anna::comm::INetAddress(device, port), cl.exists("httpServerShared") /* shared bind */, &anna::http::Transport::getFactory());
    }
 
index 0007882..2fe63df 100644 (file)
@@ -5,5 +5,5 @@ SENDHEX2E aar-bad.hex
 WAIT4MESSAGE
 
 # Check Failed-AVP & Subscription-Id within:
-CHECKPATTERN <avp name="Result-Code" data="5004" alias="DIAMETER_INVALID_AVP_VALUE"/>
+CHECKPATTERN <avp name="Result-Code" data="5014" alias="DIAMETER_INVALID_AVP_LENGTH"/>
 CHECKPATTERN <avp name="Failed-AVP">( *)<avp name="Subscription-Id"
index 7c74679..f6e9327 100755 (executable)
@@ -166,6 +166,7 @@ echo "Test cases manager"
 echo "------------------"
 # Mandatory parameter:
 [ -z "$1" ] && _exit "Usage: $SCR_BN <test cases parent directory (or .tc file)>"
+[ ! -f "$1" -a ! -d "$1" ] && _exit "Invalid file or directory '$1' !!"
 
 # Gather .tc files to be processed:
 [ ! -f $1 ] && echo -e "\nGathering list of test cases from '`readlink -f $1`' ..."
index 1e5c529..bdf2227 100644 (file)
@@ -727,7 +727,7 @@ public:
      Diameter Relay and redirect agents MUST NOT reject messages with unrecognized AVPs.
 
      Default implementation launch alarm and counter indicating the anomaly but don't launch exception (traces at warning level).
-     Realy and Redirect agents could reimplement this method to avoid oam management (another way is avoid alarm/counter registration on
+     Relay and Redirect agents could reimplement this method to avoid oam management (another way is avoid alarm/counter registration on
      these applications). Result-Code DIAMETER_AVP_UNSUPPORTED will be stored for possible answer message.
 
      @param answer Answer built for request decoding/validation
index 5944c63..fb519b5 100644 (file)
@@ -146,7 +146,9 @@ class Message {
   bool flagsOK(int &rc) const throw(); // flags coherence regarding dictionary. Only must be called when Message is identified at the dictionary.
   int addChild(Avp *avp) throw() { return Avp::addChild(a_avps, a_insertionPositionForChilds, avp); }
   const anna::diameter::stack::Command *getStackCommand(CommandId id) const throw(anna::RuntimeException);
-  Avp * addFailedAVP() throw(); // returns Failed-AVP if exists, creates it when missing
+  Avp * addTheFailedAVP() throw(); // returns the Failed-AVP if exists, creates it when missing. The method could be named 'addFailedAVP'
+                                      // but we consider this better because only one instance (as RFC 6733 says in section 7.5) will be
+                                      // added by internal procedures (although, the application could obviously add more).
 
 protected:
 
@@ -406,32 +408,31 @@ public:
 
 
   /**
-     Adds a new AVP within a Failed-AVP over an answer message (for requests, do nothing).
+     Adds the wrong AVP within the Failed-AVP over an answer message (for requests, do nothing).
      If Failed-AVP AVP doesn't exists, is added and then filled (added within) with the value provided (empty AVP id representantion).
-     If Failed-AVP AVP already exists, is filled (added within) with the value provided (empty AVP id representantion).
+     If Failed-AVP AVP already exists, is probably filled by a previous found error, but anyway this is verified and if empty then is
+     filled (added within) with the value provided (empty AVP id representantion).
 
-     This method is internally used during #decode and/or #valid procedures in order to build automatic answers.
+     This method is internally used during #decode and/or #valid procedures in order to build automatic answers, but application
+     could call this for set another Failed-AVP content no detected by these methods, for example: DIAMETER_CONTRADICTING_AVPS or
+     DIAMETER_INVALID_AVP_BIT_COMBO). Also, application could add more Failed-AVP avps with other wrong avps, or accumulate wrong
+     avps inside the one and only Failed-AVP managed by the stack (see section 7.5 of RFC 6733).
 
      @param id Avp identifier as pair (code,vendor-id).
 
-     @return Pointer to the new AVP added within Failed-AVP, to make easy data-part accessif needed.
+     @return Pointer to the new AVP added within Failed-AVP, to make easy data-part access if needed.
   */
-  Avp * setNewFailedAvp(AvpId id) throw(anna::RuntimeException) { if(isRequest()) return NULL; return (addFailedAVP()->addAvp(id)); }
+  Avp * setFailedAvp(AvpId id) throw(anna::RuntimeException);
 
   /**
-     Adds a new AVP within a Failed-AVP over an answer message (for requests, do nothing).
-     If Failed-AVP AVP doesn't exists, is added and then filled (added within) with the value provided (empty AVP id representantion).
-     If Failed-AVP AVP already exists, is filled (added within) with the value provided (empty AVP id representantion).
-
-     This method is internally used during #decode and/or #valid procedures in order to build automatic answers, but application
-     could call this for set another Failed-AVP content no detected by these methods, for example: DIAMETER_CONTRADICTING_AVPS or
-     DIAMETER_INVALID_AVP_BIT_COMBO).
+     Same as #setFailedAvp(AvpId id) but providing an avp pointer with the needed information
 
-     @param id Avp identifier as pair (code,vendor-id).
+     @param avp Pointer to the added wrong avp
 
-     @return Pointer to the new AVP added within Failed-AVP, to make easy data-part accessif needed.
+     @return Pointer to the new AVP added within Failed-AVP, to make easy data-part access if needed.
   */
-  Avp * setNewFailedAvp(Avp *avp) throw() { if(!avp || isRequest()) return NULL; return (addFailedAVP()->addAvp(avp)); }
+  Avp * setFailedAvp(Avp *avp) throw(anna::RuntimeException);
+
 
 
   /**
index 19a0780..7f33851 100644 (file)
@@ -644,7 +644,7 @@ void Avp::unknownAvpWithMandatoryBit(Message *answer) const throw(anna::RuntimeE
 
   if(answer) {
     answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_AVP_UNSUPPORTED);
-    answer->setNewFailedAvp((Avp*)this);
+    answer->setFailedAvp((Avp*)this);
   }
 }
 
@@ -679,6 +679,12 @@ void Avp::decodeDataPart(const char * buffer, int size, Message *answer) throw(a
     if((size % 4) != 0) {
       oamModule.activateAlarm(OamModule::Alarm::AvpDecode__IncorrectLength);
       oamModule.count(OamModule::Counter::AvpDecode__IncorrectLength);
+
+      if(answer) {
+        answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_LENGTH);
+        //answer->setFailedAvp(a_id);
+      }
+
       throw anna::RuntimeException("Avp format error, the avp length is incorrect (must be multiple of 4 on grouped type)", ANNA_FILE_LOCATION);
     }
 
@@ -724,7 +730,7 @@ void Avp::decode(const anna::DataBlock &db, Message *answer) throw(anna::Runtime
   if(db.getSize() < HeaderLengthVinactive) {
     oamModule.activateAlarm(OamModule::Alarm::AvpDecode__NotEnoughBytesToCoverAvpHeaderLength);
     oamModule.count(OamModule::Counter::AvpDecode__NotEnoughBytesToCoverAvpHeaderLength);
-    // DIAMETER_INVALID_AVP_LENGTH; // no podré construir un avp fiable, así que no registro el result-code
+    // DIAMETER_INVALID_AVP_LENGTH; // no podr� construir un avp fiable, as� que no registro el result-code
     throw anna::RuntimeException("Not enough bytes to cover avp header length", ANNA_FILE_LOCATION);
   }
 
@@ -752,7 +758,7 @@ void Avp::decode(const anna::DataBlock &db, Message *answer) throw(anna::Runtime
     if(db.getSize() < HeaderLengthVactive) {
       oamModule.activateAlarm(OamModule::Alarm::AvpDecode__NotEnoughBytesToCoverAvpHeaderLength);
       oamModule.count(OamModule::Counter::AvpDecode__NotEnoughBytesToCoverAvpHeaderLength);
-      // DIAMETER_INVALID_AVP_LENGTH; // no podré construir un avp fiable, así que no registro el result-code
+      // DIAMETER_INVALID_AVP_LENGTH; // no podr� construir un avp fiable, as� que no registro el result-code
       throw anna::RuntimeException(anna::functions::asString("Not enough bytes to cover avp header length (avp code = %u)", code), ANNA_FILE_LOCATION);
     }
 
@@ -794,7 +800,7 @@ void Avp::decode(const anna::DataBlock &db, Message *answer) throw(anna::Runtime
 
     if(answer) {
       answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_LENGTH);
-      answer->setNewFailedAvp(a_id);
+      //answer->setFailedAvp(a_id);
     }
 
     throw anna::RuntimeException(anna::functions::asString("Avp format error, the avp length is incorrect (avp code = %u)", code), ANNA_FILE_LOCATION);
@@ -808,7 +814,7 @@ void Avp::decode(const anna::DataBlock &db, Message *answer) throw(anna::Runtime
 
     if(answer) {
       answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_VALUE); // unspecified error ...
-      answer->setNewFailedAvp((Avp*)this);
+      //answer->setFailedAvp((Avp*)this);
     }
 
     throw anna::RuntimeException(anna::functions::asString("Internal Avp decoding error (avp code = %u): %s", code, ex.getText().c_str()), ANNA_FILE_LOCATION);
@@ -885,7 +891,7 @@ void Avp::fix() throw() {
   }
 
   if(!stackFormat || !stackFormat->isGrouped()) {
-    //LOGDEBUG(anna::Logger::debug("Avp is not grouped. Nothing fix.", ANNA_FILE_LOCATION));
+    //LOGDEBUG(anna::Logger::debug("Avp is not grouped. Nothing to fix.", ANNA_FILE_LOCATION));
     return;
   }
 
@@ -939,7 +945,7 @@ bool Avp::validLevel(const avp_container &avps, anna::diameter::stack::const_avp
 
         if(answer) {
           answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_MISSING_AVP);
-          answer->setNewFailedAvp((*rule_it).second.getId());
+          answer->setFailedAvp((*rule_it).second.getId());
         }
 
         engine->validationAnomaly(anna::functions::asString("Missing fixed rule %s inside %s", STRING_WITH_QUOTATION_MARKS__C_STR((*rule_it).second.asString(false /*ommit dots & pair*/)), STRING_WITH_QUOTATION_MARKS__C_STR(parentDescription)));
@@ -973,14 +979,14 @@ bool Avp::validLevel(const avp_container &avps, anna::diameter::stack::const_avp
 
         if(answer) {
           answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_MISSING_AVP);
-          answer->setNewFailedAvp(id);
+          answer->setFailedAvp(id);
         }
       } else {
         oamModule.count(OamModule::Counter::LevelValidation__FailedRuleForCardinalityMoreThanNeeded);
 
         if(answer) {
           answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_AVP_OCCURS_TOO_MANY_TIMES);
-          answer->setNewFailedAvp((Avp*)firstAvp(avps, id) /* first instance */);
+          answer->setFailedAvp((Avp*)firstAvp(avps, id) /* first instance */);
         }
       }
 
@@ -1030,7 +1036,7 @@ bool Avp::validLevel(const avp_container &avps, anna::diameter::stack::const_avp
 
       if(answer) {
         answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_AVP_NOT_ALLOWED);
-        //answer->setNewFailedAvp((Avp*)firstAvp(avps, id) /* first instance */); // NO SENSE... what to put ?
+        //answer->setFailedAvp((Avp*)firstAvp(avps, id) /* first instance */); // NO SENSE... what to put ?
       }
 
       engine->validationAnomaly(anna::functions::asString("Failed Generic AVP rule %s for cardinality (found %d disregarded items inside %s)", STRING_WITH_QUOTATION_MARKS__C_STR((*generic_rule_it).second.asString(false /*ommit dots & pair*/)), amount, STRING_WITH_QUOTATION_MARKS__C_STR(parentDescription)));
@@ -1051,7 +1057,7 @@ bool Avp::validLevel(const avp_container &avps, anna::diameter::stack::const_avp
       // We wouldn't know where are these disregarded, but...
       if(answer) {
         answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_AVP_NOT_ALLOWED);
-        answer->setNewFailedAvp((Avp*)firstAvp(avps, id) /* first instance */);
+        answer->setFailedAvp((Avp*)firstAvp(avps, id) /* first instance */);
       }
     }
 
@@ -1101,7 +1107,7 @@ bool Avp::valid(const std::string & parentDescription, Message *answer) const th
 
     if(answer) {
       answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_BITS);
-      //answer->setNewFailedAvp((Avp*)this); RFC 6733 says nothing about Failed-AVP in this case...
+      answer->setFailedAvp((Avp*)this); // RFC 6733 says nothing about Failed-AVP in this case...
     }
 
     getEngine()->validationAnomaly(anna::functions::asString("The AVP %s flags (%d) does not fulfill the defined flag rules: %s", STRING_WITH_QUOTATION_MARKS__C_STR(me), (int)a_flags, STRING_WITH_QUOTATION_MARKS__C_STR(stackAvp->getFlagRulesDescription())));
@@ -1119,7 +1125,7 @@ bool Avp::valid(const std::string & parentDescription, Message *answer) const th
 
       if(answer) {
         answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_VALUE);
-        answer->setNewFailedAvp((Avp*)this);
+        answer->setFailedAvp((Avp*)this);
       }
 
       getEngine()->validationAnomaly(anna::functions::asString("Enumerated AVP %s with value %d does not comply to restriction: %s", STRING_WITH_QUOTATION_MARKS__C_STR(me), a_Enumerated->getValue(), stackAvp->getEnums()));
index 9deaf2e..7fc92e8 100644 (file)
@@ -300,7 +300,7 @@ void Message::decode(const anna::DataBlock &db, Message *ptrAnswer) throw(anna::
   if(db.getSize() < HeaderLength) {
     oamModule.activateAlarm(OamModule::Alarm::MessageDecode__NotEnoughBytesToCoverMessageHeaderLength);
     oamModule.count(OamModule::Counter::MessageDecode__NotEnoughBytesToCoverMessageHeaderLength);
-    // DIAMETER_INVALID_MESSAGE_LENGTH; // no podré construir un answer fiable, así que no registro el result-code
+    // DIAMETER_INVALID_MESSAGE_LENGTH; // no podr� construir un answer fiable, as� que no registro el result-code
     throw anna::RuntimeException("Not enough bytes to cover message header length (20 bytes)", ANNA_FILE_LOCATION);
   }
 
@@ -376,10 +376,9 @@ void Message::decode(const anna::DataBlock &db, Message *ptrAnswer) throw(anna::
       avp -> decode(db_aux, answer);
     } catch(anna::RuntimeException &ex) {
       if(answer) {
-        answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_VALUE); // unspecified error ...
-        answer->setNewFailedAvp(avp->getId());
+        //answer->setResultCode(helpers::base::AVPVALUES__Result_Code::DIAMETER_INVALID_AVP_VALUE); // unspecified error ...
+        answer->setFailedAvp(avp->getId());
       }
-
       getEngine()->releaseAvp(avp);
       LOGWARNING(
         anna::Logger::warning(ex.getText(), ANNA_FILE_LOCATION);
@@ -457,17 +456,85 @@ int Message::getResultCode() const throw() {
 
 
 //------------------------------------------------------------------------------
-//------------------------------------------------------ Message::addFailedAVP()
+//--------------------------------------------------- Message::addTheFailedAVP()
 //------------------------------------------------------------------------------
-Avp * Message::addFailedAVP() throw() {
+Avp * Message::addTheFailedAVP() throw() {
   Avp *result = getAvp(helpers::base::AVPID__Failed_AVP, 1, anna::Exception::Mode::Ignore);
 
+  // Section 7.5 RFC 6733: A Diameter message SHOULD contain one Failed-AVP AVP
   if(!result) result = addAvp(helpers::base::AVPID__Failed_AVP);
 
   return result;
 }
 
 
+//------------------------------------------------------------------------------
+//------------------------------------------------------ Message::setFailedAvp()
+//------------------------------------------------------------------------------
+Avp * Message::setFailedAvp(AvpId id) throw(anna::RuntimeException) {
+
+  if(isRequest()) return NULL;
+
+// RFC 6733:
+//
+//  7.5.  Failed-AVP AVP
+//
+//     The Failed-AVP AVP (AVP Code 279) is of type Grouped and provides
+//     debugging information in cases where a request is rejected or not
+//     fully processed due to erroneous information in a specific AVP.  The
+//     value of the Result-Code AVP will provide information on the reason
+//     for the Failed-AVP AVP.  A Diameter answer message SHOULD contain an
+//     instance of the Failed-AVP AVP that corresponds to the error
+//     indicated by the Result-Code AVP.  For practical purposes, this
+//     Failed-AVP would typically refer to the first AVP processing error
+//     that a Diameter node encounters.
+
+  // Although the Failed-AVP definition has cardinality 1* and Failed-AVP itself is defined in
+  // most of the command codes as *[Failed-AVP], i think this is not a deliberate ambiguity.
+  // Probably the RFC wants to give freedom to the application layer, but it is recommended to
+  // have only one child (wrong avp) inside a unique message Failed-AVP to ease the Result-Code
+  // correspondence. Anyway, this behaviour could be easily  opened commenting condition block (*).
+  Avp *theFailedAvp = addTheFailedAVP();
+  LOGDEBUG(
+    std::string msg = "Adding wrong avp ";
+    msg += anna::diameter::functions::avpIdAsPairString(id);
+    msg += " within the message Failed-AVP ...";
+    anna::Logger::debug(msg, ANNA_FILE_LOCATION);
+  );
+
+  if (theFailedAvp->countChilds()) { // (*)
+       LOGDEBUG(anna::Logger::debug("Discarding wrong avp. A previous wrong avp was already added into the Failed-AVP. RFC 6733 Section 7.5 recommends to store only the first.", ANNA_FILE_LOCATION));
+       return NULL;
+  }
+
+  return (theFailedAvp->addAvp(id));
+}
+
+
+//------------------------------------------------------------------------------
+//------------------------------------------------------ Message::setFailedAvp()
+//------------------------------------------------------------------------------
+Avp * Message::setFailedAvp(Avp *avp) throw(anna::RuntimeException) {
+
+  if(!avp || isRequest()) return NULL;
+
+  Avp *theFailedAvp = addTheFailedAVP();
+  LOGDEBUG(
+    std::string msg = "Adding wrong avp ";
+    msg += anna::diameter::functions::avpIdAsPairString(avp->getId());
+    msg += " within the message Failed-AVP ...";
+    anna::Logger::debug(msg, ANNA_FILE_LOCATION);
+  );
+
+  if (theFailedAvp->countChilds()) { // (*)
+       LOGDEBUG(anna::Logger::debug("Discarding wrong avp. A previous wrong avp was already added into the Failed-AVP. RFC 6733 Section 7.5 recommends to store only the first.", ANNA_FILE_LOCATION));
+       return NULL;
+  }
+
+  return (theFailedAvp->addAvp(avp));
+}
+
+
 //------------------------------------------------------------------------------
 //----------------------------------------------- Message::setStandardToAnswer()
 //------------------------------------------------------------------------------
@@ -480,7 +547,8 @@ void Message::setStandardToAnswer(const Message &request, const std::string &ori
   const Avp *reqSessionId = request.getAvp(helpers::base::AVPID__Session_Id, 1, anna::Exception::Mode::Ignore);
 
   if(reqSessionId)
-    addAvp(helpers::base::AVPID__Session_Id)->getUTF8String()->setValue(reqSessionId->getUTF8String()->getValue());
+       if(!getAvp(helpers::base::AVPID__Session_Id, 1, anna::Exception::Mode::Ignore))
+      addAvp(helpers::base::AVPID__Session_Id)->getUTF8String()->setValue(reqSessionId->getUTF8String()->getValue());
 
   // Origin-Host & Realm
   if(!getAvp(helpers::base::AVPID__Origin_Host, 1, anna::Exception::Mode::Ignore))