Fix inclusive parseUntil condition in the main packet parse loop.

Packet++/src/Packet.cpp

@@ -63,39 +63,95 @@ namespace pcpp
 
 		m_FirstLayer = createFirstLayer(linkType);
 
-		m_LastLayer = m_FirstLayer;
-		Layer* curLayer = m_FirstLayer;
+		// As the stop conditions are inclusive, the parse must go one layer further and then roll back if needed
+		bool rollbackLastLayer = false;
+		bool foundTargetProtocol = false;
+		for (auto* curLayer = m_FirstLayer; curLayer != nullptr; curLayer = curLayer->getNextLayer())
+		{
+			// Mark the current layer as allocated in the packet
+			curLayer->m_IsAllocatedInPacket = true;
+			m_LastLayer = curLayer;  // Update last layer to current layer
+
+			// If the current layer is of a higher OSI layer than the target, stop parsing
+			if (curLayer->getOsiModelLayer() > parseUntilLayer)
+			{
+				rollbackLastLayer = true;
+				break;
+			}
+
+			// If we are searching for a specific layer protocol, record when we find at least one target.
+			const bool matchesTarget = curLayer->isMemberOfProtocolFamily(parseUntil);
+			if (parseUntil != UnknownProtocol && matchesTarget)
+			{
+				foundTargetProtocol = true;
+			}
+
+			// If we have found the target protocol already, we are parsing until we find a different protocol
+			if (foundTargetProtocol && !matchesTarget)
+			{
+				rollbackLastLayer = true;
+				break;
+			}
+
+			// Parse the next layer. This will update the next layer pointer of the current layer.
+			curLayer->parseNextLayer();
+		}
+
+		// Roll back one layer, if parsing with search condition as the conditions are inclusive.
+		// Don't delete the first layer. If already past the target layer, treat the same as if the layer was found.
+		if (rollbackLastLayer && m_LastLayer != m_FirstLayer)
+		{
+			m_LastLayer = m_LastLayer->getPrevLayer();
+			delete m_LastLayer->m_NextLayer;
+			m_LastLayer->m_NextLayer = nullptr;
+		}
+
+		/*
 		while (curLayer != nullptr &&
+		       // Check the parse until condition
 		       (parseUntil == UnknownProtocol || !curLayer->isMemberOfProtocolFamily(parseUntil)) &&
+		       // Check the parse until OSI condition
 		       curLayer->getOsiModelLayer() <= parseUntilLayer)
 		{
-			curLayer->parseNextLayer();
-			curLayer->m_IsAllocatedInPacket = true;
-			curLayer = curLayer->getNextLayer();
-			if (curLayer != nullptr)
-				m_LastLayer = curLayer;
+		    // Parse layer, allocate it
+		    curLayer->parseNextLayer();
+		    curLayer->m_IsAllocatedInPacket = true;
+
+		    // Move to next layer.
+		    curLayer = curLayer->getNextLayer();
+
+		    // If a next layer exists, update last layer.
+		    if (curLayer != nullptr)
+		        m_LastLayer = curLayer;
 		}
+		*/
 
+		/*
+		// The loop ends with curLayer being either nullptr or one past the last parsed layer
 		if (curLayer != nullptr && curLayer->isMemberOfProtocolFamily(parseUntil))
 		{
-			curLayer->m_IsAllocatedInPacket = true;
+		    curLayer->m_IsAllocatedInPacket = true;
 		}
 
+		// If the target OSI layer is exceeded, roll back the last layer (why?)
 		if (curLayer != nullptr && curLayer->getOsiModelLayer() > parseUntilLayer)
 		{
-			// don't delete the first layer. If already past the target layer, treat the same as if the layer was found.
-			if (curLayer == m_FirstLayer)
-			{
-				curLayer->m_IsAllocatedInPacket = true;
-			}
-			else
-			{
-				m_LastLayer = curLayer->getPrevLayer();
-				delete curLayer;
-				m_LastLayer->m_NextLayer = nullptr;
-			}
-		}
-
+		    // don't delete the first layer. If already past the target layer, treat the same as if the layer was found.
+		    if (curLayer == m_FirstLayer)
+		    {
+		        curLayer->m_IsAllocatedInPacket = true;
+		    }
+		    else
+		    {
+		        // Rolls back last layer if it exceeded the target OSI layer
+		        m_LastLayer = curLayer->getPrevLayer();
+		        delete curLayer;
+		        m_LastLayer->m_NextLayer = nullptr;
+		    }
+		}
+		*/
+
+		// If there is data left in the raw packet that doesn't belong to any layer, create a PacketTrailerLayer
 		if (m_LastLayer != nullptr && parseUntil == UnknownProtocol && parseUntilLayer == OsiModelLayerUnknown)
 		{
 			// find if there is data left in the raw packet that doesn't belong to any layer. In that case it's probably

Tests/Packet++Test/TestDefinition.h

@@ -60,6 +60,7 @@ PTF_TEST_CASE(ResizeLayerTest);
 PTF_TEST_CASE(PrintPacketAndLayersTest);
 PTF_TEST_CASE(ProtocolFamilyMembershipTest);
 PTF_TEST_CASE(PacketParseLayerLimitTest);
+PTF_TEST_CASE(PacketParseMultiLayerTest);
 
 // Implemented in HttpTests.cpp
 PTF_TEST_CASE(HttpRequestParseMethodTest);

Tests/Packet++Test/Tests/PacketTests.cpp

@@ -20,6 +20,7 @@
 #include "PayloadLayer.h"
 #include "GeneralUtils.h"
 #include "SystemUtils.h"
+#include "BgpLayer.h"
 
 using pcpp_tests::utils::createPacketFromHexResource;
 
@@ -1064,3 +1065,33 @@ PTF_TEST_CASE(PacketParseLayerLimitTest)
 	pcpp::Packet packet1(rawPacket1.get(), pcpp::OsiModelTransportLayer);
 	PTF_ASSERT_EQUAL(packet1.getLastLayer()->getOsiModelLayer(), pcpp::OsiModelTransportLayer);
 }
