Fix exception with stoul for droppruning

When src/dst prefix lengths were unset, fields contained an empty space, which was inadvertently added while parsing for mask, which caused an exception in opflex_agent Signed-off-by: Kiran Shastri <[email protected]> (cherry picked from commit cf54871)
noironetworks · Aug 2, 2024 · 29164e8 · 29164e8
1 parent 466a7c1
commit 29164e8
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 14 deletions.
diff --git a/.travis/travis-build.sh b/.travis/travis-build.sh
@@ -37,7 +37,8 @@ popd
 pushd agent-ovs
 export LD_LIBRARY_PATH=/usr/local/lib
 ./autogen.sh &> /dev/null
-./configure --enable-coverage --enable-gprof --enable-grpc &> /dev/null
+# --enable-grpc
+./configure --enable-coverage --enable-gprof &> /dev/null
 make -j2
 sudo make install
 set +e

diff --git a/agent-ovs/lib/FSPacketDropLogConfigSource.cpp b/agent-ovs/lib/FSPacketDropLogConfigSource.cpp
@@ -130,34 +130,49 @@ static int validateMacMask(const std::string& arg,
     }
     return 0;
 }
-static int validatePrefix(int filter_idx, const std::string & arg, 
-        optional<boost::asio::ip::address> &addr, optional<uint8_t> &pfxLen) {
+static int validatePrefix(int filter_idx, const std::string & arg,
+       optional<boost::asio::ip::address> &addr, optional<uint8_t> &pfxLen) {
     using namespace boost;
     int ret=0;
     if(arg.empty()) {
         return ret;
     }
-    std::string address_string = arg;
+    string address_string = arg;
+    string pfxLenArg;
+    bool maskPresent = false;
     if(arg.find('/') != std::string::npos) {
         std::vector<std::string> splitVec;
         split(splitVec,arg, is_any_of("/"), token_compress_on);
         address_string = splitVec[0];
-        pfxLen = strtol(splitVec[1].c_str(),NULL,10);
+        pfxLenArg = splitVec[1];
+        maskPresent = true;
     }
     boost::system::error_code ec; 
     addr = asio::ip::address::from_string(address_string, ec);
     if (ec) {
         LOG(ERROR) << "Invalid address for filter " << filter_idx;
         return -1;
     }
-    if(pfxLen) {
+    if(!pfxLenArg.empty()) {
+        LOG(INFO) << "PrefixLen is set!";
+        size_t *end = nullptr;
+        uint8_t pfxLenVal;
+        try {
+            pfxLenVal = stoul(pfxLenArg, end, 10);
+        } catch (std::exception const& ex) {
+            LOG(ERROR) << "Incorrect prefix length for filter" << filter_idx;
+            return -1;
+        }
         if(addr.get().is_v4()) {
-            ret = (pfxLen.get()>32 || pfxLen.get()==0)?-1:0;
+            ret = (pfxLenVal>32)?-1:0;
         } else {
-            ret = (pfxLen.get()>128 || pfxLen.get()==0)?-1:0;
+            ret = (pfxLenVal>128)?-1:0;
+        }
+        if (ret == 0) {
+            pfxLen = pfxLenVal;
         } 
-    } else {
-        pfxLen = (addr.get().is_v4()?32:128);
+    } else if(maskPresent) {
+        ret = -1;
     }
     if(ret != 0) {
         LOG(ERROR) << "Incorrect prefix length for filter" << filter_idx;

diff --git a/agent-ovs/lib/test/ExtraConfigManager_test.cpp b/agent-ovs/lib/test/ExtraConfigManager_test.cpp
@@ -135,7 +135,6 @@ BOOST_FIXTURE_TEST_CASE( droplogpruneconfigsource, FSConfigFixture ) {
     boost::optional<shared_ptr<DropPruneConfig>> dropPruneCfg = DropPruneConfig::resolve(agent.getFramework(), dropPruneUri);
     BOOST_CHECK(dropPruneCfg.get()->getFilterName().get() == "flt1");
     BOOST_CHECK(dropPruneCfg.get()->getSrcAddress().get() == "1.2.3.4");
-    BOOST_CHECK(dropPruneCfg.get()->getSrcPrefixLen().get() == 32);
     BOOST_CHECK(dropPruneCfg.get()->getDstAddress().get() == "5.6.7.0");
     BOOST_CHECK(dropPruneCfg.get()->getDstPrefixLen().get() == 24);
     BOOST_CHECK(dropPruneCfg.get()->getSrcMac().get().toString() == "00:01:02:03:04:05");

diff --git a/agent-ovs/ovs/OVSRenderer.cpp b/agent-ovs/ovs/OVSRenderer.cpp
@@ -617,15 +617,15 @@ static void convertPruneFilter(std::shared_ptr<PacketDropLogPruneSpec> &sourceSp
     }
     if(sourceSpec->srcPfxLen) {
         std::stringstream strSrcPfxLen;
-        strSrcPfxLen << sourceSpec->srcPfxLen.get();
+        strSrcPfxLen << (uint8_t)sourceSpec->srcPfxLen.get();
         filter->setField(TFLD_SPFX_LEN,strSrcPfxLen.str());
     }
     if(sourceSpec->dstIp) {
         filter->setField(TFLD_DST_IP,sourceSpec->dstIp.get().to_string());
     }
     if(sourceSpec->dstPfxLen) {
         std::stringstream strDstPfxLen;
-        strDstPfxLen << sourceSpec->dstPfxLen.get();
+        strDstPfxLen << (uint8_t)sourceSpec->dstPfxLen.get();
         filter->setField(TFLD_DPFX_LEN,strDstPfxLen.str());
     }
     if(sourceSpec->srcMac) {

diff --git a/agent-ovs/ovs/include/PacketLogHandler.h b/agent-ovs/ovs/include/PacketLogHandler.h
@@ -233,7 +233,11 @@ class PacketFilterSpec: public PacketTuple {
             pfxLen = 128;
         }
         if(!prefixLen.empty()) {
-            pfxLen = stoul(prefixLen);
+            try {
+                pfxLen = stoul(prefixLen);
+            } catch(std::exception const& ex) {
+                /*Should not get here as value is already vetted*/
+            }
         }
         if((pfxLen > 128) || (pfxLen == 0))
            return false;