mgmt: improved control response identification and bug fixes
tests/management: extended unit tests for new response types
Change-Id: Iebed4b3172ff6a07b71021a0ffa4870013d86f48
refs: #1138
diff --git a/daemon/mgmt/fib-manager.cpp b/daemon/mgmt/fib-manager.cpp
index 569a3be..5885e48 100644
--- a/daemon/mgmt/fib-manager.cpp
+++ b/daemon/mgmt/fib-manager.cpp
@@ -11,58 +11,63 @@
#include "mgmt/internal-face.hpp"
#include "mgmt/app-face.hpp"
+
+
+#include <ndn-cpp-dev/management/fib-management-options.hpp>
+#include <ndn-cpp-dev/encoding/tlv.hpp>
+
#include <ndn-cpp-dev/management/fib-management-options.hpp>
namespace nfd {
NFD_LOG_INIT("FibManager");
-const Name FibManager::FIB_MANAGER_REQUEST_PREFIX = "/localhost/nfd/fib";
+const Name FibManager::FIB_MANAGER_COMMAND_PREFIX = "/localhost/nfd/fib";
-// In the future this should be 3 (Signature TLV, Signature Info TLV, Timestamp)
-const size_t FibManager::FIB_MANAGER_REQUEST_SIGNED_INTEREST_NCOMPS = 0;
-
-const size_t FibManager::FIB_MANAGER_REQUEST_COMMAND_MIN_NCOMPS =
- FibManager::FIB_MANAGER_REQUEST_PREFIX.size() +
+const size_t FibManager::FIB_MANAGER_COMMAND_UNSIGNED_NCOMPS =
+ FibManager::FIB_MANAGER_COMMAND_PREFIX.size() +
1 + // verb
- 1 + // verb options
- FibManager::FIB_MANAGER_REQUEST_SIGNED_INTEREST_NCOMPS;
+ 1; // verb options
-const Name::Component FibManager::FIB_MANAGER_REQUEST_VERB_INSERT = "insert";
-const Name::Component FibManager::FIB_MANAGER_REQUEST_VERB_DELETE = "delete";
-const Name::Component FibManager::FIB_MANAGER_REQUEST_VERB_ADD_NEXTHOP = "add-nexthop";
-const Name::Component FibManager::FIB_MANAGER_REQUEST_VERB_REMOVE_NEXTHOP = "remove-nexthop";
-const Name::Component FibManager::FIB_MANAGER_REQUEST_VERB_STRATEGY = "strategy";
+const size_t FibManager::FIB_MANAGER_COMMAND_SIGNED_NCOMPS =
+ FibManager::FIB_MANAGER_COMMAND_UNSIGNED_NCOMPS +
+ 0; // No signed Interest support in mock, otherwise 3 (timestamp, signed info tlv, signature tlv)
+
+const Name::Component FibManager::FIB_MANAGER_COMMAND_VERB_INSERT = "insert";
+const Name::Component FibManager::FIB_MANAGER_COMMAND_VERB_DELETE = "delete";
+const Name::Component FibManager::FIB_MANAGER_COMMAND_VERB_ADD_NEXTHOP = "add-nexthop";
+const Name::Component FibManager::FIB_MANAGER_COMMAND_VERB_REMOVE_NEXTHOP = "remove-nexthop";
+const Name::Component FibManager::FIB_MANAGER_COMMAND_VERB_STRATEGY = "strategy";
-const FibManager::VerbAndProcessor FibManager::FIB_MANAGER_REQUEST_VERBS[] =
+const FibManager::VerbAndProcessor FibManager::FIB_MANAGER_COMMAND_VERBS[] =
{
// Unsupported
// VerbAndProcessor(
- // FibManager::FIB_MANAGER_REQUEST_VERB_INSERT,
+ // FibManager::FIB_MANAGER_COMMAND_VERB_INSERT,
// &FibManager::fibInsert
// ),
// VerbAndProcessor(
- // FibManager::FIB_MANAGER_REQUEST_VERB_DELETE,
+ // FibManager::FIB_MANAGER_COMMAND_VERB_DELETE,
// &FibManager::fibDelete
// ),
VerbAndProcessor(
- FibManager::FIB_MANAGER_REQUEST_VERB_ADD_NEXTHOP,
+ FibManager::FIB_MANAGER_COMMAND_VERB_ADD_NEXTHOP,
&FibManager::fibAddNextHop
),
// Unsupported
// VerbAndProcessor(
- // FibManager::FIB_MANAGER_REQUEST_VERB_REMOVE_NEXTHOP,
+ // FibManager::FIB_MANAGER_COMMAND_VERB_REMOVE_NEXTHOP,
// &FibManager::fibRemoveNextHop
// ),
// VerbAndProcessor(
- // FibManager::FIB_MANAGER_REQUEST_VERB_STRATEGY,
+ // FibManager::FIB_MANAGER_COMMAND_VERB_STRATEGY,
// &FibManager::fibStrategy
// )
@@ -74,41 +79,51 @@
: ManagerBase(face),
m_managedFib(fib),
m_getFace(getFace),
- m_verbDispatch(FIB_MANAGER_REQUEST_VERBS,
- FIB_MANAGER_REQUEST_VERBS +
- (sizeof(FIB_MANAGER_REQUEST_VERBS) / sizeof(VerbAndProcessor)))
+ m_verbDispatch(FIB_MANAGER_COMMAND_VERBS,
+ FIB_MANAGER_COMMAND_VERBS +
+ (sizeof(FIB_MANAGER_COMMAND_VERBS) / sizeof(VerbAndProcessor)))
{
-
+ face->setInterestFilter("/localhost/nfd/fib",
+ bind(&FibManager::onFibRequest, this, _2));
}
void
FibManager::onFibRequest(const Interest& request)
{
- const Name& requestCommand = request.getName();
+ const Name& command = request.getName();
+ const size_t commandNComps = command.size();
/// \todo Separate out response status codes 400 and 401
- if (requestCommand.size() < FIB_MANAGER_REQUEST_COMMAND_MIN_NCOMPS ||
- !FIB_MANAGER_REQUEST_PREFIX.isPrefixOf(requestCommand))
+ if (FIB_MANAGER_COMMAND_UNSIGNED_NCOMPS <= commandNComps &&
+ commandNComps < FIB_MANAGER_COMMAND_SIGNED_NCOMPS)
{
- NFD_LOG_INFO("Malformed command: " << requestCommand);
- sendResponse(request.getName(), 404, "MALFORMED");
+ NFD_LOG_INFO("Unsigned command: " << command);
+ sendResponse(command, 401, "SIGREQ");
+
+ return;
+ }
+ else if (commandNComps < FIB_MANAGER_COMMAND_SIGNED_NCOMPS ||
+ !FIB_MANAGER_COMMAND_PREFIX.isPrefixOf(command))
+ {
+ NFD_LOG_INFO("Malformed command: " << command);
+ sendResponse(command, 400, "MALFORMED");
return;
}
- const Name::Component& requestVerb = requestCommand.get(FIB_MANAGER_REQUEST_PREFIX.size());
- VerbDispatchTable::const_iterator verbProcessor = m_verbDispatch.find (requestVerb);
- if (verbProcessor == m_verbDispatch.end())
+ const Name::Component& verb = command.get(FIB_MANAGER_COMMAND_PREFIX.size());
+
+ VerbDispatchTable::const_iterator verbProcessor = m_verbDispatch.find (verb);
+ if (verbProcessor != m_verbDispatch.end())
{
- NFD_LOG_INFO("Unsupported command verb: " << requestVerb);
- sendResponse(request.getName(), 404, "UNSUPPORTED");
-
+ NFD_LOG_INFO("Processing command verb: " << verb);
+ (verbProcessor->second)(this, request);
}
else
{
- NFD_LOG_INFO("Processing command verb: " << requestVerb);
- (verbProcessor->second)(this, request);
+ NFD_LOG_INFO("Unsupported command verb: " << verb);
+ sendResponse(request.getName(), 404, "UNSUPPORTED");
}
}
@@ -128,23 +143,32 @@
void
FibManager::fibAddNextHop(const Interest& request)
{
+ const Name& command = request.getName();
ndn::FibManagementOptions options;
const size_t optionCompIndex =
- FIB_MANAGER_REQUEST_PREFIX.size() + 1;
+ FIB_MANAGER_COMMAND_PREFIX.size() + 1;
const ndn::Buffer& optionBuffer =
request.getName()[optionCompIndex].getValue();
- shared_ptr<const ndn::Buffer> tmpOptionBuffer(new ndn::Buffer(optionBuffer));
- Block rawOptions(tmpOptionBuffer);
+ shared_ptr<const ndn::Buffer> tmpOptionBuffer(make_shared<ndn::Buffer>(optionBuffer));
- options.wireDecode(rawOptions);
+ try
+ {
+ Block rawOptions(tmpOptionBuffer);
+ options.wireDecode(rawOptions);
+ }
+ catch (const ndn::Tlv::Error& e)
+ {
+ NFD_LOG_INFO("Bad command option parse: " << command);
+ sendResponse(request.getName(), 400, "MALFORMED");
+ return;
+ }
/// \todo authorize command
if (false)
{
- NFD_LOG_INFO("Unauthorized command attempt");
-
- sendResponse(request.getName(), 403, "");
+ NFD_LOG_INFO("Unauthorized command attempt: " << command);
+ sendResponse(request.getName(), 403, "UNAUTHORIZED");
return;
}
@@ -153,7 +177,7 @@
<< " Cost: " << options.getCost());
shared_ptr<Face> nextHopFace = m_getFace(options.getFaceId());
- if (nextHopFace)
+ if (nextHopFace != 0)
{
std::pair<shared_ptr<fib::Entry>, bool> insertResult = m_managedFib.insert(options.getName());
insertResult.first->addNextHop(nextHopFace, options.getCost());
@@ -161,7 +185,8 @@
}
else
{
- sendResponse(request.getName(), 400, "INCORRECT");
+ NFD_LOG_INFO("Unknown FaceId: " << command);
+ sendResponse(request.getName(), 400, "MALFORMED");
}
}
diff --git a/daemon/mgmt/fib-manager.hpp b/daemon/mgmt/fib-manager.hpp
index a0de2fc..60b2385 100644
--- a/daemon/mgmt/fib-manager.hpp
+++ b/daemon/mgmt/fib-manager.hpp
@@ -67,17 +67,23 @@
const VerbDispatchTable m_verbDispatch;
- static const Name FIB_MANAGER_REQUEST_PREFIX;
- static const size_t FIB_MANAGER_REQUEST_COMMAND_MIN_NCOMPS;
- static const size_t FIB_MANAGER_REQUEST_SIGNED_INTEREST_NCOMPS;
+ static const Name FIB_MANAGER_COMMAND_PREFIX; // /localhost/nfd/fib
- static const Name::Component FIB_MANAGER_REQUEST_VERB_INSERT;
- static const Name::Component FIB_MANAGER_REQUEST_VERB_DELETE;
- static const Name::Component FIB_MANAGER_REQUEST_VERB_ADD_NEXTHOP;
- static const Name::Component FIB_MANAGER_REQUEST_VERB_REMOVE_NEXTHOP;
- static const Name::Component FIB_MANAGER_REQUEST_VERB_STRATEGY;
+ // number of components in an invalid, but not malformed, unsigned command.
+ // (/localhost/nfd/fib + verb + options) = 5
+ static const size_t FIB_MANAGER_COMMAND_UNSIGNED_NCOMPS;
- static const VerbAndProcessor FIB_MANAGER_REQUEST_VERBS[];
+ // number of components in a valid signed Interest.
+ // 5 in mock (see UNSIGNED_NCOMPS), 8 with signed Interest support.
+ static const size_t FIB_MANAGER_COMMAND_SIGNED_NCOMPS;
+
+ static const Name::Component FIB_MANAGER_COMMAND_VERB_INSERT;
+ static const Name::Component FIB_MANAGER_COMMAND_VERB_DELETE;
+ static const Name::Component FIB_MANAGER_COMMAND_VERB_ADD_NEXTHOP;
+ static const Name::Component FIB_MANAGER_COMMAND_VERB_REMOVE_NEXTHOP;
+ static const Name::Component FIB_MANAGER_COMMAND_VERB_STRATEGY;
+
+ static const VerbAndProcessor FIB_MANAGER_COMMAND_VERBS[];
};
diff --git a/daemon/mgmt/manager-base.hpp b/daemon/mgmt/manager-base.hpp
index 78ee38e..fd3bcef 100644
--- a/daemon/mgmt/manager-base.hpp
+++ b/daemon/mgmt/manager-base.hpp
@@ -35,7 +35,7 @@
};
-} // namespace ndf
+} // namespace nfd
#endif // NFD_MGMT_MANAGER_BASE_HPP
diff --git a/tests/mgmt/fib-manager.cpp b/tests/mgmt/fib-manager.cpp
index 4e4f587..fec1f63 100644
--- a/tests/mgmt/fib-manager.cpp
+++ b/tests/mgmt/fib-manager.cpp
@@ -30,11 +30,12 @@
shared_ptr<Face>
getFace(FaceId id)
{
- if (m_faces.size() < id)
+ if (id > 0 && id <= m_faces.size())
{
- BOOST_FAIL("Attempted to access invalid FaceId: " << id);
+ return m_faces[id-1];
}
- return m_faces[id-1];
+ NFD_LOG_DEBUG("No face found returning NULL");
+ return shared_ptr<DummyFace>();
}
void
@@ -69,10 +70,30 @@
BOOST_REQUIRE(control.getText() == expectedText);
}
-BOOST_AUTO_TEST_CASE(MalformedCommmand)
+bool
+foundNextHop(FaceId id, uint32_t cost, const fib::NextHop& next)
+{
+ return id == next.getFace()->getId() && next.getCost() == cost;
+}
+
+bool
+addedNextHopWithCost(const Fib& fib, const Name& prefix, size_t oldSize, uint32_t cost)
+{
+ shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch(prefix);
+
+ if (entry)
+ {
+ const fib::NextHopList& hops = entry->getNextHops();
+ return hops.size() == oldSize + 1 &&
+ std::find_if(hops.begin(), hops.end(), bind(&foundNextHop, -1, cost, _1)) != hops.end();
+ }
+ return false;
+}
+
+BOOST_AUTO_TEST_CASE(TestFireInterestFilter)
{
FibManagerFixture fixture;
- shared_ptr<InternalFace> face(new InternalFace);
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
Fib fib;
FibManager manager(fib,
bind(&FibManagerFixture::getFace,
@@ -80,7 +101,24 @@
face);
face->onReceiveData +=
- bind(&validateControlResponse, _1, 404, "MALFORMED");
+ bind(&validateControlResponse, _1, 400, "MALFORMED");
+
+ Interest command("/localhost/nfd/fib");
+ face->sendInterest(command);
+}
+
+BOOST_AUTO_TEST_CASE(MalformedCommmand)
+{
+ FibManagerFixture fixture;
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
+ Fib fib;
+ FibManager manager(fib,
+ bind(&FibManagerFixture::getFace,
+ &fixture, _1),
+ face);
+
+ face->onReceiveData +=
+ bind(&validateControlResponse, _1, 400, "MALFORMED");
Interest command("/localhost/nfd/fib");
manager.onFibRequest(command);
@@ -89,12 +127,13 @@
BOOST_AUTO_TEST_CASE(UnsupportedVerb)
{
FibManagerFixture fixture;
- shared_ptr<InternalFace> face(new InternalFace);
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
Fib fib;
FibManager manager(fib,
bind(&FibManagerFixture::getFace,
&fixture, _1),
face);
+
face->onReceiveData +=
bind(&validateControlResponse, _1, 404, "UNSUPPORTED");
@@ -113,27 +152,21 @@
manager.onFibRequest(command);
}
-bool
-foundNextHop(FaceId id, uint32_t cost, const fib::NextHop& next)
-{
- return id == next.getFace()->getId() && next.getCost() == cost;
-}
-
-BOOST_AUTO_TEST_CASE(AddNextHopVerbInitialAdd)
+BOOST_AUTO_TEST_CASE(UnsignedCommand)
{
FibManagerFixture fixture;
fixture.addFace(make_shared<DummyFace>());
- shared_ptr<InternalFace> face(new InternalFace);
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
Fib fib;
FibManager manager(fib,
bind(&FibManagerFixture::getFace,
&fixture, _1),
- face);
+ face);
face->onReceiveData +=
bind(&validateControlResponse, _1, 200, "OK");
-
-
+ /// \todo enable once sig checking implement
+ // bind(&validateControlResponse, _1, 401, "SIGREG");
ndn::FibManagementOptions options;
options.setName("/hello");
@@ -149,21 +182,126 @@
Interest command(commandName);
manager.onFibRequest(command);
- shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch("/hello");
+ BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
+}
- if (entry)
- {
- const fib::NextHopList& hops = entry->getNextHops();
- NFD_LOG_DEBUG("FaceId: " << hops[0].getFace()->getId());
- BOOST_REQUIRE(hops.size() == 1);
- BOOST_REQUIRE(std::find_if(hops.begin(),
- hops.end(),
- bind(&foundNextHop, -1, 101, _1)) != hops.end());
- }
- else
- {
- BOOST_FAIL("Failed to find expected fib entry");
- }
+BOOST_AUTO_TEST_CASE(UnauthorizedCommand)
+{
+ FibManagerFixture fixture;
+ fixture.addFace(make_shared<DummyFace>());
+
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
+ Fib fib;
+ FibManager manager(fib,
+ bind(&FibManagerFixture::getFace,
+ &fixture, _1),
+ face);
+ face->onReceiveData +=
+ bind(&validateControlResponse, _1, 200, "OK");
+ /// \todo enable once sig checking implement
+ // bind(&validateControlResponse, _1, 403, "UNAUTHORIZED");
+
+ ndn::FibManagementOptions options;
+ options.setName("/hello");
+ options.setFaceId(1);
+ options.setCost(101);
+
+ Block encodedOptions(options.wireEncode());
+
+ Name commandName("/localhost/nfd/fib");
+ commandName.append("add-nexthop");
+ commandName.append(encodedOptions);
+
+ Interest command(commandName);
+ manager.onFibRequest(command);
+
+ BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
+}
+
+BOOST_AUTO_TEST_CASE(BadOptionParse)
+{
+ FibManagerFixture fixture;
+ fixture.addFace(make_shared<DummyFace>());
+
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
+ Fib fib;
+ FibManager manager(fib,
+ bind(&FibManagerFixture::getFace,
+ &fixture, _1),
+ face);
+
+ face->onReceiveData +=
+ bind(&validateControlResponse, _1, 400, "MALFORMED");
+
+ Name commandName("/localhost/nfd/fib");
+ commandName.append("add-nexthop");
+ commandName.append("NotReallyOptions");
+
+ Interest command(commandName);
+ manager.onFibRequest(command);
+}
+
+BOOST_AUTO_TEST_CASE(UnknownFaceId)
+{
+ FibManagerFixture fixture;
+ fixture.addFace(make_shared<DummyFace>());
+
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
+ Fib fib;
+ FibManager manager(fib,
+ bind(&FibManagerFixture::getFace,
+ &fixture, _1),
+ face);
+
+ face->onReceiveData +=
+ bind(&validateControlResponse, _1, 400, "MALFORMED");
+
+ ndn::FibManagementOptions options;
+ options.setName("/hello");
+ options.setFaceId(1000);
+ options.setCost(101);
+
+ Block encodedOptions(options.wireEncode());
+
+ Name commandName("/localhost/nfd/fib");
+ commandName.append("add-nexthop");
+ commandName.append(encodedOptions);
+
+ Interest command(commandName);
+ manager.onFibRequest(command);
+
+ BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101) == false);
+}
+
+BOOST_AUTO_TEST_CASE(AddNextHopVerbInitialAdd)
+{
+ FibManagerFixture fixture;
+ fixture.addFace(make_shared<DummyFace>());
+
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
+ Fib fib;
+ FibManager manager(fib,
+ bind(&FibManagerFixture::getFace,
+ &fixture, _1),
+ face);
+ face->onReceiveData +=
+ bind(&validateControlResponse, _1, 200, "OK");
+
+ ndn::FibManagementOptions options;
+ options.setName("/hello");
+ options.setFaceId(1);
+ options.setCost(101);
+
+ Block encodedOptions(options.wireEncode());
+
+ Name commandName("/localhost/nfd/fib");
+ commandName.append("add-nexthop");
+ commandName.append(encodedOptions);
+
+ Interest command(commandName);
+ manager.onFibRequest(command);
+
+ BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
}
BOOST_AUTO_TEST_CASE(AddNextHopVerbAddToExisting)
@@ -172,7 +310,7 @@
fixture.addFace(make_shared<DummyFace>());
fixture.addFace(make_shared<DummyFace>());
- shared_ptr<InternalFace> face(new InternalFace);
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
Fib fib;
FibManager manager(fib,
bind(&FibManagerFixture::getFace,
@@ -208,11 +346,7 @@
const fib::NextHopList& hops = entry->getNextHops();
for (int j = 1; j <= i; j++)
{
- BOOST_REQUIRE(hops.size() == i);
- // BOOST_REQUIRE(hops[j-1].getCost() == j);
- BOOST_REQUIRE(std::find_if(hops.begin(),
- hops.end(),
- bind(&foundNextHop, -1, 100 + j, _1)) != hops.end());
+ BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", i - 1, 100 + j));
}
}
else
@@ -227,7 +361,7 @@
FibManagerFixture fixture;
fixture.addFace(make_shared<DummyFace>());
- shared_ptr<InternalFace> face(new InternalFace);
+ shared_ptr<InternalFace> face(make_shared<InternalFace>());
Fib fib;
FibManager manager(fib,
bind(&FibManagerFixture::getFace,
diff --git a/tests/mgmt/internal-face.cpp b/tests/mgmt/internal-face.cpp
index 2fce7f1..1c6f70d 100644
--- a/tests/mgmt/internal-face.cpp
+++ b/tests/mgmt/internal-face.cpp
@@ -76,10 +76,6 @@
&fixture, _1),
face);
- face->setInterestFilter("/localhost/nfd/fib",
- bind(&FibManager::onFibRequest,
- &manager, _2));
-
face->onReceiveData +=
bind(&validateControlResponse, _1, 200, "OK");
@@ -123,10 +119,6 @@
&fixture, _1),
face);
- face->setInterestFilter("/localhost/nfd/fib",
- bind(&FibManager::onFibRequest,
- &manager, _2));
-
face->onReceiveData +=
bind(&validateControlResponse, _1, 404, "MALFORMED");