+
+PTF_TEST_CASE(PacketParseMultiLayerTest)
+{
+	// The BGP packet has 4 BGP messages inside.
+	auto rawPacket = createPacketFromHexResource("PacketExamples/Bgp_update2.dat");
+
+	// Limit to BGP layer
+	pcpp::Packet packet(rawPacket.get(), pcpp::BGP);
+
+	const size_t expectedNumOfBgpMessages = 4;
+	size_t actualNumOfBgpMessages = 0;
+
+	pcpp::BgpLayer* bgpLayer = packet.getLayerOfType<pcpp::BgpLayer>();
+	if (bgpLayer != nullptr)
+	{
+		++actualNumOfBgpMessages;
+	}
+
+	// The fallback iteration uses expected * 2, just to be sure we won't get into an infinite loop
+	for (; bgpLayer != nullptr && actualNumOfBgpMessages < expectedNumOfBgpMessages * 2;)
+	{
+		bgpLayer = packet.getNextLayerOfType<pcpp::BgpLayer>(bgpLayer);
+		if (bgpLayer != nullptr)
+		{
+			++actualNumOfBgpMessages;
+		}
+	}
+
+	PTF_ASSERT_EQUAL(actualNumOfBgpMessages, expectedNumOfBgpMessages);
+}

Tests/Packet++Test/main.cpp

@@ -169,6 +169,7 @@ int main(int argc, char* argv[])
 	PTF_RUN_TEST(PrintPacketAndLayersTest, "packet;print");
 	PTF_RUN_TEST(ProtocolFamilyMembershipTest, "packet");
 	PTF_RUN_TEST(PacketParseLayerLimitTest, "packet");
+	PTF_RUN_TEST(PacketParseMultiLayerTest, "packet");
 
 	PTF_RUN_TEST(HttpRequestParseMethodTest, "http");
 	PTF_RUN_TEST(HttpRequestLayerParsingTest, "http");

Tests/PcppTestUtilities/Resources.cpp

@@ -51,7 +51,8 @@ namespace pcpp_tests
 			}
 		}  // namespace
 
-		ResourceProvider::ResourceProvider(std::string dataRoot) : m_DataRoot(std::move(dataRoot))
+		ResourceProvider::ResourceProvider(std::string dataRoot, bool frozen)
+		    : m_DataRoot(std::move(dataRoot)), m_Frozen(frozen)
 		{}
 
 		Resource ResourceProvider::loadResource(const char* filename, ResourceType resourceType) const
@@ -101,6 +102,56 @@ namespace pcpp_tests
 				throw std::invalid_argument("Unsupported resource type");
 			}
 		}
+
+		void ResourceProvider::saveResource(ResourceType resourceType, const char* filename, const uint8_t* data,
+		                                    size_t length) const
+		{
+			if (m_Frozen)
+			{
+				throw std::runtime_error("Resource provider is frozen and does not allow saving");
+			}
+
+			if (data == nullptr || length == 0)
+			{
+				throw std::invalid_argument("Data is null or length is zero");
+			}
+
+			std::string fullPath;
+			if (!m_DataRoot.empty())
+			{
+				fullPath = m_DataRoot + getOsPathSeparator() + filename;
+			}
+			else
+			{
+				fullPath = filename;
+			}
+
+			auto const requireOpen = [filename](std::ofstream const& fileStream) {
+				if (!fileStream)
+				{
+					throw std::runtime_error(std::string("Failed to open file: ") + filename);
+				}
+			};
+
+			switch (resourceType)
+			{
+			case ResourceType::HexData:
+			{
+				std::ofstream fileStream(fullPath);
+				requireOpen(fileStream);
+				for (size_t i = 0; i < length; ++i)
+				{
+					fileStream << std::hex;
+					fileStream.width(2);
+					fileStream.fill('0');
+					fileStream << static_cast<int>(data[i]);
+				}
+				break;
+			}
+			default:
+				throw std::invalid_argument("Unsupported resource type");
+			}
+		}
 	}  // namespace utils
 
 	namespace

Tests/PcppTestUtilities/Resources.h

@@ -29,7 +29,8 @@ namespace pcpp_tests
 		public:
 			/// @brief Constructs a ResourceProvider with a specified data root directory.
 			/// @param dataRoot The root directory from which resources will be loaded.
-			explicit ResourceProvider(std::string dataRoot);
+			/// @param frozen If true, the provider is read-only and does not allow saving resources.
+			explicit ResourceProvider(std::string dataRoot, bool frozen = true);
 
 			/// @brief Loads a resource from resource provider.
 			/// @param filename The name of the resource file to load.
@@ -43,8 +44,18 @@ namespace pcpp_tests
 			/// @return A vector containing the loaded data.
 			std::vector<uint8_t> loadResourceToVector(const char* filename, ResourceType resourceType) const;
 
+			/// @brief Saves a resource to the resource provider.
+			/// @param resourceType The type of the resource being saved.
+			/// @param filename The name of the file to save the resource to.
+			/// @param data Pointer to the data to be saved.
+			/// @param length The length of the data in bytes.
+			/// @throw std::runtime_error if the provider is frozen and does not allow saving.
+			void saveResource(ResourceType resourceType, const char* filename, const uint8_t* data,
+			                  size_t length) const;
+
 		private:
 			std::string m_DataRoot;  ///< The root directory for test data files
+			bool m_Frozen = true;    ///< Indicates if the provider is frozen (no modifications allowed)
 		};
 
 	}  // namespace utils