interest+data: add setSignatureValue() overload that takes a span

Change-Id: Ic6a55bf91e0d62760fb75a643cd7dbc1e92c31f2

ndn-cxx/data.cpp

@@ -306,6 +306,14 @@ Data::setSignatureInfo(const SignatureInfo& info)
   return *this;
 }
 
+Data&
+Data::setSignatureValue(span<const uint8_t> value)
+{
+  m_signatureValue = makeBinaryBlock(tlv::SignatureValue, value);
+  resetWire();
+  return *this;
+}
+
 Data&
 Data::setSignatureValue(ConstBufferPtr value)
 {

ndn-cxx/data.hpp

@@ -243,7 +243,8 @@ public: // Data fields
   Data&
   setSignatureInfo(const SignatureInfo& info);
 
-  /** @brief Get SignatureValue
+  /**
+   * @brief Get the SignatureValue element.
    */
   const Block&
   getSignatureValue() const noexcept
@@ -251,14 +252,28 @@ public: // Data fields
     return m_signatureValue;
   }
 
-  /** @brief Set SignatureValue
-   *  @param value buffer containing the TLV-VALUE of the SignatureValue; must not be nullptr
+  /**
+   * @brief Set SignatureValue by copying from a contiguous sequence of bytes.
+   * @param value buffer from which the TLV-VALUE of the SignatureValue will be copied
+   * @return a reference to this Data, to allow chaining
    *
-   *  This is a low-level function that should not normally be called directly by applications.
-   *  Instead, use KeyChain::sign() to sign the packet.
+   * This is a low-level function that should not normally be called directly by applications.
+   * Instead, use KeyChain::sign() to sign the packet.
    *
-   *  @return a reference to this Data, to allow chaining
-   *  @warning SignatureValue is overwritten when the packet is signed via KeyChain::sign().
+   * @warning SignatureValue is overwritten when the packet is signed via KeyChain::sign().
+   */
+  Data&
+  setSignatureValue(span<const uint8_t> value);
+
+  /**
+   * @brief Set SignatureValue from a shared buffer.
+   * @param value buffer containing the TLV-VALUE of the SignatureValue; must not be nullptr
+   * @return a reference to this Data, to allow chaining
+   *
+   * This is a low-level function that should not normally be called directly by applications.
+   * Instead, use KeyChain::sign() to sign the packet.
+   *
+   * @warning SignatureValue is overwritten when the packet is signed via KeyChain::sign().
    */
   Data&
   setSignatureValue(ConstBufferPtr value);

ndn-cxx/interest.cpp

@@ -455,7 +455,7 @@ Interest::setHopLimit(optional<uint8_t> hopLimit)
   return *this;
 }
 
-void
+Interest&
 Interest::setApplicationParametersInternal(Block parameters)
 {
   parameters.encode(); // ensure we have wire encoding needed by computeParametersDigest()
@@ -466,6 +466,10 @@ Interest::setApplicationParametersInternal(Block parameters)
     BOOST_ASSERT(m_parameters[0].type() == tlv::ApplicationParameters);
     m_parameters[0] = std::move(parameters);
   }
+
+  addOrReplaceParametersDigestComponent();
+  m_wire.reset();
+  return *this;
 }
 
 Interest&
@@ -476,23 +480,17 @@ Interest::setApplicationParameters(const Block& parameters)
   }
 
   if (parameters.type() == tlv::ApplicationParameters) {
-    setApplicationParametersInternal(parameters);
+    return setApplicationParametersInternal(parameters);
   }
   else {
-    setApplicationParametersInternal(Block(tlv::ApplicationParameters, parameters));
+    return setApplicationParametersInternal({tlv::ApplicationParameters, parameters});
   }
-  addOrReplaceParametersDigestComponent();
-  m_wire.reset();
-  return *this;
 }
 
 Interest&
 Interest::setApplicationParameters(span<const uint8_t> value)
 {
-  setApplicationParametersInternal(makeBinaryBlock(tlv::ApplicationParameters, value));
-  addOrReplaceParametersDigestComponent();
-  m_wire.reset();
-  return *this;
+  return setApplicationParametersInternal(makeBinaryBlock(tlv::ApplicationParameters, value));
 }
 
 Interest&
@@ -512,10 +510,7 @@ Interest::setApplicationParameters(ConstBufferPtr value)
     NDN_THROW(std::invalid_argument("ApplicationParameters buffer cannot be nullptr"));
   }
 
-  setApplicationParametersInternal(Block(tlv::ApplicationParameters, std::move(value)));
-  addOrReplaceParametersDigestComponent();
-  m_wire.reset();
-  return *this;
+  return setApplicationParametersInternal({tlv::ApplicationParameters, std::move(value)});
 }
 
 Interest&
@@ -595,12 +590,8 @@ Interest::getSignatureValue() const
 }
 
 Interest&
