forever loop bugfix (#80)

* tests for finding out forever loop in block transfer * handling client & server cases for sudden change to error * client to receive error message instead of throwing * meh * responding with full error * updating copyright year
open-coap · Mar 15, 2024 · 1a85926 · 1a85926
1 parent 84c812c
commit 1a85926
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 12 deletions.
diff --git a/coap-core/src/main/java/com/mbed/coap/packet/Code.java b/coap-core/src/main/java/com/mbed/coap/packet/Code.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022-2023 java-coap contributors (https://github.com/open-coap/java-coap)
+ * Copyright (C) 2022-2024 java-coap contributors (https://github.com/open-coap/java-coap)
  * Copyright (C) 2011-2018 ARM Limited. All rights reserved.
  * SPDX-License-Identifier: Apache-2.0
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -96,4 +96,13 @@ public static Code valueOf(int codeClass, int codeDetail) {
     public boolean isSignaling() {
         return coapCode >>> 5 == 7;
     }
+
+    public boolean isError() {
+        return coapCode >>> 5 == 4 || coapCode >>> 5 == 5;
+    }
+
+    public boolean isSuccess() {
+        return coapCode >>> 5 == 2;
+    }
+
 }
diff --git a/coap-core/src/main/java/com/mbed/coap/server/block/BlockWiseCallback.java b/coap-core/src/main/java/com/mbed/coap/server/block/BlockWiseCallback.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022-2023 java-coap contributors (https://github.com/open-coap/java-coap)
+ * Copyright (C) 2022-2024 java-coap contributors (https://github.com/open-coap/java-coap)
  * Copyright (C) 2011-2021 ARM Limited. All rights reserved.
  * SPDX-License-Identifier: Apache-2.0
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -137,6 +137,10 @@ private CompletableFuture<CoapResponse> handleIfBlock1(CoapResponse response) {
     private CompletableFuture<CoapResponse> receiveBlock2(CoapResponse blResponse) {
         LOGGER.trace("Received CoAP block [{}]", blResponse.options().getBlock2Res());
 
+        if (response != null && !response.getCode().isError() && blResponse.getCode().isError()) {
+            return completedFuture(blResponse);
+        }
+
         String errMsg = verifyBlockResponse(request.options().getBlock2Res(), blResponse);
         if (errMsg != null) {
             return failedFuture(new CoapBlockException(errMsg));
@@ -147,8 +151,8 @@ private CompletableFuture<CoapResponse> receiveBlock2(CoapResponse blResponse) {
         } else {
             this.response = CoapResponse.of(blResponse.getCode(), response.getPayload().concat(blResponse.getPayload()), response.options())
                     .withOptions(o -> o.block2Res(blResponse.options().getBlock2Res()));
-
         }
+
         if (hasResourceChanged(blResponse)) {
             return restartBlockTransfer(blResponse);
         }

diff --git a/coap-core/src/main/java/com/mbed/coap/server/block/BlockWiseIncomingFilter.java b/coap-core/src/main/java/com/mbed/coap/server/block/BlockWiseIncomingFilter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022-2023 java-coap contributors (https://github.com/open-coap/java-coap)
+ * Copyright (C) 2022-2024 java-coap contributors (https://github.com/open-coap/java-coap)
  * Copyright (C) 2011-2021 ARM Limited. All rights reserved.
  * SPDX-License-Identifier: Apache-2.0
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -115,6 +115,10 @@ private void removeBlockRequest(BlockRequestId blockRequestId) {
     }
 
     public CoapResponse adjustPayloadSize(CoapRequest req, CoapResponse resp) {
+        if (resp.getCode().isError()) {
+            return resp;
+        }
+
         resp.options().setBlock1Req(req.options().getBlock1Req());
         if (resp.options().getBlock2Res() == null) {
 
@@ -147,6 +151,7 @@ private CoapResponse updateBlockResponse(final BlockOption block2Response, final
         } else {
             block2Res = new BlockOption(block2Res.getNr(), block2Res.getBlockSize(), true);
         }
+
         int newLength = blTo - blFrom;
         if (newLength < 0) {
             newLength = 0;

diff --git a/coap-core/src/test/java/com/mbed/coap/packet/CodeTest.java b/coap-core/src/test/java/com/mbed/coap/packet/CodeTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022 java-coap contributors (https://github.com/open-coap/java-coap)
+ * Copyright (C) 2022-2024 java-coap contributors (https://github.com/open-coap/java-coap)
  * Copyright (C) 2011-2021 ARM Limited. All rights reserved.
  * SPDX-License-Identifier: Apache-2.0
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -16,7 +16,9 @@
  */
 package com.mbed.coap.packet;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import org.junit.jupiter.api.Test;
 
 public class CodeTest {
@@ -49,4 +51,49 @@ public void testSignaling() {
         assertFalse(Code.C405_METHOD_NOT_ALLOWED.isSignaling());
         assertFalse(Code.C503_SERVICE_UNAVAILABLE.isSignaling());
     }
+
+    @Test
+    public void testIsError() {
+        assertTrue(Code.C400_BAD_REQUEST.isError());
+        assertTrue(Code.C401_UNAUTHORIZED.isError());
+        assertTrue(Code.C402_BAD_OPTION.isError());
+        assertTrue(Code.C403_FORBIDDEN.isError());
+        assertTrue(Code.C404_NOT_FOUND.isError());
+        assertTrue(Code.C405_METHOD_NOT_ALLOWED.isError());
+        assertTrue(Code.C406_NOT_ACCEPTABLE.isError());
+        assertTrue(Code.C408_REQUEST_ENTITY_INCOMPLETE.isError());
+        assertTrue(Code.C409_CONFLICT.isError());
+        assertTrue(Code.C412_PRECONDITION_FAILED.isError());
+        assertTrue(Code.C413_REQUEST_ENTITY_TOO_LARGE.isError());
+        assertTrue(Code.C415_UNSUPPORTED_MEDIA_TYPE.isError());
+        assertTrue(Code.C422_UNPROCESSABLE_ENTITY.isError());
+        assertTrue(Code.C500_INTERNAL_SERVER_ERROR.isError());
+        assertTrue(Code.C501_NOT_IMPLEMENTED.isError());
+        assertTrue(Code.C502_BAD_GATEWAY.isError());
+        assertTrue(Code.C503_SERVICE_UNAVAILABLE.isError());
+        assertTrue(Code.C504_GATEWAY_TIMEOUT.isError());
+        assertTrue(Code.C505_PROXYING_NOT_SUPPORTED.isError());
+
+        assertFalse(Code.C203_VALID.isError());
+        assertFalse(Code.C231_CONTINUE.isError());
+        assertFalse(Code.C701_CSM.isError());
+        assertFalse(Code.C702_PING.isError());
+    }
+
+    @Test
+    public void testIsSuccess() {
+        assertTrue(Code.C201_CREATED.isSuccess());
+        assertTrue(Code.C202_DELETED.isSuccess());
+        assertTrue(Code.C203_VALID.isSuccess());
+        assertTrue(Code.C204_CHANGED.isSuccess());
+        assertTrue(Code.C205_CONTENT.isSuccess());
+        assertTrue(Code.C231_CONTINUE.isSuccess());
+
+        assertFalse(Code.C405_METHOD_NOT_ALLOWED.isSuccess());
+        assertFalse(Code.C412_PRECONDITION_FAILED.isSuccess());
+        assertFalse(Code.C701_CSM.isSuccess());
+        assertFalse(Code.C705_ABORT.isSuccess());
+        assertFalse(Code.C500_INTERNAL_SERVER_ERROR.isSuccess());
+        assertFalse(Code.C503_SERVICE_UNAVAILABLE.isSuccess());
+    }
 }
diff --git a/coap-core/src/test/java/protocolTests/BlockTransferOnDemandTest.java b/coap-core/src/test/java/protocolTests/BlockTransferOnDemandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022-2023 java-coap contributors (https://github.com/open-coap/java-coap)
+ * Copyright (C) 2022-2024 java-coap contributors (https://github.com/open-coap/java-coap)
  * Copyright (C) 2011-2021 ARM Limited. All rights reserved.
  * SPDX-License-Identifier: Apache-2.0
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -42,28 +42,24 @@
  * Block1 header option block transfer size limit tests.
  */
 public class BlockTransferOnDemandTest {
-    private static final int MAX_DATA = 32;
-
     private CoapServer server = null;
     private CoapClient client = null;
 
     @BeforeEach
     public void setUp() throws IOException {
 
         server = CoapServer.builder()
-                .maxIncomingBlockTransferSize(MAX_DATA)
-                .blockSize(BlockSize.S_16)
                 .transport(InMemoryCoapTransport.create(5683))
                 .route(RouterService.builder()
                         .get("/man", new ManualBlockTransferCoapResource())
+                        .get("/missing-second-block", new MissingSecondBlock())
                         .build()
                 )
                 .build();
 
         server.start();
 
         client = CoapServer.builder()
-                .maxIncomingBlockTransferSize(MAX_DATA)
                 .transport(InMemoryCoapTransport.create())
                 .buildClient(localhost(5683));
     }
@@ -81,6 +77,18 @@ public void onDemandTest() throws ExecutionException, InterruptedException, Coap
         assertEquals(1, resp.options().getBlock2Res().getNr());
     }
 
+    @Test
+    public void MissingBlockTest() throws ExecutionException, InterruptedException, CoapException {
+        CoapResponse resp = client.sendSync(get("/missing-second-block").blockSize(BlockSize.S_16));
+        assertEquals(coapResponse(Code.C404_NOT_FOUND).payload("This is exactly a 35 characters!!!!").build(), resp);
+    }
+
+    @Test
+    public void MissingBlockTest2() throws ExecutionException, InterruptedException, CoapException {
+        CoapResponse resp = client.sendSync(get("/missing-second-block").blockSize(BlockSize.S_32));
+        assertEquals(coapResponse(Code.C404_NOT_FOUND).payload("This is exactly a 35 characters!!!!").build(), resp);
+    }
+
     private class ManualBlockTransferCoapResource implements Service<CoapRequest, CoapResponse> {
 
         @Override
@@ -96,5 +104,20 @@ public CompletableFuture<CoapResponse> apply(CoapRequest req) {
 
         }
     }
+
+    private class MissingSecondBlock implements Service<CoapRequest, CoapResponse> {
+
+        @Override
+        public CompletableFuture<CoapResponse> apply(CoapRequest req) {
+            int blockNr = req.options().getBlock2Res() == null ? 0 : req.options().getBlock2Res().getNr();
+            String payload = req.options().getBlock2Res().getBlockSize() == BlockSize.S_16 ? "16B-of-data-here" : "32B-of-data-here32B-of-data-here";
+            if (blockNr == 0) {
+                return coapResponse(Code.C205_CONTENT).block2Res(0, req.options().getBlock2Res().getBlockSize(), true).payload(payload).toFuture();
+            } else {
+                return coapResponse(Code.C404_NOT_FOUND).payload("This is exactly a 35 characters!!!!").toFuture();
+            }
+
+        }
+    }
 }