lp+mgmt+security: tidy up TLV parsing code in a few classes

Change-Id: I458bd10e16a05b08ed444f983a18453648cd191b

Author: Pesa

ndn-cxx/lp/cache-policy.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -37,11 +37,6 @@ operator<<(std::ostream& os, CachePolicyType policy)
   }
 }
 
-CachePolicy::CachePolicy()
-  : m_policy(CachePolicyType::NONE)
-{
-}
-
 CachePolicy::CachePolicy(const Block& block)
 {
   wireDecode(block);
@@ -82,7 +77,6 @@ CachePolicy::wireEncode() const
   wireEncode(buffer);
 
   m_wire = buffer.block();
-
   return m_wire;
 }
 
@@ -92,23 +86,20 @@ CachePolicy::wireDecode(const Block& wire)
   if (wire.type() != tlv::CachePolicy) {
     NDN_THROW(Error("CachePolicy", wire.type()));
   }
-
   m_wire = wire;
   m_wire.parse();
 
-  auto it = m_wire.elements_begin();
-  if (it == m_wire.elements_end()) {
-    NDN_THROW(Error("Empty CachePolicy"));
-  }
+  m_policy = CachePolicyType::NONE;
 
-  if (it->type() == tlv::CachePolicyType) {
+  auto it = m_wire.elements_begin();
+  if (it != m_wire.elements_end() && it->type() == tlv::CachePolicyType) {
     m_policy = readNonNegativeIntegerAs<CachePolicyType>(*it);
     if (getPolicy() == CachePolicyType::NONE) {
-      NDN_THROW(Error("Unknown CachePolicyType"));
+      NDN_THROW(Error("Unknown CachePolicyType " + std::to_string(to_underlying(m_policy))));
     }
   }
   else {
-    NDN_THROW(Error("CachePolicyType", it->type()));
+    NDN_THROW(Error("CachePolicyType is missing or out of order"));
   }
 }
 

ndn-cxx/lp/cache-policy.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -34,14 +34,14 @@ namespace ndn::lp {
  */
 enum class CachePolicyType {
   NONE = 0,
-  NO_CACHE = 1
+  NO_CACHE = 1,
 };
 
 std::ostream&
 operator<<(std::ostream& os, CachePolicyType policy);
 
 /**
- * \brief Represents a CachePolicy header field.
+ * \brief Represents a %CachePolicy header field.
  */
 class CachePolicy
 {
@@ -52,7 +52,7 @@ class CachePolicy
     using ndn::tlv::Error::Error;
   };
 
-  CachePolicy();
+  CachePolicy() = default;
 
   explicit
   CachePolicy(const Block& block);
@@ -81,7 +81,7 @@ class CachePolicy
 public: // policy type
   /**
    * \brief Get policy type code.
-   * \retval CachePolicyType::NONE if policy type is unset or has an unknown code
+   * \retval CachePolicyType::NONE if policy type is unset or has an unknown code.
    */
   CachePolicyType
   getPolicy() const;
@@ -94,7 +94,7 @@ class CachePolicy
   setPolicy(CachePolicyType policy);
 
 private:
-  CachePolicyType m_policy;
+  CachePolicyType m_policy = CachePolicyType::NONE;
   mutable Block m_wire;
 };
 

ndn-cxx/lp/nack-header.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -22,6 +22,7 @@
  */
 
 #include "ndn-cxx/lp/nack-header.hpp"
+#include "ndn-cxx/encoding/block-helpers.hpp"
 
 namespace ndn::lp {
 
@@ -53,11 +54,6 @@ isLessSevere(lp::NackReason x, lp::NackReason y)
   return to_underlying(x) < to_underlying(y);
 }
 
-NackHeader::NackHeader()
-  : m_reason(NackReason::NONE)
-{
-}
-
 NackHeader::NackHeader(const Block& block)
 {
   wireDecode(block);
@@ -90,7 +86,6 @@ NackHeader::wireEncode() const
   wireEncode(buffer);
 
   m_wire = buffer.block();
-
   return m_wire;
 }
 
@@ -100,13 +95,13 @@ NackHeader::wireDecode(const Block& wire)
   if (wire.type() != tlv::Nack) {
     NDN_THROW(ndn::tlv::Error("Nack", wire.type()));
   }
-
   m_wire = wire;
   m_wire.parse();
+
   m_reason = NackReason::NONE;
 