-Interest::setSignatureValue(ConstBufferPtr value)
+Interest::setSignatureValueInternal(Block sigValue)
 {
-  if (value == nullptr) {
-    NDN_THROW(std::invalid_argument("InterestSignatureValue buffer cannot be nullptr"));
-  }
-
   // Ensure presence of InterestSignatureInfo
   auto infoIt = findFirstParameter(tlv::InterestSignatureInfo);
   if (infoIt == m_parameters.end()) {
@@ -611,22 +602,21 @@ Interest::setSignatureValue(ConstBufferPtr value)
     return block.type() == tlv::InterestSignatureValue;
   });
 
-  Block valueBlock(tlv::InterestSignatureValue, std::move(value));
   if (valueIt != m_parameters.end()) {
-    if (*valueIt == valueBlock) {
+    if (*valueIt == sigValue) {
       // New InterestSignatureValue is the same as the old InterestSignatureValue
       return *this;
     }
 
     // Replace existing InterestSignatureValue
-    *valueIt = std::move(valueBlock);
+    *valueIt = std::move(sigValue);
   }
   else {
     // Place after first InterestSignatureInfo element
-    valueIt = m_parameters.insert(std::next(infoIt), std::move(valueBlock));
+    valueIt = m_parameters.insert(std::next(infoIt), std::move(sigValue));
   }
 
-  // computeParametersDigest needs encoded SignatureValue
+  // computeParametersDigest needs encoded InterestSignatureValue
   valueIt->encode();
 
   addOrReplaceParametersDigestComponent();
@@ -634,6 +624,22 @@ Interest::setSignatureValue(ConstBufferPtr value)
   return *this;
 }
 
