Limit max data length based on used memory read/write service

Author: bmalinowsky

src/io/calimero/mgmt/ManagementClientImpl.java

@@ -1201,7 +1201,7 @@ private byte[] readMemoryExt(final Destination dst, final int startAddr, final i
 			KNXDisconnectException, KNXRemoteException, KNXLinkClosedException, InterruptedException {
 		final byte[] send = createAPDU(MemoryExtendedRead,
 				(byte) bytes, (byte) (startAddr >>> 16), (byte) (startAddr >>> 8), (byte) startAddr);
-		final byte[] apdu = sendWait(dst, priority, send, MemoryExtendedReadResponse, 4, 252);
+		final byte[] apdu = sendWait(dst, priority, send, MemoryExtendedReadResponse, 4, 253);
 		final ReturnCode ret = ReturnCode.of(apdu[2] & 0xff);
 		if (ret != ReturnCode.Success)
 			throw new KnxNegativeReturnCodeException(format("read memory from %s 0x%x", dst.getAddress(), startAddr), ret);

src/io/calimero/mgmt/ManagementProceduresImpl.java

@@ -671,11 +671,12 @@ public void writeMemory(final IndividualAddress device, final long startAddress,
 				1, 1, ctrl);
 		}
 
-        // write memory in chunks with a maximum length of asduLength
-		final int asduLength = maxApduLength(d) - (extMemoryServices ? 5 : 3);
-		for (int i = 0; i < data.length; i += asduLength) {
-            int remainingBytes = data.length - i;
-			final byte[] range = Arrays.copyOfRange(data, i, i + Math.min(asduLength, remainingBytes));
+		final int apduLength = maxApduLength(d);
+		// write memory in chunks with a maximum length of dataLength
+		final int dataLength = extMemoryServices ? Math.min(249, apduLength - 5) : Math.min(63, apduLength - 3);
+		for (int i = 0; i < data.length; i += dataLength) {
+			int remainingBytes = data.length - i;
+			final byte[] range = Arrays.copyOfRange(data, i, i + Math.min(dataLength, remainingBytes));
 
 			// on server verification, our mgmt client will already compare the response value
 			mc.writeMemory(d, (int) startAddress + i, range);
@@ -697,18 +698,15 @@ public byte[] readMemory(final IndividualAddress device, final long startAddress
 			throw new KNXIllegalArgumentException("start address is no 32 Bit address");
 		if (bytes < 0)
 			throw new KNXIllegalArgumentException("bytes to read require a positive number");
-		// sanity check, at least emit a warning
-		// if (bytes > 4096)
-		// logger.log(WARNING, "reading over 4K of device memory "
-		// + "(hope you know what you are doing)");
 
 		final Destination d = getOrCreateDestination(device);
 		final boolean extMemoryServices = ((ManagementClientImpl) mc).supportsFeature(d, SupportedServiceGroup.ExtMemory);
 
+		final int apduLength = maxApduLength(d);
+		final int dataLength = extMemoryServices ? Math.min(249, apduLength - 5) : Math.min(63, apduLength - 3);
 		final byte[] read = new byte[bytes];
-		final int asduLength = maxApduLength(d) - (extMemoryServices ? 5 : 3);
-		for (int i = 0; i < read.length; i += asduLength) {
-			final int size = i + asduLength <= read.length ? asduLength : read.length - i;
+		for (int i = 0; i < read.length; i += dataLength) {
+			final int size = i + dataLength <= read.length ? dataLength : read.length - i;
 			final byte[] range = mc.readMemory(d, (int) startAddress + i, size);
 			System.arraycopy(range, 0, read, i, range.length);
 		}

test/io/calimero/mgmt/ManagementClientImplTest.java

@@ -220,7 +220,7 @@ void readMemory() throws KNXException, InterruptedException
 	@Test
 	void readMemoryMaxDataLength() throws KNXException, InterruptedException {
 		try {
-			mc.readMemory(dco, 0x1000, 63);
+			mc.readMemory(dcl, 0x1000, 63);
 		}
 		catch (KNXTimeoutException ok) {
 			// dst max apdu < 66, read is ignored
@@ -233,7 +233,7 @@ void readMemoryMaxDataLength() throws KNXException, InterruptedException {
 
 	@Test
 	void readMemoryDataLengthOutOfRange() {
-		assertThrows(KNXIllegalArgumentException.class, () -> mc.readMemory(dco, 0x1000, 64));
+		assertThrows(KNXIllegalArgumentException.class, () -> mc.readMemory(dcl, 0x1000, 64));
 	}
 
 	@Test
@@ -499,7 +499,7 @@ void writeMemory() throws KNXException, InterruptedException
 
 	@Test
 	void writeMemoryTooMuchData() {
-		assertThrows(KNXIllegalArgumentException.class, () -> mc.writeMemory(dco, 0x105, new byte[64]));
+		assertThrows(KNXIllegalArgumentException.class, () -> mc.writeMemory(dcl, 0x105, new byte[64]));
 	}
 
 	@Test

test/io/calimero/mgmt/ManagementProceduresImplTest.java

@@ -67,6 +67,7 @@ class ManagementProceduresImplTest
 	private ManagementProcedures mp;
 	private KNXNetworkLink link;
 	private final IndividualAddress device = Util.getKnxDeviceCO();
+	private final IndividualAddress deviceNoExtMem = Util.getKnxDevice();
 	private final IndividualAddress deviceInProgMode = Util.getKnxDevice();
 	private IndividualAddress nonexist;
 
@@ -223,34 +224,50 @@ void writeMemoryShort() throws KNXException, InterruptedException
 	{
 		final byte[] data = new byte[8];
 
-		mp.writeMemory(device, 0x10, data, false, false);
+		mp.writeMemory(deviceNoExtMem, 0x10, data, false, false);
 
-		mp.writeMemory(device, 0x10, data, true, false);
+		mp.writeMemory(deviceNoExtMem, 0x10, data, true, false);
 
-		mp.writeMemory(device, 0x10, data, false, true);
+		mp.writeMemory(deviceNoExtMem, 0x10, data, false, true);
 	}
 
 	@Test
 	void writeMemoryLong() throws KNXException, InterruptedException
 	{
-		final byte[] data = new byte[32];
+		final byte[] data = new byte[2 * 63 + 10];
 
-		mp.writeMemory(device, 0x10, data, false, false);
+		mp.writeMemory(deviceNoExtMem, 0x1000, data, false, false);
 
-		mp.writeMemory(device, 0x10, data, true, false);
+		mp.writeMemory(deviceNoExtMem, 0x1000, data, true, false);
 
-		mp.writeMemory(device, 0x10, data, false, true);
+		mp.writeMemory(deviceNoExtMem, 0x1000, data, false, true);
+	}
+
+	@Test
+	void writeMemoryExt() throws KNXException, InterruptedException {
+		final byte[] data = new byte[2 * 149 + 10];
+		final int startAddress = 0x10000;
+
+		mp.writeMemory(Util.getRouterAddress(), startAddress, data, false, false);
+		mp.writeMemory(Util.getRouterAddress(), startAddress, data, true, false);
+		mp.writeMemory(Util.getRouterAddress(), startAddress, data, false, true);
 	}
 
 	@Test
 	void readMemoryShort() throws KNXException, InterruptedException
 	{
-		/*final byte[] data =*/ mp.readMemory(device, 0x10, 8);
+		/*final byte[] data =*/ mp.readMemory(deviceNoExtMem, 0x10, 8);
 	}
 
 	@Test
 	void readMemoryLong() throws KNXException, InterruptedException
 	{
-		/*final byte[] data =*/ mp.readMemory(device, 0x20, 17);
+		/*final byte[] data =*/ mp.readMemory(deviceNoExtMem, 0x20, 2 * 63 + 10);
+	}
+
+	@Test
+	void readMemoryExt() throws KNXException, InterruptedException {
+		final int startAddress = 0x10000;
+		mp.readMemory(Util.getRouterAddress(), startAddress, 2 * 249 + 10);
 	}
 }