lp+mgmt+security: tidy up TLV parsing code in a few classes
Change-Id: I458bd10e16a05b08ed444f983a18453648cd191b
diff --git a/ndn-cxx/lp/cache-policy.cpp b/ndn-cxx/lp/cache-policy.cpp
index a460b13..8984315 100644
--- a/ndn-cxx/lp/cache-policy.cpp
+++ b/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 @@
}
}
-CachePolicy::CachePolicy()
- : m_policy(CachePolicyType::NONE)
-{
-}
-
CachePolicy::CachePolicy(const Block& block)
{
wireDecode(block);
@@ -82,7 +77,6 @@
wireEncode(buffer);
m_wire = buffer.block();
-
return m_wire;
}
@@ -92,23 +86,20 @@
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"));
}
}
diff --git a/ndn-cxx/lp/cache-policy.hpp b/ndn-cxx/lp/cache-policy.hpp
index 738fd8e..db5edcb 100644
--- a/ndn-cxx/lp/cache-policy.hpp
+++ b/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 @@
*/
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 @@
using ndn::tlv::Error::Error;
};
- CachePolicy();
+ CachePolicy() = default;
explicit
CachePolicy(const Block& block);
@@ -81,7 +81,7 @@
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 @@
setPolicy(CachePolicyType policy);
private:
- CachePolicyType m_policy;
+ CachePolicyType m_policy = CachePolicyType::NONE;
mutable Block m_wire;
};
diff --git a/ndn-cxx/lp/nack-header.cpp b/ndn-cxx/lp/nack-header.cpp
index 71c76ee..a13692c 100644
--- a/ndn-cxx/lp/nack-header.cpp
+++ b/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 @@
return to_underlying(x) < to_underlying(y);
}
-NackHeader::NackHeader()
- : m_reason(NackReason::NONE)
-{
-}
-
NackHeader::NackHeader(const Block& block)
{
wireDecode(block);
@@ -90,7 +86,6 @@
wireEncode(buffer);
m_wire = buffer.block();
-
return m_wire;
}
@@ -100,13 +95,13 @@
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);
}
diff --git a/ndn-cxx/lp/nack-header.hpp b/ndn-cxx/lp/nack-header.hpp
index bc78e36..85178a6 100644
--- a/ndn-cxx/lp/nack-header.hpp
+++ b/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 @@
NONE = 0,
CONGESTION = 50,
DUPLICATE = 100,
- NO_ROUTE = 150
+ NO_ROUTE = 150,
};
std::ostream&
@@ -57,7 +56,7 @@
class NackHeader
{
public:
- NackHeader();
+ NackHeader() = default;
explicit
NackHeader(const Block& block);
@@ -75,7 +74,7 @@
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 @@
setReason(NackReason reason);
private:
- NackReason m_reason;
+ NackReason m_reason = NackReason::NONE;
mutable Block m_wire;
};
diff --git a/ndn-cxx/mgmt/control-response.cpp b/ndn-cxx/mgmt/control-response.cpp
index ffff61a..eb0a0d3 100644
--- a/ndn-cxx/mgmt/control-response.cpp
+++ b/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 @@
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;
}
- 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"));
+ else {
+ NDN_THROW(Error("StatusCode 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() && it->type() == tlv::nfd::StatusText) {
+ m_text = readString(*it);
+ ++it;
+ }
+ else {
+ NDN_THROW(Error("StatusText is missing or out of order"));
+ }
+
+ if (it != m_wire.elements_end()) {
+ m_body = *it;
+ }
}
} // namespace ndn::mgmt
diff --git a/ndn-cxx/mgmt/nfd/strategy-choice.cpp b/ndn-cxx/mgmt/nfd/strategy-choice.cpp
index cf13c3f..208735a 100644
--- a/ndn-cxx/mgmt/nfd/strategy-choice.cpp
+++ b/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 @@
if (val != m_wire.elements_end() && val->type() == tlv::nfd::Strategy) {
val->parse();
if (val->elements().empty()) {
- NDN_THROW(Error("expecting Strategy/Name"));
+ NDN_THROW(Error("expecting Strategy.Name"));
}
- else {
- m_strategy.wireDecode(*val->elements_begin());
- }
+ m_strategy.wireDecode(val->elements().front());
++val;
}
else {
diff --git a/ndn-cxx/security/additional-description.cpp b/ndn-cxx/security/additional-description.cpp
index 52c4650..ebd12c4 100644
--- a/ndn-cxx/security/additional-description.cpp
+++ b/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 @@
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 @@
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()));
+ m_info.clear();
- 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++;
+ 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;
+ }
}
}
diff --git a/ndn-cxx/security/validity-period.cpp b/ndn-cxx/security/validity-period.cpp
index 1d49147..8cdbc75 100644
--- a/ndn-cxx/security/validity-period.cpp
+++ b/ndn-cxx/security/validity-period.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).
*
@@ -84,32 +84,28 @@
wireEncode(buffer);
m_wire = buffer.block();
- m_wire.parse();
-
return m_wire;
}
void
ValidityPeriod::wireDecode(const Block& wire)
{
- if (!wire.hasWire()) {
- NDN_THROW(Error("The supplied block does not contain wire format"));
+ if (wire.type() != tlv::ValidityPeriod) {
+ NDN_THROW(Error("ValidityPeriod", wire.type()));
}
-
m_wire = wire;
m_wire.parse();
- if (m_wire.type() != tlv::ValidityPeriod)
- NDN_THROW(Error("ValidityPeriod", m_wire.type()));
-
- if (m_wire.elements_size() != 2)
- NDN_THROW(Error("ValidityPeriod does not have two sub-TLVs"));
-
+ if (m_wire.elements_size() != 2) {
+ NDN_THROW(Error("ValidityPeriod does not have two sub-elements"));
+ }
if (m_wire.elements()[NOT_BEFORE_OFFSET].type() != tlv::NotBefore ||
- m_wire.elements()[NOT_BEFORE_OFFSET].value_size() != ISO_DATETIME_SIZE ||
- m_wire.elements()[NOT_AFTER_OFFSET].type() != tlv::NotAfter ||
+ m_wire.elements()[NOT_BEFORE_OFFSET].value_size() != ISO_DATETIME_SIZE) {
+ NDN_THROW(Error("Missing or invalid NotBefore field"));
+ }
+ if (m_wire.elements()[NOT_AFTER_OFFSET].type() != tlv::NotAfter ||
m_wire.elements()[NOT_AFTER_OFFSET].value_size() != ISO_DATETIME_SIZE) {
- NDN_THROW(Error("Invalid NotBefore or NotAfter field"));
+ NDN_THROW(Error("Missing or invalid NotAfter field"));
}
try {
@@ -117,7 +113,7 @@
m_notAfter = decodeTimePoint(m_wire.elements()[NOT_AFTER_OFFSET]);
}
catch (const std::bad_cast&) {
- NDN_THROW(Error("Invalid date format in NOT-BEFORE or NOT-AFTER field"));
+ NDN_THROW(Error("Invalid date format in NotBefore or NotAfter field"));
}
}
diff --git a/tests/unit/mgmt/control-response.t.cpp b/tests/unit/mgmt/control-response.t.cpp
index 6bda463..3d0f4b6 100644
--- a/tests/unit/mgmt/control-response.t.cpp
+++ b/tests/unit/mgmt/control-response.t.cpp
@@ -74,19 +74,24 @@
return e.what() == "Expecting ControlResponse element, but TLV has type 100"sv;
});
- // empty TLV
+ // missing StatusCode
BOOST_CHECK_EXCEPTION(cr.wireDecode("6500"_block), tlv::Error, [] (const auto& e) {
- return e.what() == "missing StatusCode sub-element"sv;
+ return e.what() == "StatusCode is missing or out of order"sv;
});
- // missing StatusCode
- BOOST_CHECK_EXCEPTION(cr.wireDecode("65026700"_block), tlv::Error, [] (const auto& e) {
- return e.what() == "missing StatusCode sub-element"sv;
+ // out-of-order StatusCode
+ BOOST_CHECK_EXCEPTION(cr.wireDecode("650567006601C8"_block), tlv::Error, [] (const auto& e) {
+ return e.what() == "StatusCode is missing or out of order"sv;
});
// missing StatusText
BOOST_CHECK_EXCEPTION(cr.wireDecode("650466020194"_block), tlv::Error, [] (const auto& e) {
- return e.what() == "missing StatusText sub-element"sv;
+ return e.what() == "StatusText is missing or out of order"sv;
+ });
+
+ // out-of-order StatusText
+ BOOST_CHECK_EXCEPTION(cr.wireDecode("65086602019469006700"_block), tlv::Error, [] (const auto& e) {
+ return e.what() == "StatusText is missing or out of order"sv;
});
}