-  if (m_wire.elements_size() > 0) {
-    auto it = m_wire.elements_begin();
+  auto it = m_wire.elements_begin();
+  if (it != m_wire.elements_end()) {
     if (it->type() == tlv::NackReason) {
       m_reason = readNonNegativeIntegerAs<NackReason>(*it);
     }

ndn-cxx/lp/nack-header.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -24,7 +24,6 @@
 #ifndef NDN_CXX_LP_NACK_HEADER_HPP
 #define NDN_CXX_LP_NACK_HEADER_HPP
 
-#include "ndn-cxx/encoding/block-helpers.hpp"
 #include "ndn-cxx/encoding/encoding-buffer.hpp"
 #include "ndn-cxx/lp/tlv.hpp"
 
@@ -37,7 +36,7 @@ enum class NackReason {
   NONE = 0,
   CONGESTION = 50,
   DUPLICATE = 100,
-  NO_ROUTE = 150
+  NO_ROUTE = 150,
 };
 
 std::ostream&
@@ -57,7 +56,7 @@ isLessSevere(lp::NackReason x, lp::NackReason y);
 class NackHeader
 {
 public:
-  NackHeader();
+  NackHeader() = default;
 
   explicit
   NackHeader(const Block& block);
@@ -75,7 +74,7 @@ class NackHeader
 public: // reason
   /**
    * \brief Get reason code.
-   * \retval NackReason::NONE if NackReason element does not exist or has an unknown code
+   * \retval NackReason::NONE if NackReason element does not exist or has an unknown code.
    */
   NackReason
   getReason() const;
@@ -88,7 +87,7 @@ class NackHeader
   setReason(NackReason reason);
 
 private:
-  NackReason m_reason;
+  NackReason m_reason = NackReason::NONE;
   mutable Block m_wire;
 };
 

ndn-cxx/mgmt/control-response.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -87,26 +87,31 @@ ControlResponse::wireDecode(const Block& wire)
   if (wire.type() != tlv::nfd::ControlResponse) {
     NDN_THROW(Error("ControlResponse", wire.type()));
   }
+
+  *this = {};
   m_wire = wire;
   m_wire.parse();
 
-  auto val = m_wire.elements_begin();
-  if (val == m_wire.elements_end() || val->type() != tlv::nfd::StatusCode) {
-    NDN_THROW(Error("missing StatusCode sub-element"));
+  auto it = m_wire.elements_begin();
+  if (it != m_wire.elements_end() && it->type() == tlv::nfd::StatusCode) {
+    m_code = readNonNegativeIntegerAs<uint32_t>(*it);
+    ++it;
+  }
+  else {
+    NDN_THROW(Error("StatusCode is missing or out of order"));
   }
-  m_code = readNonNegativeIntegerAs<uint32_t>(*val);
-  ++val;
 
-  if (val == m_wire.elements_end() || val->type() != tlv::nfd::StatusText) {
-    NDN_THROW(Error("missing StatusText sub-element"));
+  if (it != m_wire.elements_end() && it->type() == tlv::nfd::StatusText) {
+    m_text = readString(*it);
+    ++it;
+  }
+  else {
+    NDN_THROW(Error("StatusText is missing or out of order"));
   }
-  m_text = readString(*val);
-  ++val;
 
-  if (val != m_wire.elements_end())
-    m_body = *val;
-  else
-    m_body = {};
+  if (it != m_wire.elements_end()) {
+    m_body = *it;
+  }
 }
 
 } // namespace ndn::mgmt

ndn-cxx/mgmt/nfd/strategy-choice.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -87,11 +87,9 @@ StrategyChoice::wireDecode(const Block& block)
   if (val != m_wire.elements_end() && val->type() == tlv::nfd::Strategy) {
     val->parse();
     if (val->elements().empty()) {
-      NDN_THROW(Error("expecting Strategy/Name"));
-    }
-    else {
-      m_strategy.wireDecode(*val->elements_begin());
+      NDN_THROW(Error("expecting Strategy.Name"));
     }
+    m_strategy.wireDecode(val->elements().front());
     ++val;
   }
   else {

ndn-cxx/security/additional-description.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -40,7 +40,7 @@ AdditionalDescription::get(const std::string& key) const
     return it->second;
   }
 
-  NDN_THROW(Error("Entry does not exist for key (" + key + ")"));
+  NDN_THROW(Error("Entry does not exist for key '" + key + "'"));
 }
 
 void
@@ -115,41 +115,42 @@ AdditionalDescription::wireEncode() const
   wireEncode(buffer);
 
   m_wire = buffer.block();
-  m_wire.parse();
-
   return m_wire;
 }
 
 void
 AdditionalDescription::wireDecode(const Block& wire)
 {
-   if (!wire.hasWire()) {
-     NDN_THROW(Error("The supplied block does not contain wire format"));
+  if (wire.type() != tlv::AdditionalDescription) {
+    NDN_THROW(Error("AdditionalDescription", wire.type()));
   }
-
   m_wire = wire;
   m_wire.parse();
 
-  if (m_wire.type() != tlv::AdditionalDescription)
-    NDN_THROW(Error("AdditionalDescription", m_wire.type()));
-
-  auto it = m_wire.elements_begin();
-  while (it != m_wire.elements_end()) {
-    const Block& entry = *it;
-    entry.parse();
-
-    if (entry.type() != tlv::DescriptionEntry)
-      NDN_THROW(Error("DescriptionEntry", entry.type()));
-
-    if (entry.elements_size() != 2)
-      NDN_THROW(Error("DescriptionEntry does not have two sub-TLVs"));
-
-    if (entry.elements()[KEY_OFFSET].type() != tlv::DescriptionKey ||
-        entry.elements()[VALUE_OFFSET].type() != tlv::DescriptionValue)
-      NDN_THROW(Error("Invalid DescriptionKey or DescriptionValue field"));
-
-    m_info[readString(entry.elements()[KEY_OFFSET])] = readString(entry.elements()[VALUE_OFFSET]);
-    it++;
+  m_info.clear();
+
+  for (const auto& e : m_wire.elements()) {
+    switch (e.type()) {
+    case tlv::DescriptionEntry:
+      e.parse();
+      if (e.elements_size() != 2) {
+        NDN_THROW(Error("DescriptionEntry does not have two sub-elements"));
+      }
+      if (e.elements()[KEY_OFFSET].type() != tlv::DescriptionKey ||
+          e.elements()[VALUE_OFFSET].type() != tlv::DescriptionValue) {
+        NDN_THROW(Error("Missing DescriptionKey or DescriptionValue field"));
+      }
+      m_info.insert_or_assign(readString(e.elements()[KEY_OFFSET]),
+                              readString(e.elements()[VALUE_OFFSET]));
+      break;
+    default: // unrecognized element
+      // if the TLV-TYPE is critical, abort decoding
+      if (tlv::isCriticalType(e.type())) {
+        NDN_THROW(Error("Unrecognized element of critical type " + std::to_string(e.type())));
+      }
+      // otherwise, ignore it
+      break;
+    }
   }
 }