From 330136a89e0b4e7ffc98431a129d54221a4e9340 Mon Sep 17 00:00:00 2001
From: Junxiao Shi <git@mail1.yoursunny.com>
Date: Thu, 10 Mar 2016 04:53:08 -0700
Subject: [PATCH] fw: fix PIT leak from DNL-detected duplicate Nonce

refs #3484

Change-Id: Id5928356a39b2f8988b6d4969c15ac744d283e3f
---
 daemon/fw/forwarder.cpp         | 24 +++++++++++++++---------
 daemon/fw/forwarder.hpp         | 17 ++++++++---------
 tests/daemon/fw/forwarder.t.cpp | 22 +++++++++++++++++++++-
 3 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 280c7133..e8a0ade5 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2016,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -117,16 +117,23 @@ Forwarder::onIncomingInterest(Face& inFace, const Interest& interest)
     return;
   }
 
+  // detect duplicate Nonce with Dead Nonce List
+  bool hasDuplicateNonceInDnl = m_deadNonceList.has(interest.getName(), interest.getNonce());
+  if (hasDuplicateNonceInDnl) {
+    // goto Interest loop pipeline
+    this->onInterestLoop(inFace, interest);
+    return;
+  }
+
   // PIT insert
   shared_ptr<pit::Entry> pitEntry = m_pit.insert(interest).first;
 
-  // detect duplicate Nonce
-  int dnw = pitEntry->findNonce(interest.getNonce(), inFace);
-  bool hasDuplicateNonce = (dnw != pit::DUPLICATE_NONCE_NONE) ||
-                           m_deadNonceList.has(interest.getName(), interest.getNonce());
-  if (hasDuplicateNonce) {
+  // detect duplicate Nonce in PIT entry
+  bool hasDuplicateNonceInPit = pitEntry->findNonce(interest.getNonce(), inFace) !=
+                                pit::DUPLICATE_NONCE_NONE;
+  if (hasDuplicateNonceInPit) {
     // goto Interest loop pipeline
-    this->onInterestLoop(inFace, interest, pitEntry);
+    this->onInterestLoop(inFace, interest);
     return;
   }
 
@@ -147,8 +154,7 @@ Forwarder::onIncomingInterest(Face& inFace, const Interest& interest)
 }
 
 void
-Forwarder::onInterestLoop(Face& inFace, const Interest& interest,
-                          shared_ptr<pit::Entry> pitEntry)
+Forwarder::onInterestLoop(Face& inFace, const Interest& interest)
 {
   // if multi-access face, drop
   if (inFace.getLinkType() == ndn::nfd::LINK_TYPE_MULTI_ACCESS) {
diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp
index 1997427c..2bac6b07 100644
--- a/daemon/fw/forwarder.hpp
+++ b/daemon/fw/forwarder.hpp
@@ -1,12 +1,12 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014,  Regents of the University of California,
- *                      Arizona Board of Regents,
- *                      Colorado State University,
- *                      University Pierre & Marie Curie, Sorbonne University,
- *                      Washington University in St. Louis,
- *                      Beijing Institute of Technology,
- *                      The University of Memphis
+ * Copyright (c) 2014-2016,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
  *
  * This file is part of NFD (Named Data Networking Forwarding Daemon).
  * See AUTHORS.md for complete list of NFD authors and contributors.
@@ -132,8 +132,7 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines
   /** \brief Interest loop pipeline
    */
   VIRTUAL_WITH_TESTS void
-  onInterestLoop(Face& inFace, const Interest& interest,
-                 shared_ptr<pit::Entry> pitEntry);
+  onInterestLoop(Face& inFace, const Interest& interest);
 
   /** \brief Content Store miss pipeline
   */
diff --git a/tests/daemon/fw/forwarder.t.cpp b/tests/daemon/fw/forwarder.t.cpp
index cfa5f71b..83374916 100644
--- a/tests/daemon/fw/forwarder.t.cpp
+++ b/tests/daemon/fw/forwarder.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2016,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -675,6 +675,26 @@ BOOST_FIXTURE_TEST_CASE(InterestLoopWithShortLifetime, UnitTestTimeFixture) // B
   // an Interest if its Name+Nonce has appeared any point in the past.
 }
 
+BOOST_FIXTURE_TEST_CASE(PitLeak, UnitTestTimeFixture) // Bug 3484
+{
+  Forwarder forwarder;
+  shared_ptr<Face> face1 = make_shared<DummyFace>();
+  forwarder.addFace(face1);
+
+  shared_ptr<Interest> interest = makeInterest("ndn:/hcLSAsQ9A");
+  interest->setNonce(61883075);
+  interest->setInterestLifetime(time::seconds(2));
+
+  DeadNonceList& dnl = forwarder.getDeadNonceList();
+  dnl.add(interest->getName(), interest->getNonce());
+  Pit& pit = forwarder.getPit();
+  BOOST_REQUIRE_EQUAL(pit.size(), 0);
+
+  forwarder.startProcessInterest(*face1, *interest);
+  this->advanceClocks(time::milliseconds(100), time::seconds(20));
+  BOOST_CHECK_EQUAL(pit.size(), 0);
+}
+
 BOOST_AUTO_TEST_CASE(LinkDelegation)
 {
   Forwarder forwarder;