Honor DatagramPacket offsets and update received packet length

src/main/cpp/aprdatagramsocket.cpp

@@ -97,13 +97,20 @@ void APRDatagramSocket::receive(DatagramPacketPtr& p)
 
 	// receive the datagram packet
 	apr_size_t len = p->getLength();
+	auto data = static_cast<char*>(p->getData());
+	if (data != nullptr)
+	{
+		data += p->getOffset();
+	}
 	status = apr_socket_recvfrom(addr, _priv->socket, 0,
-			(char*)p->getData(), &len);
+			data, &len);
 
 	if (status != APR_SUCCESS)
 	{
 		throw IOException(status);
 	}
+
+	p->setLength(static_cast<int>(len));
 }
 
 void APRDatagramSocket::send(DatagramPacketPtr& p)
@@ -124,8 +131,13 @@ void APRDatagramSocket::send(DatagramPacketPtr& p)
 
 	// send the datagram packet
 	apr_size_t len = p->getLength();
+	auto data = static_cast<char*>(p->getData());
+	if (data != nullptr)
+	{
+		data += p->getOffset();
+	}
 	status = apr_socket_sendto(_priv->socket, addr, 0,
-			(char*)p->getData(), &len);
+			data, &len);
 
 	if (status != APR_SUCCESS)
 	{

src/main/cpp/datagrampacket.cpp

@@ -17,31 +17,57 @@
 
 #include <log4cxx/logstring.h>
 #include <log4cxx/helpers/datagrampacket.h>
+#include <log4cxx/helpers/exception.h>
 
 using namespace LOG4CXX_NS::helpers;
 
+namespace
+{
+void validateBuffer(void* buf, int offset, int length)
+{
+	if (offset < 0)
+	{
+		throw IllegalArgumentException(LOG4CXX_STR("DatagramPacket offset must be non-negative"));
+	}
+
+	if (length < 0)
+	{
+		throw IllegalArgumentException(LOG4CXX_STR("DatagramPacket length must be non-negative"));
+	}
+
+	if (buf == nullptr && length != 0)
+	{
+		throw NullPointerException(LOG4CXX_STR("DatagramPacket buffer must not be null for a non-empty packet"));
+	}
+}
+}
+
 struct DatagramPacket::DatagramPacketPriv
 {
 	DatagramPacketPriv(void* buf1, int length1)
 		: buf(buf1), offset(0), length(length1), address(), port(0)
 	{
+		validateBuffer(buf, offset, length);
 	}
 
 	DatagramPacketPriv(void* buf1, int length1, InetAddressPtr address1,
 		int port1)
 		: buf(buf1), offset(0), length(length1), address(address1), port(port1)
 	{
+		validateBuffer(buf, offset, length);
 	}
 
 	DatagramPacketPriv(void* buf1, int offset1, int length1)
 		: buf(buf1), offset(offset1), length(length1), address(), port(0)
 	{
+		validateBuffer(buf, offset, length);
 	}
 
 	DatagramPacketPriv(void* buf1, int offset1, int length1,
 		InetAddressPtr address1, int port1)
 		: buf(buf1), offset(offset1), length(length1), address(address1), port(port1)
 	{
+		validateBuffer(buf, offset, length);
 	}
 
 	/** the data for this packet. */
@@ -129,18 +155,21 @@ void DatagramPacket::setAddress(InetAddressPtr address1)
 
 void DatagramPacket::setData(void* buf1)
 {
+	validateBuffer(buf1, m_priv->offset, m_priv->length);
 	m_priv->buf = buf1;
 }
 
 void DatagramPacket::setData(void* buf1, int offset1, int length1)
 {
+	validateBuffer(buf1, offset1, length1);
 	m_priv->buf = buf1;
 	m_priv->offset = offset1;
 	m_priv->length = length1;
 }
 
 void DatagramPacket::setLength(int length1)
 {
+	validateBuffer(m_priv->buf, m_priv->offset, length1);
 	m_priv->length = length1;
 }
 

src/test/cpp/helpers/CMakeLists.txt

@@ -23,6 +23,8 @@ set(HELPER_TESTS
     charsetencodertestcase
     consolewritertestcase
     cyclicbuffertestcase
+    datagrampackettest
+    datagramsockettestcase
     datetimedateformattestcase
     filewatchdogtest
     inetaddresstestcase

src/test/cpp/helpers/datagrampackettest.cpp

@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "../logunit.h"
+#include <log4cxx/helpers/datagrampacket.h>
+#include <log4cxx/helpers/exception.h>
+
+using namespace log4cxx::helpers;
+
+LOGUNIT_CLASS(DatagramPacketTest)
+{
+	LOGUNIT_TEST_SUITE(DatagramPacketTest);
+	LOGUNIT_TEST_EXCEPTION(testConstructorRejectsNegativeLength, IllegalArgumentException);
+	LOGUNIT_TEST_EXCEPTION(testConstructorRejectsNegativeOffset, IllegalArgumentException);
+	LOGUNIT_TEST_EXCEPTION(testConstructorRejectsNullBufferForNonEmptyPacket, NullPointerException);
+	LOGUNIT_TEST_EXCEPTION(testSetDataRejectsNegativeLength, IllegalArgumentException);
+	LOGUNIT_TEST_EXCEPTION(testSetLengthRejectsNegativeLength, IllegalArgumentException);
+	LOGUNIT_TEST(testZeroLengthPacketAllowsNullBuffer);
+	LOGUNIT_TEST_SUITE_END();
+
+public:
+	void testConstructorRejectsNegativeLength()
+	{
+		char buffer[1];
+		DatagramPacket packet(buffer, -1);
+	}
+
+	void testConstructorRejectsNegativeOffset()
+	{
+		char buffer[1];
+		DatagramPacket packet(buffer, -1, 1);
+	}
+
+	void testConstructorRejectsNullBufferForNonEmptyPacket()
+	{
+		DatagramPacket packet(nullptr, 1);
+	}
+
+	void testSetDataRejectsNegativeLength()
+	{
+		char buffer[1];
+		DatagramPacket packet(buffer, 1);
+		packet.setData(buffer, 0, -1);
+	}
+
+	void testSetLengthRejectsNegativeLength()
+	{
+		char buffer[1];
+		DatagramPacket packet(buffer, 1);
+		packet.setLength(-1);
+	}
+
+	void testZeroLengthPacketAllowsNullBuffer()
+	{
+		DatagramPacket packet(nullptr, 0);
+		LOGUNIT_ASSERT_EQUAL(0, packet.getLength());
+	}
+};
+
+LOGUNIT_TEST_SUITE_REGISTRATION(DatagramPacketTest);

src/test/cpp/helpers/datagramsockettestcase.cpp

@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "../logunit.h"
+#include <log4cxx/helpers/datagramsocket.h>
+#include <log4cxx/helpers/datagrampacket.h>
+#include <log4cxx/helpers/inetaddress.h>
+
+using namespace log4cxx;
+using namespace log4cxx::helpers;
+
+LOGUNIT_CLASS(DatagramSocketTestCase)
+{
+	LOGUNIT_TEST_SUITE(DatagramSocketTestCase);
+	LOGUNIT_TEST(testSendHonorsOffset);
+	LOGUNIT_TEST(testReceiveHonorsOffset);
+	LOGUNIT_TEST(testReceiveUpdatesLength);
+	LOGUNIT_TEST_SUITE_END();
+
+public:
+	void testSendHonorsOffset()
+	{
+		int port = 45678;
+		auto receiver = DatagramSocket::create(port);
+		auto sender = DatagramSocket::create();
+		auto addr = InetAddress::getByName(LOG4CXX_STR("127.0.0.1"));
+
+		char buf[] = "ABCDEFGHIJ";
+		// Packet with offset 3, length 5 should transmit "DEFGH"
+		auto packet = std::make_shared<DatagramPacket>(buf, 3, 5, addr, port);
+		sender->send(packet);
+
+		char recvBuf[11] = {0};
+		// Must provide address to avoid crash in receive()
+		auto recvPacket = std::make_shared<DatagramPacket>(recvBuf, 10, addr, port);
+		receiver->receive(recvPacket);
+
+		LOGUNIT_ASSERT_EQUAL(std::string("DEFGH"), std::string(recvBuf, 5));
+	}
+
+	void testReceiveHonorsOffset()
+	{
+		int port = 45679;
+		auto receiver = DatagramSocket::create(port);
+		auto sender = DatagramSocket::create();
+		auto addr = InetAddress::getByName(LOG4CXX_STR("127.0.0.1"));
+
+		char sendBuf[] = "WXYZ";
+		auto sendPacket = std::make_shared<DatagramPacket>(sendBuf, 4, addr, port);
+		sender->send(sendPacket);
+
+		char recvBuf[11] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+		// Packet with offset 2, length 4 should write "WXYZ" starting at index 2
+		auto recvPacket = std::make_shared<DatagramPacket>(recvBuf, 2, 4, addr, port);
+		receiver->receive(recvPacket);
+
+		LOGUNIT_ASSERT_EQUAL((char)0, recvBuf[0]);
+		LOGUNIT_ASSERT_EQUAL((char)0, recvBuf[1]);
+		LOGUNIT_ASSERT_EQUAL('W', recvBuf[2]);
+		LOGUNIT_ASSERT_EQUAL('X', recvBuf[3]);
+		LOGUNIT_ASSERT_EQUAL('Y', recvBuf[4]);
+		LOGUNIT_ASSERT_EQUAL('Z', recvBuf[5]);
+	}
+
+	void testReceiveUpdatesLength()
+	{
+		int port = 45680;
+		auto receiver = DatagramSocket::create(port);
+		auto sender = DatagramSocket::create();
+		auto addr = InetAddress::getByName(LOG4CXX_STR("127.0.0.1"));
+
+		char sendBuf[] = "SHORT";
+		auto sendPacket = std::make_shared<DatagramPacket>(sendBuf, 5, addr, port);
+		sender->send(sendPacket);
+
+		char recvBuf[100];
+		auto recvPacket = std::make_shared<DatagramPacket>(recvBuf, 100, addr, port);
+		receiver->receive(recvPacket);
+
+		LOGUNIT_ASSERT_EQUAL(5, recvPacket->getLength());
+	}
+};
+
+LOGUNIT_TEST_SUITE_REGISTRATION(DatagramSocketTestCase);