interest: prevent InterestLifetime overflow
Refs: #4997
Change-Id: Ic116e4d6d681ab89c655774a90d9d652db4a8916
diff --git a/ndn-cxx/data.hpp b/ndn-cxx/data.hpp
index b4f8179..043d702 100644
--- a/ndn-cxx/data.hpp
+++ b/ndn-cxx/data.hpp
@@ -44,19 +44,23 @@
using tlv::Error::Error;
};
- /** @brief Construct an unsigned Data packet with given @p name and empty Content.
- * @warning In certain contexts that use `Data::shared_from_this()`, Data must be created using
- * `std::make_shared`. Otherwise, `shared_from_this()` may trigger undefined behavior.
- * One example where this is necessary is storing Data into a subclass of InMemoryStorage.
+ /**
+ * @brief Construct an unsigned Data packet with given @p name and empty Content.
+ *
+ * @warning In certain contexts that use `Data::shared_from_this()`, the Data must be created
+ * using `std::make_shared`. Otherwise, `shared_from_this()` will throw `std::bad_weak_ptr`.
+ * One example where this is necessary is storing Data into a subclass of InMemoryStorage.
*/
explicit
Data(const Name& name = Name());
- /** @brief Construct a Data packet by decoding from @p wire.
- * @param wire TLV element of type tlv::Data; may be signed or unsigned.
- * @warning In certain contexts that use `Data::shared_from_this()`, Data must be created using
- * `std::make_shared`. Otherwise, `shared_from_this()` may trigger undefined behavior.
- * One example where this is necessary is storing Data into a subclass of InMemoryStorage.
+ /**
+ * @brief Construct a Data packet by decoding from @p wire.
+ * @param wire TLV element of type tlv::Data; may be signed or unsigned.
+ *
+ * @warning In certain contexts that use `Data::shared_from_this()`, the Data must be created
+ * using `std::make_shared`. Otherwise, `shared_from_this()` will throw `std::bad_weak_ptr`.
+ * One example where this is necessary is storing Data into a subclass of InMemoryStorage.
*/
explicit
Data(const Block& wire);
@@ -301,37 +305,54 @@
extractSignedRanges() const;
public: // MetaInfo fields
+ /**
+ * @copydoc MetaInfo::getType()
+ */
uint32_t
- getContentType() const
+ getContentType() const noexcept
{
return m_metaInfo.getType();
}
+ /**
+ * @copydoc MetaInfo::setType()
+ */
Data&
setContentType(uint32_t type);
+ /**
+ * @copydoc MetaInfo::getFreshnessPeriod()
+ */
time::milliseconds
- getFreshnessPeriod() const
+ getFreshnessPeriod() const noexcept
{
return m_metaInfo.getFreshnessPeriod();
}
+ /**
+ * @copydoc MetaInfo::setFreshnessPeriod()
+ */
Data&
setFreshnessPeriod(time::milliseconds freshnessPeriod);
+ /**
+ * @copydoc MetaInfo::getFinalBlock()
+ */
const std::optional<name::Component>&
- getFinalBlock() const
+ getFinalBlock() const noexcept
{
return m_metaInfo.getFinalBlock();
}
+ /**
+ * @copydoc MetaInfo::setFinalBlock()
+ */
Data&
setFinalBlock(std::optional<name::Component> finalBlockId);
public: // SignatureInfo fields
/**
- * @brief Get the `SignatureType`.
- * @return tlv::SignatureTypeValue, or -1 to indicate the signature is invalid.
+ * @copydoc SignatureInfo::getSignatureType()
*/
int32_t
getSignatureType() const noexcept
diff --git a/ndn-cxx/interest.cpp b/ndn-cxx/interest.cpp
index 4dfde5c..8a07cc8 100644
--- a/ndn-cxx/interest.cpp
+++ b/ndn-cxx/interest.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-2024 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -82,14 +82,13 @@
}
// HopLimit
- if (getHopLimit()) {
+ if (m_hopLimit) {
totalLength += prependBinaryBlock(encoder, tlv::HopLimit, {*m_hopLimit});
}
// InterestLifetime
- if (getInterestLifetime() != DEFAULT_INTEREST_LIFETIME) {
- totalLength += prependNonNegativeIntegerBlock(encoder, tlv::InterestLifetime,
- static_cast<uint64_t>(getInterestLifetime().count()));
+ if (m_interestLifetime != DEFAULT_INTEREST_LIFETIME.count()) {
+ totalLength += prependNonNegativeIntegerBlock(encoder, tlv::InterestLifetime, m_interestLifetime);
}
// Nonce
@@ -177,7 +176,7 @@
m_canBePrefix = m_mustBeFresh = false;
m_forwardingHint.clear();
m_nonce.reset();
- m_interestLifetime = DEFAULT_INTEREST_LIFETIME;
+ m_interestLifetime = DEFAULT_INTEREST_LIFETIME.count();
m_hopLimit.reset();
m_parameters.clear();
@@ -262,7 +261,7 @@
if (lastElement >= 6) {
NDN_THROW(Error("InterestLifetime element is out of order"));
}
- m_interestLifetime = time::milliseconds(readNonNegativeInteger(*element));
+ m_interestLifetime = readNonNegativeInteger(*element);
lastElement = 6;
break;
}
@@ -439,6 +438,15 @@
m_wire.reset();
}
+time::milliseconds
+Interest::getInterestLifetime() const noexcept
+{
+ if (m_interestLifetime > static_cast<uint64_t>(time::milliseconds::max().count())) {
+ return time::milliseconds::max();
+ }
+ return time::milliseconds(m_interestLifetime);
+}
+
Interest&
Interest::setInterestLifetime(time::milliseconds lifetime)
{
@@ -446,8 +454,9 @@
NDN_THROW(std::invalid_argument("InterestLifetime must be >= 0"));
}
- if (lifetime != m_interestLifetime) {
- m_interestLifetime = lifetime;
+ uint64_t lifetimeMillis = static_cast<uint64_t>(lifetime.count());
+ if (lifetimeMillis != m_interestLifetime) {
+ m_interestLifetime = lifetimeMillis;
m_wire.reset();
}
return *this;
diff --git a/ndn-cxx/interest.hpp b/ndn-cxx/interest.hpp
index 965da32..86b7d95 100644
--- a/ndn-cxx/interest.hpp
+++ b/ndn-cxx/interest.hpp
@@ -94,40 +94,46 @@
}
};
- /** @brief Construct an Interest with given @p name and @p lifetime.
+ /**
+ * @brief Construct an Interest with given @p name and @p lifetime.
*
- * @throw std::invalid_argument @p name is invalid or @p lifetime is negative
- * @warning In certain contexts that use `Interest::shared_from_this()`, Interest must be created
- * using `make_shared`. Otherwise, `shared_from_this()` will trigger undefined behavior.
+ * @throw std::invalid_argument @p name is invalid or @p lifetime is negative.
+ * @warning In certain contexts that use `Interest::shared_from_this()`, the Interest must be created
+ * using `std::make_shared`. Otherwise, `shared_from_this()` will throw `std::bad_weak_ptr`.
*/
explicit
Interest(const Name& name = {}, time::milliseconds lifetime = DEFAULT_INTEREST_LIFETIME);
- /** @brief Construct an Interest by decoding from @p wire.
+ /**
+ * @brief Construct an Interest by decoding from @p wire.
*
- * @warning In certain contexts that use `Interest::shared_from_this()`, Interest must be created
- * using `make_shared`. Otherwise, `shared_from_this()` will trigger undefined behavior.
+ * @warning In certain contexts that use `Interest::shared_from_this()`, the Interest must be created
+ * using `std::make_shared`. Otherwise, `shared_from_this()` will throw `std::bad_weak_ptr`.
*/
explicit
Interest(const Block& wire);
- /** @brief Prepend wire encoding to @p encoder.
+ /**
+ * @brief Prepend wire encoding to @p encoder.
*/
template<encoding::Tag TAG>
size_t
wireEncode(EncodingImpl<TAG>& encoder) const;
- /** @brief Encode into a Block.
+ /**
+ * @brief Encode into a Block.
*/
const Block&
wireEncode() const;
- /** @brief Decode from @p wire.
+ /**
+ * @brief Decode from @p wire.
*/
void
wireDecode(const Block& wire);
- /** @brief Check if this instance has cached wire encoding.
+ /**
+ * @brief Check if this instance has cached wire encoding.
*/
bool
hasWire() const noexcept
@@ -135,12 +141,14 @@
return m_wire.hasWire();
}
- /** @brief Return a URI-like string that represents the Interest.
+ /**
+ * @brief Return a URI-like string that represents the Interest.
*
- * The string always starts with `getName().toUri()`. After the name, if any of the
- * Interest's CanBePrefix, MustBeFresh, Nonce, InterestLifetime, or HopLimit fields
- * are present, their textual representation is appended as a query string.
- * Example: "/test/name?MustBeFresh&Nonce=123456"
+ * The string always starts with the Interest's name in URI format. After the name, if any
+ * of the Interest's CanBePrefix, MustBeFresh, Nonce, InterestLifetime, or HopLimit fields
+ * are present, their textual representation is appended as a query string.
+ *
+ * Example: `"/test/name?MustBeFresh&Nonce=123456"`
*/
std::string
toUri() const;
@@ -265,12 +273,13 @@
/**
* @brief Get the %Interest's lifetime.
+ *
+ * If the `InterestLifetime` element is not present, returns #DEFAULT_INTEREST_LIFETIME.
+ * If the `InterestLifetime` value is not representable in the return type, it's clamped to
+ * the nearest representable value.
*/
time::milliseconds
- getInterestLifetime() const noexcept
- {
- return m_interestLifetime;
- }
+ getInterestLifetime() const noexcept;
/**
* @brief Set the %Interest's lifetime.
@@ -508,7 +517,7 @@
Name m_name;
std::vector<Name> m_forwardingHint;
mutable std::optional<Nonce> m_nonce;
- time::milliseconds m_interestLifetime = DEFAULT_INTEREST_LIFETIME;
+ uint64_t m_interestLifetime = DEFAULT_INTEREST_LIFETIME.count();
std::optional<uint8_t> m_hopLimit;
bool m_canBePrefix = false;
bool m_mustBeFresh = false;
diff --git a/ndn-cxx/meta-info.cpp b/ndn-cxx/meta-info.cpp
index 3e4d6f1..918b089 100644
--- a/ndn-cxx/meta-info.cpp
+++ b/ndn-cxx/meta-info.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-2024 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -43,7 +43,7 @@
}
time::milliseconds
-MetaInfo::getFreshnessPeriod() const
+MetaInfo::getFreshnessPeriod() const noexcept
{
if (m_freshnessPeriod > static_cast<uint64_t>(time::milliseconds::max().count())) {
return time::milliseconds::max();
@@ -144,7 +144,7 @@
}
// FreshnessPeriod
- if (m_freshnessPeriod != static_cast<uint64_t>(DEFAULT_FRESHNESS_PERIOD.count())) {
+ if (m_freshnessPeriod != DEFAULT_FRESHNESS_PERIOD.count()) {
totalLength += prependNonNegativeIntegerBlock(encoder, tlv::FreshnessPeriod, m_freshnessPeriod);
}
@@ -205,7 +205,7 @@
++val;
}
else {
- m_freshnessPeriod = static_cast<uint64_t>(DEFAULT_FRESHNESS_PERIOD.count());
+ m_freshnessPeriod = DEFAULT_FRESHNESS_PERIOD.count();
}
// FinalBlockId
diff --git a/ndn-cxx/meta-info.hpp b/ndn-cxx/meta-info.hpp
index 5f0df92..52a8f08 100644
--- a/ndn-cxx/meta-info.hpp
+++ b/ndn-cxx/meta-info.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-2024 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -86,18 +86,20 @@
wireDecode(const Block& wire);
public: // getter/setter
- /** @brief Return the value of `ContentType`.
+ /**
+ * @brief Return the value of `ContentType`.
*
- * If the `ContentType` element is not present, returns tlv::ContentType_Blob.
+ * If the `ContentType` element is not present, returns tlv::ContentType_Blob.
*/
uint32_t
- getType() const
+ getType() const noexcept
{
return m_type;
}
- /** @brief Set `ContentType`.
- * @param type a number defined in tlv::ContentTypeValue
+ /**
+ * @brief Set the `ContentType`.
+ * @param type A number defined in tlv::ContentTypeValue
*/
MetaInfo&
setType(uint32_t type);
@@ -110,10 +112,11 @@
* the nearest representable value.
*/
time::milliseconds
- getFreshnessPeriod() const;
+ getFreshnessPeriod() const noexcept;
- /** @brief Set `FreshnessPeriod`.
- * @throw std::invalid_argument specified FreshnessPeriod is negative
+ /**
+ * @brief Set the `FreshnessPeriod`.
+ * @throw std::invalid_argument specified FreshnessPeriod is negative.
*/
MetaInfo&
setFreshnessPeriod(time::milliseconds freshnessPeriod);
@@ -122,13 +125,13 @@
* @brief Return the value of `FinalBlockId`.
*/
const std::optional<name::Component>&
- getFinalBlock() const
+ getFinalBlock() const noexcept
{
return m_finalBlockId;
}
/**
- * @brief Set `FinalBlockId`.
+ * @brief Set the `FinalBlockId`.
*/
MetaInfo&
setFinalBlock(std::optional<name::Component> finalBlockId);
@@ -196,7 +199,7 @@
private:
uint32_t m_type = tlv::ContentType_Blob;
- uint64_t m_freshnessPeriod = static_cast<uint64_t>(DEFAULT_FRESHNESS_PERIOD.count());
+ uint64_t m_freshnessPeriod = DEFAULT_FRESHNESS_PERIOD.count();
std::optional<name::Component> m_finalBlockId;
std::list<Block> m_appMetaInfo;
diff --git a/tests/unit/interest.t.cpp b/tests/unit/interest.t.cpp
index 4c540b7..5e64fb1 100644
--- a/tests/unit/interest.t.cpp
+++ b/tests/unit/interest.t.cpp
@@ -611,6 +611,23 @@
[] (const auto& e) { return e.what() == "Nonce element is malformed"sv; });
}
+BOOST_AUTO_TEST_CASE(LargeLifetime,
+ * ut::description("test for bug #4997"))
+{
+ i.wireDecode("050F 0703(080149) 0C087FFFFFFFFFFFFFFF"_block);
+ BOOST_CHECK_EQUAL(i.getInterestLifetime(), 0x7FFFFFFFFFFFFFFF_ms);
+
+ i.wireDecode("050F 0703(080149) 0C088000000000000000"_block);
+ BOOST_CHECK_EQUAL(i.getInterestLifetime(), time::milliseconds::max());
+
+ i.wireDecode("050F 0703(080149) 0C08FFFFFFFFFFFFFFFF"_block);
+ BOOST_CHECK_EQUAL(i.getInterestLifetime(), time::milliseconds::max());
+
+ // force re-encoding
+ i.setNonce(0x957c6554);
+ BOOST_CHECK_EQUAL(i.wireEncode(), "0515 0703(080149) 0A04957C6554 0C08FFFFFFFFFFFFFFFF"_block);
+}
+
BOOST_AUTO_TEST_CASE(BadHopLimit)
{
BOOST_CHECK_EXCEPTION(i.wireDecode("0507 0703080149 2200"_block), tlv::Error,
diff --git a/tests/unit/meta-info.t.cpp b/tests/unit/meta-info.t.cpp
index f375607..b9bb0df 100644
--- a/tests/unit/meta-info.t.cpp
+++ b/tests/unit/meta-info.t.cpp
@@ -78,9 +78,9 @@
BOOST_CHECK_EQUAL(b.wireEncode(), wire3);
}
-BOOST_AUTO_TEST_CASE(FreshnessPeriodOverflow)
+BOOST_AUTO_TEST_CASE(FreshnessPeriodOverflow,
+ * ut::description("test for bug #4997"))
{
- // Bug #4997
MetaInfo mi0("140A 19087FFFFFFFFFFFFFFF"_block);
BOOST_CHECK_EQUAL(mi0.getFreshnessPeriod(), 0x7FFFFFFFFFFFFFFF_ms);
@@ -89,6 +89,10 @@
MetaInfo mi2("140A 1908FFFFFFFFFFFFFFFF"_block);
BOOST_CHECK_EQUAL(mi2.getFreshnessPeriod(), time::milliseconds::max());
+
+ // force re-encoding
+ mi2.setType(tlv::ContentType_Key);
+ BOOST_CHECK_EQUAL(mi2.wireEncode(), "140D 180102 1908FFFFFFFFFFFFFFFF"_block);
}
BOOST_AUTO_TEST_CASE(AppMetaInfo)