Code Review fix 1
Change-Id: I92fa40b6dd0a05913461006acc87542884c1f3a5
diff --git a/src/detail/ca-request-state.cpp b/src/detail/ca-request-state.cpp
index 0de0820..923f9be 100644
--- a/src/detail/ca-request-state.cpp
+++ b/src/detail/ca-request-state.cpp
@@ -24,7 +24,8 @@
namespace ndn {
namespace ndncert {
-std::string statusToString(Status status) {
+std::string statusToString(Status status)
+{
switch (status)
{
case Status::BEFORE_CHALLENGE:
@@ -46,6 +47,15 @@
}
}
+Status
+statusFromBlock(const Block& block)
+{
+ auto status_int = readNonNegativeInteger(block);
+ if (status_int > 6)
+ NDN_THROW(std::runtime_error("Unrecognized Status"));
+ return static_cast<Status>(status_int);
+}
+
namespace ca {
ChallengeState::ChallengeState(const std::string& challengeStatus,
diff --git a/src/detail/ca-request-state.hpp b/src/detail/ca-request-state.hpp
index 7fa9f23..27d19fa 100644
--- a/src/detail/ca-request-state.hpp
+++ b/src/detail/ca-request-state.hpp
@@ -45,6 +45,12 @@
std::string
statusToString(Status status);
+/**
+ * @brief Convert request status to string.
+ */
+Status
+statusFromBlock(const Block& block);
+
namespace ca {
/**
diff --git a/src/detail/challenge-encoder.cpp b/src/detail/challenge-encoder.cpp
index 22dc6c7..c9fa34b 100644
--- a/src/detail/challenge-encoder.cpp
+++ b/src/detail/challenge-encoder.cpp
@@ -24,10 +24,10 @@
namespace ndncert {
Block
-ChallengeEncoder::encodeDataContent(ca::RequestState& request, optional<Name> issuedCertName)
+ChallengeEncoder::encodeDataContent(ca::RequestState& request, const Name& issuedCertName)
{
Block response = makeEmptyBlock(tlv::EncryptedPayload);
- response.push_back(makeNonNegativeIntegerBlock(tlv::Status, static_cast<size_t>(request.m_status)));
+ response.push_back(makeNonNegativeIntegerBlock(tlv::Status, static_cast<uint64_t>(request.m_status)));
if (request.m_challengeState) {
response.push_back(makeStringBlock(tlv::ChallengeStatus, request.m_challengeState->m_challengeStatus));
response.push_back(
@@ -35,8 +35,8 @@
response.push_back(
makeNonNegativeIntegerBlock(tlv::RemainingTime, request.m_challengeState->m_remainingTime.count()));
}
- if (issuedCertName.has_value()) {
- response.push_back(makeNestedBlock(tlv::IssuedCertName, *issuedCertName));
+ if (!issuedCertName.empty()) {
+ response.push_back(makeNestedBlock(tlv::IssuedCertName, issuedCertName));
}
response.encode();
return encodeBlockWithAesGcm128(ndn::tlv::Content, request.m_encryptionKey.data(),
@@ -45,13 +45,13 @@
}
void
-ChallengeEncoder::decodeDataContent(const Block& contentBlock, requester::RequestContext& state)
+ChallengeEncoder::decodeDataContent(const Block& contentBlock, requester::RequestState& state)
{
auto result = decodeBlockWithAesGcm128(contentBlock, state.m_aesKey.data(),
state.m_requestId.data(), state.m_requestId.size());
auto data = makeBinaryBlock(tlv::EncryptedPayload, result.data(), result.size());
data.parse();
- state.m_status = static_cast<Status>(readNonNegativeInteger(data.get(tlv::Status)));
+ state.m_status = statusFromBlock(data.get(tlv::Status));
if (data.find(tlv::ChallengeStatus) != data.elements_end()) {
state.m_challengeStatus = readString(data.get(tlv::ChallengeStatus));
}
diff --git a/src/detail/challenge-encoder.hpp b/src/detail/challenge-encoder.hpp
index 3eaf4c4..4aa3b5a 100644
--- a/src/detail/challenge-encoder.hpp
+++ b/src/detail/challenge-encoder.hpp
@@ -22,7 +22,7 @@
#define NDNCERT_DETAIL_CHALLENGE_ENCODER_HPP
#include "detail/ca-request-state.hpp"
-#include "requester-state.hpp"
+#include "requester-request-state.hpp"
namespace ndn {
namespace ndncert {
@@ -31,10 +31,10 @@
{
public:
static Block
- encodeDataContent(ca::RequestState& request, optional<Name> issuedCertName = nullopt);
+ encodeDataContent(ca::RequestState& request, const Name& issuedCertName = Name());
static void
- decodeDataContent(const Block& data, requester::RequestContext& state);
+ decodeDataContent(const Block& contentBlock, requester::RequestState& state);
};
} // namespace ndncert
diff --git a/src/detail/new-renew-revoke-encoder.cpp b/src/detail/new-renew-revoke-encoder.cpp
index 577d1de..fd4f48c 100644
--- a/src/detail/new-renew-revoke-encoder.cpp
+++ b/src/detail/new-renew-revoke-encoder.cpp
@@ -101,7 +101,7 @@
std::array<uint8_t, 32>& salt, RequestId& requestId, Status& status)
{
content.parse();
- status = static_cast<Status>(readNonNegativeInteger(content.get(tlv::Status)));
+ status = statusFromBlock(content.get(tlv::Status));
const auto& ecdhBlock = content.get(tlv::EcdhPub);
ecdhKey.resize(ecdhBlock.value_size());
diff --git a/src/requester-state.cpp b/src/requester-request-state.cpp
similarity index 88%
rename from src/requester-state.cpp
rename to src/requester-request-state.cpp
index 3e14de5..032e56c 100644
--- a/src/requester-state.cpp
+++ b/src/requester-request-state.cpp
@@ -18,13 +18,13 @@
* See AUTHORS.md for complete list of ndncert authors and contributors.
*/
-#include "requester-state.hpp"
+#include "requester-request-state.hpp"
namespace ndn {
namespace ndncert {
namespace requester {
-RequestContext::RequestContext(security::KeyChain& keyChain, const CaProfile& caItem, RequestType requestType)
+RequestState::RequestState(security::KeyChain& keyChain, const CaProfile& caItem, RequestType requestType)
: m_caItem(caItem)
, m_keyChain(keyChain)
, m_type(requestType)
diff --git a/src/requester-state.hpp b/src/requester-request-state.hpp
similarity index 91%
rename from src/requester-state.hpp
rename to src/requester-request-state.hpp
index 9c14c1e..fcda478 100644
--- a/src/requester-state.hpp
+++ b/src/requester-request-state.hpp
@@ -18,8 +18,8 @@
* See AUTHORS.md for complete list of ndncert authors and contributors.
*/
-#ifndef NDNCERT_REQUESTER_STATE_HPP
-#define NDNCERT_REQUESTER_STATE_HPP
+#ifndef NDNCERT_REQUESTER_REQUEST_STATE_HPP
+#define NDNCERT_REQUESTER_REQUEST_STATE_HPP
#include "detail/ca-request-state.hpp"
#include "detail/crypto-helpers.hpp"
@@ -29,9 +29,9 @@
namespace ndncert {
namespace requester {
-struct RequestContext {
+struct RequestState {
explicit
- RequestContext(security::KeyChain& keyChain, const CaProfile& caItem, RequestType requestType);
+ RequestState(security::KeyChain& keyChain, const CaProfile& caItem, RequestType requestType);
/**
* @brief The CA profile for this request.
@@ -104,4 +104,4 @@
} // namespace ndncert
} // namespace ndn
-#endif // NDNCERT_REQUESTER_STATE_HPP
+#endif // NDNCERT_REQUESTER_REQUEST_STATE_HPP
diff --git a/src/requester.cpp b/src/requester.cpp
index 8a8dc86..ef839c0 100644
--- a/src/requester.cpp
+++ b/src/requester.cpp
@@ -113,7 +113,7 @@
}
shared_ptr<Interest>
-Requester::genNewInterest(RequestContext& state, const Name& identityName,
+Requester::genNewInterest(RequestState& state, const Name& identityName,
const time::system_clock::TimePoint& notBefore,
const time::system_clock::TimePoint& notAfter)
{
@@ -173,7 +173,7 @@
}
shared_ptr<Interest>
-Requester::genRevokeInterest(RequestContext& state, const security::Certificate& certificate)
+Requester::genRevokeInterest(RequestState& state, const security::Certificate& certificate)
{
if (!state.m_caItem.m_caPrefix.isPrefixOf(certificate.getName())) {
return nullptr;
@@ -190,7 +190,7 @@
}
std::list<std::string>
-Requester::onNewRenewRevokeResponse(RequestContext& state, const Data& reply)
+Requester::onNewRenewRevokeResponse(RequestState& state, const Data& reply)
{
if (!security::verifySignature(reply, *state.m_caItem.m_cert)) {
NDN_LOG_ERROR("Cannot verify replied Data packet signature.");
@@ -213,7 +213,7 @@
}
std::vector<std::tuple<std::string, std::string>>
-Requester::selectOrContinueChallenge(RequestContext& state, const std::string& challengeSelected)
+Requester::selectOrContinueChallenge(RequestState& state, const std::string& challengeSelected)
{
auto challenge = ChallengeModule::createChallengeModule(challengeSelected);
if (challenge == nullptr) {
@@ -224,7 +224,7 @@
}
shared_ptr<Interest>
-Requester::genChallengeInterest(RequestContext& state,
+Requester::genChallengeInterest(RequestState& state,
std::vector<std::tuple<std::string, std::string>>&& parameters)
{
if (state.m_challengeType == "") {
@@ -254,7 +254,7 @@
}
void
-Requester::onChallengeResponse(RequestContext& state, const Data& reply)
+Requester::onChallengeResponse(RequestState& state, const Data& reply)
{
if (!security::verifySignature(reply, *state.m_caItem.m_cert)) {
NDN_LOG_ERROR("Cannot verify replied Data packet signature.");
@@ -265,7 +265,7 @@
}
shared_ptr<Interest>
-Requester::genCertFetchInterest(const RequestContext& state)
+Requester::genCertFetchInterest(const RequestState& state)
{
Name interestName = state.m_issuedCertName;
auto interest =std::make_shared<Interest>(interestName);
@@ -288,7 +288,7 @@
}
void
-Requester::endSession(RequestContext& state)
+Requester::endSession(RequestState& state)
{
if (state.m_status == Status::SUCCESS || state.m_status == Status::ENDED) {
return;
diff --git a/src/requester.hpp b/src/requester.hpp
index 4e2a1b1..825efd9 100644
--- a/src/requester.hpp
+++ b/src/requester.hpp
@@ -21,7 +21,7 @@
#ifndef NDNCERT_REQUESTER_HPP
#define NDNCERT_REQUESTER_HPP
-#include "requester-state.hpp"
+#include "requester-request-state.hpp"
namespace ndn {
namespace ndncert {
@@ -112,7 +112,7 @@
* @return The shared pointer to the encoded interest.
*/
static shared_ptr<Interest>
- genNewInterest(RequestContext& state, const Name& identityName,
+ genNewInterest(RequestState& state, const Name& identityName,
const time::system_clock::TimePoint& notBefore,
const time::system_clock::TimePoint& notAfter);
@@ -124,7 +124,7 @@
* @return The shared pointer to the encoded interest.
*/
static shared_ptr<Interest>
- genRevokeInterest(RequestContext& state, const security::Certificate& certificate);
+ genRevokeInterest(RequestState& state, const security::Certificate& certificate);
/**
* @brief Decodes the replied data of NEW, RENEW, or REVOKE interest from the CA.
@@ -135,7 +135,7 @@
* @throw std::runtime_error if the decoding fails or receiving an error packet.
*/
static std::list<std::string>
- onNewRenewRevokeResponse(RequestContext& state, const Data& reply);
+ onNewRenewRevokeResponse(RequestState& state, const Data& reply);
// CHALLENGE helpers
/**
@@ -148,7 +148,7 @@
* @throw std::runtime_error if the challenge is not supported.
*/
static std::vector<std::tuple<std::string, std::string>>
- selectOrContinueChallenge(RequestContext& state, const std::string& challengeSelected);
+ selectOrContinueChallenge(RequestState& state, const std::string& challengeSelected);
/**
* @brief Generates the CHALLENGE interest for the request.
@@ -159,7 +159,7 @@
* @throw std::runtime_error if the challenge is not selected or is not supported.
*/
static shared_ptr<Interest>
- genChallengeInterest(RequestContext& state,
+ genChallengeInterest(RequestState& state,
std::vector<std::tuple<std::string, std::string>>&& parameters);
/**
@@ -170,7 +170,7 @@
* @throw std::runtime_error if the decoding fails or receiving an error packet.
*/
static void
- onChallengeResponse(RequestContext& state, const Data& reply);
+ onChallengeResponse(RequestState& state, const Data& reply);
/**
* @brief Generate the interest to fetch the issued certificate
@@ -179,7 +179,7 @@
* @return The shared pointer to the encoded interest
*/
static shared_ptr<Interest>
- genCertFetchInterest(const RequestContext& state);
+ genCertFetchInterest(const RequestState& state);
/**
* @brief Decoded and installs the response certificate from the certificate fetch.
@@ -196,7 +196,7 @@
* @param state, the requester state for the request.
*/
static void
- endSession(RequestContext& state);
+ endSession(RequestState& state);
private:
static void
diff --git a/tests/unit-tests/bench.t.cpp b/tests/unit-tests/bench.t.cpp
index a36f16f..9589f6e 100644
--- a/tests/unit-tests/bench.t.cpp
+++ b/tests/unit-tests/bench.t.cpp
@@ -103,7 +103,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto newInterest = requester::Requester::genNewInterest(state, Name("/ndn/alice"),
time::system_clock::now(),
time::system_clock::now() + time::days(1));
diff --git a/tests/unit-tests/ca-module.t.cpp b/tests/unit-tests/ca-module.t.cpp
index 97727c0..64dcfa4 100644
--- a/tests/unit-tests/ca-module.t.cpp
+++ b/tests/unit-tests/ca-module.t.cpp
@@ -231,7 +231,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto interest = requester::Requester::genNewInterest(state, Name("/ndn/zhiyi"),
time::system_clock::now(),
time::system_clock::now() + time::days(1));
@@ -282,7 +282,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto current_tp = time::system_clock::now();
auto interest1 = requester::Requester::genNewInterest(state, Name("/ndn/zhiyi"), current_tp, current_tp - time::hours(1));
auto interest2 = requester::Requester::genNewInterest(state, Name("/ndn/zhiyi"), current_tp, current_tp + time::days(361));
@@ -313,7 +313,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto interest1 = requester::Requester::genNewInterest(state, Name("/ndn/a"), time::system_clock::now(),
time::system_clock::now() + time::days(1));
@@ -354,7 +354,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto current_tp = time::system_clock::now();
auto interest1 = requester::Requester::genNewInterest(state, Name("/ndn"), current_tp, current_tp + time::days(1));
@@ -385,7 +385,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto newInterest = requester::Requester::genNewInterest(state, Name("/ndn/zhiyi"), time::system_clock::now(),
time::system_clock::now() + time::days(1));
@@ -475,7 +475,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::REVOKE);
+ requester::RequestState state(m_keyChain, item, RequestType::REVOKE);
auto interest = requester::Requester::genRevokeInterest(state, issuedCert);
@@ -538,7 +538,7 @@
CaProfile item;
item.m_caPrefix = Name("/ndn");
item.m_cert = std::make_shared<security::Certificate>(cert);
- requester::RequestContext state(m_keyChain, item, RequestType::NEW);
+ requester::RequestState state(m_keyChain, item, RequestType::NEW);
auto interest = requester::Requester::genRevokeInterest(state, clientCert);
diff --git a/tests/unit-tests/protocol-encoders.t.cpp b/tests/unit-tests/protocol-encoders.t.cpp
index f2411f1..c43bf84 100644
--- a/tests/unit-tests/protocol-encoders.t.cpp
+++ b/tests/unit-tests/protocol-encoders.t.cpp
@@ -156,7 +156,7 @@
std::move(aesKey), 0);
auto contentBlock = ChallengeEncoder::encodeDataContent(state, Name("/ndn/ucla/a/b/c"));
- requester::RequestContext context(m_keyChain, caCache.m_caItems.front(), RequestType::NEW);
+ requester::RequestState context(m_keyChain, caCache.m_caItems.front(), RequestType::NEW);
context.m_requestId = id;
std::memcpy(context.m_aesKey.data(), key, sizeof(key));
ChallengeEncoder::decodeDataContent(contentBlock, context);
diff --git a/tests/unit-tests/requester.t.cpp b/tests/unit-tests/requester.t.cpp
index ffb70a1..588c4c8 100644
--- a/tests/unit-tests/requester.t.cpp
+++ b/tests/unit-tests/requester.t.cpp
@@ -109,7 +109,7 @@
CaProfile item;
item.m_caPrefix = Name("/site");
item.m_cert = std::make_shared<security::Certificate>(cert);
- RequestContext state(m_keyChain, item, RequestType::NEW);
+ RequestState state(m_keyChain, item, RequestType::NEW);
Data errorPacket;
errorPacket.setName(Name("/site/pretend/this/is/error/packet"));
diff --git a/tools/ndncert-client.cpp b/tools/ndncert-client.cpp
index f1b99dd..3a9a2b6 100644
--- a/tools/ndncert-client.cpp
+++ b/tools/ndncert-client.cpp
@@ -43,7 +43,7 @@
size_t nStep = 1;
Face face;
security::KeyChain keyChain;
-shared_ptr<RequestContext> requesterState = nullptr;
+shared_ptr<RequestState> requesterState = nullptr;
static void
captureParams(std::vector<std::tuple<std::string, std::string>>& requirement)
@@ -394,7 +394,7 @@
int validityPeriod = captureValidityPeriod();
auto now = time::system_clock::now();
std::cerr << "The validity period of your certificate will be: " << validityPeriod << " hours" << std::endl;
- requesterState =std::make_shared<RequestContext>(keyChain, profile, RequestType::NEW);
+ requesterState =std::make_shared<RequestState>(keyChain, profile, RequestType::NEW);
auto interest = Requester::genNewInterest(*requesterState, identityName, now, now + time::hours(validityPeriod));
if (interest != nullptr) {
face.expressInterest(*interest, bind(&newCb, _2), bind(&onNackCb), bind(&timeoutCb));