+Interest&
+Interest::setSignatureValue(span<const uint8_t> value)
+{
+  return setSignatureValueInternal(makeBinaryBlock(tlv::InterestSignatureValue, value));
+}
+
+Interest&
+Interest::setSignatureValue(ConstBufferPtr value)
+{
+  if (value == nullptr) {
+    NDN_THROW(std::invalid_argument("InterestSignatureValue buffer cannot be nullptr"));
+  }
+
+  return setSignatureValueInternal({tlv::InterestSignatureValue, std::move(value)});
+}
+
 InputBuffers
 Interest::extractSignedRanges() const
 {

ndn-cxx/interest.hpp

@@ -392,18 +392,32 @@ public: // element access
   Interest&
   setSignatureInfo(const SignatureInfo& info);
 
-  /** @brief Get the InterestSignatureValue
+  /**
+   * @brief Get the InterestSignatureValue element.
    *
-   *  If the element is not present, an invalid Block will be returned.
+   * If the element is not present, an invalid Block will be returned.
    */
   Block
   getSignatureValue() const;
 
-  /** @brief Set the InterestSignatureValue
-   *  @param value Buffer containing the TLV-VALUE of the InterestSignatureValue; must not be nullptr
-   *  @throw Error InterestSignatureInfo is unset
+  /**
+   * @brief Set InterestSignatureValue by copying from a contiguous sequence of bytes.
+   * @param value buffer from which the TLV-VALUE of the InterestSignatureValue will be copied
+   * @return a reference to this Interest
+   * @throw Error InterestSignatureInfo is unset
    *
-   *  InterestSignatureInfo must be set before setting InterestSignatureValue
+   * InterestSignatureInfo must be set before setting InterestSignatureValue.
+   */
+  Interest&
+  setSignatureValue(span<const uint8_t> value);
+
+  /**
+   * @brief Set InterestSignatureValue from a shared buffer.
+   * @param value buffer containing the TLV-VALUE of the InterestSignatureValue; must not be nullptr
+   * @return a reference to this Interest
+   * @throw Error InterestSignatureInfo is unset
+   *
+   * InterestSignatureInfo must be set before setting InterestSignatureValue.
    */
   Interest&
   setSignatureValue(ConstBufferPtr value);
@@ -439,9 +453,12 @@ public: // ParametersSha256DigestComponent support
   isParametersDigestValid() const;
 
 private:
-  void
+  Interest&
   setApplicationParametersInternal(Block parameters);
 
+  Interest&
+  setSignatureValueInternal(Block sigValue);
+
   NDN_CXX_NODISCARD shared_ptr<Buffer>
   computeParametersDigest() const;
 

tests/unit/data.t.cpp

@@ -510,16 +510,14 @@ BOOST_AUTO_TEST_CASE(SetContent)
   d.setContent("1502CAFE"_block);
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
-  BOOST_CHECK_EQUAL_COLLECTIONS(d.getContent().value_begin(), d.getContent().value_end(),
-                                direct, direct + sizeof(direct));
+  BOOST_TEST(d.getContent().value_bytes() == direct, boost::test_tools::per_element());
 
   // Block overload, nested inside Content element
   const uint8_t nested[] = {0x99, 0x02, 0xca, 0xfe};
   d.setContent(Block(nested));
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
-  BOOST_CHECK_EQUAL_COLLECTIONS(d.getContent().value_begin(), d.getContent().value_end(),
-                                nested, nested + sizeof(nested));
+  BOOST_TEST(d.getContent().value_bytes() == nested, boost::test_tools::per_element());
 
   // Block overload, default constructed (invalid)
   BOOST_CHECK_THROW(d.setContent(Block{}), std::invalid_argument);
@@ -528,8 +526,7 @@ BOOST_AUTO_TEST_CASE(SetContent)
   d.setContent(nested);
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
-  BOOST_CHECK_EQUAL_COLLECTIONS(d.getContent().value_begin(), d.getContent().value_end(),
-                                nested, nested + sizeof(nested));
+  BOOST_TEST(d.getContent().value_bytes() == nested, boost::test_tools::per_element());
   d.setContent(span<uint8_t>{});
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
@@ -545,8 +542,7 @@ BOOST_AUTO_TEST_CASE(SetContent)
   d.setContent(std::make_shared<Buffer>(direct, sizeof(direct)));
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
-  BOOST_CHECK_EQUAL_COLLECTIONS(d.getContent().value_begin(), d.getContent().value_end(),
-                                direct, direct + sizeof(direct));
+  BOOST_TEST(d.getContent().value_bytes() == direct, boost::test_tools::per_element());
   d.setContent(std::make_shared<Buffer>());
   BOOST_CHECK_EQUAL(d.hasContent(), true);
   BOOST_CHECK_EQUAL(d.getContent().type(), tlv::Content);
@@ -565,11 +561,18 @@ BOOST_AUTO_TEST_CASE(SetSignatureValue)
   Data d;
   BOOST_CHECK_EQUAL(d.getSignatureValue().type(), tlv::Invalid);
 
-  d.setSignatureValue(fromHex("FACADE"));
+  // span overload
+  const uint8_t sv1[] = {0xbe, 0xef};
+  d.setSignatureValue(sv1);
   BOOST_CHECK_EQUAL(d.getSignatureValue().type(), tlv::SignatureValue);
-  BOOST_CHECK_EQUAL(d.getSignatureValue().value_size(), 3);
+  BOOST_TEST(d.getSignatureValue().value_bytes() == sv1, boost::test_tools::per_element());
 
-  d.setSignatureValue(std::make_shared<Buffer>()); // empty buffer
+  // ConstBufferPtr overload
+  const uint8_t sv2[] = {0xfa, 0xca, 0xde};
+  d.setSignatureValue(std::make_shared<Buffer>(sv2, sizeof(sv2)));
+  BOOST_CHECK_EQUAL(d.getSignatureValue().type(), tlv::SignatureValue);
+  BOOST_TEST(d.getSignatureValue().value_bytes() == sv2, boost::test_tools::per_element());
+  d.setSignatureValue(std::make_shared<Buffer>());
   BOOST_CHECK_EQUAL(d.getSignatureValue().type(), tlv::SignatureValue);
   BOOST_CHECK_EQUAL(d.getSignatureValue().value_size(), 0);
 

tests/unit/interest.t.cpp

@@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(Signed)
   i2.setNonce(0x4c1ecb4a);
   i2.setApplicationParameters("2404C0C1C2C3"_block);
   i2.setSignatureInfo(si);
-  i2.setSignatureValue(make_shared<Buffer>(sv.value_begin(), sv.value_end()));
+  i2.setSignatureValue(sv.value_bytes());
   BOOST_CHECK_EQUAL(i2.isParametersDigestValid(), true);
 
   BOOST_TEST(i2.wireEncode() == WIRE, boost::test_tools::per_element());
@@ -901,13 +901,15 @@ BOOST_AUTO_TEST_CASE(SetApplicationParameters)
 BOOST_AUTO_TEST_CASE(SetSignature)
 {
   Interest i;
-  BOOST_CHECK(i.getSignatureInfo() == nullopt);
-  BOOST_CHECK_EQUAL(i.isSigned(), false);
 
-  // Throws because attempting to set InterestSignatureValue without set InterestSignatureInfo
   Block sv1("2E04 01020304"_block);
-  auto svBuffer1 = make_shared<Buffer>(sv1.value_begin(), sv1.value_end());
-  BOOST_CHECK_THROW(i.setSignatureValue(svBuffer1), tlv::Error);
+  BOOST_CHECK_EXCEPTION(i.setSignatureValue(sv1.value_bytes()), tlv::Error, [] (const auto& e) {
+    return e.what() == "InterestSignatureInfo must be present to set InterestSignatureValue"s;
+  });
+
+  BOOST_CHECK(i.getSignatureInfo() == nullopt);
+  BOOST_CHECK_EQUAL(i.getSignatureValue().isValid(), false);
+  BOOST_CHECK_EQUAL(i.isSigned(), false);
 
   // Simple set/get case for InterestSignatureInfo (no prior set)
   SignatureInfo si1(tlv::SignatureSha256WithEcdsa);
@@ -916,7 +918,7 @@ BOOST_AUTO_TEST_CASE(SetSignature)
   BOOST_CHECK_EQUAL(i.isSigned(), false);
 
   // Simple set/get case for InterestSignatureValue (no prior set)
-  BOOST_CHECK_EQUAL(i.getSignatureValue().isValid(), false);
+  auto svBuffer1 = make_shared<Buffer>(sv1.value_begin(), sv1.value_end());
   i.setSignatureValue(svBuffer1);
   BOOST_CHECK_EQUAL(i.getSignatureValue(), sv1);
   BOOST_CHECK_EQUAL(i.isSigned(), true);
@@ -948,7 +950,7 @@ BOOST_AUTO_TEST_CASE(SetSignature)
   // Ensure that wire is not reset if specified InterestSignatureValue is same
   i.wireEncode();
   BOOST_CHECK_EQUAL(i.hasWire(), true);
-  i.setSignatureValue(svBuffer1);
+  i.setSignatureValue(sv1.value_bytes());
   BOOST_CHECK_EQUAL(i.hasWire(), true);
   BOOST_CHECK(i.getSignatureInfo() == si2);
   BOOST_CHECK_EQUAL(i.getSignatureValue(), sv1);
@@ -957,12 +959,11 @@ BOOST_AUTO_TEST_CASE(SetSignature)
   // Ensure that wire is reset if specified InterestSignatureValue is different
   i.wireEncode();
   BOOST_CHECK_EQUAL(i.hasWire(), true);
-  Block sv2("2E04 99887766"_block);
-  auto svBuffer2 = make_shared<Buffer>(sv2.value_begin(), sv2.value_end());
-  i.setSignatureValue(svBuffer2);
+  const uint8_t sv2[] = {0x99, 0x88, 0x77, 0x66};
+  i.setSignatureValue(sv2);
   BOOST_CHECK_EQUAL(i.hasWire(), false);
   BOOST_CHECK(i.getSignatureInfo() == si2);
-  BOOST_CHECK_EQUAL(i.getSignatureValue(), sv2);
+  BOOST_TEST(i.getSignatureValue().value_bytes() == sv2, boost::test_tools::per_element());
   BOOST_CHECK_EQUAL(i.isSigned(), true);
 }
 
@@ -1027,8 +1028,7 @@ BOOST_AUTO_TEST_CASE(ParametersSha256DigestComponent)
   BOOST_CHECK(i.getSignatureInfo() == si);
 
   Block sv("2E04 01020304"_block);
-  i.setSignatureValue(make_shared<Buffer>(sv.value_begin(),
-                                          sv.value_end())); // updates ParametersDigestSha256Component
+  i.setSignatureValue(sv.value_bytes()); // updates ParametersDigestSha256Component
   BOOST_CHECK_EQUAL(i.getName(),
                     "/A/B/C/params-sha256=f649845ef944638390d1c689e2f0618ea02e471eff236110cbeb822d5932d342");
   BOOST_CHECK_EQUAL(i.isParametersDigestValid(), true);
@@ -1071,8 +1071,7 @@ BOOST_AUTO_TEST_CASE(ExtractSignedRanges)
                                 appParamsWire1->begin(), wire1.end());
 
   // Test with Interest with existing InterestSignatureValue
-  auto sigValue = make_shared<Buffer>();
-  i1.setSignatureValue(sigValue);
+  i1.setSignatureValue(std::make_shared<Buffer>());
   auto ranges2 = i1.extractSignedRanges();
   BOOST_REQUIRE_EQUAL(ranges2.size(), 2);
   const auto& wire2 = i1.wireEncode();

tests/unit/security/certificate.t.cpp

@@ -129,12 +129,12 @@ static void
 generateFakeSignature(Data& data)
 {
   SignatureInfo signatureInfo(Block{SIG_INFO});
-  signatureInfo.setKeyLocator(KeyLocator(Name("/ndn/site1/KEY/ksk-2516425377094")));
+  signatureInfo.setKeyLocator(Name("/ndn/site1/KEY/ksk-2516425377094"));
   signatureInfo.setValidityPeriod(ValidityPeriod(time::fromIsoString("20141111T050000"),
                                                  time::fromIsoString("20141111T060000")));
 
   data.setSignatureInfo(signatureInfo);
-  data.setSignatureValue(make_shared<Buffer>(SIG_VALUE, sizeof(SIG_VALUE)));
+  data.setSignatureValue(SIG_VALUE);
 }
 
 BOOST_AUTO_TEST_CASE(Construction)