fixup! discovery: allow to ignore resource in batch responses

iotivity · Apr 22, 2024 · 00e7195 · 00e7195
1 parent a211487
commit 00e7195
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 16 deletions.
diff --git a/api/oc_discovery.c b/api/oc_discovery.c
@@ -1091,6 +1091,18 @@ discovery_encode_sdi(CborEncoder *object, size_t device)
 
 #endif
 
+#ifdef OC_RES_BATCH_SUPPORT
+bool
+oc_discovery_batch_response_is_supported(const oc_endpoint_t *ep)
+{
+  return ep != NULL
+#ifdef OC_SECURITY
+         && (ep->flags & SECURED) != 0
+#endif /* OC_SECURITY */
+    ;
+}
+#endif /* OC_RES_BATCH_SUPPORT */
+
 static int
 discovery_encode(const oc_request_t *request, oc_interface_mask_t iface)
 {
@@ -1103,11 +1115,7 @@ discovery_encode(const oc_request_t *request, oc_interface_mask_t iface)
   }
 #ifdef OC_RES_BATCH_SUPPORT
   case OC_IF_B: {
-    if (request->origin == NULL
-#ifdef OC_SECURITY
-        || (request->origin->flags & SECURED) == 0
-#endif /* OC_SECURITY */
-    ) {
+    if (!oc_discovery_batch_response_is_supported(request->origin)) {
       OC_ERR("oc_discovery: insecure batch interface requests are unsupported");
       return -1;
     }

diff --git a/api/oc_discovery_internal.h b/api/oc_discovery_internal.h
@@ -98,6 +98,9 @@ bool oc_filter_out_ep_for_resource(const oc_endpoint_t *ep,
 
 #ifdef OC_RES_BATCH_SUPPORT
 
+/** Batch responses are currently only supported with a secure endpoint */
+bool oc_discovery_batch_response_is_supported(const oc_endpoint_t *ep);
+
 /**
  * @brief Check if resource should be included in the batch response.
  *

diff --git a/api/oc_ri.c b/api/oc_ri.c
@@ -16,6 +16,7 @@
  *
  ***************************************************************************/
 
+#include "api/oc_discovery_internal.h"
 #include "api/oc_endpoint_internal.h"
 #include "api/oc_event_callback_internal.h"
 #include "api/oc_events_internal.h"
@@ -1652,11 +1653,21 @@ ri_invoke_coap_set_etag(const ri_invoke_coap_set_etag_in_t *in,
    */
   const uint8_t *req_etag_buf = NULL;
   uint8_t req_etag_buf_len = coap_options_get_etag(in->request, &req_etag_buf);
+  bool is_batch = in->iface_mask == OC_IF_B;
+#ifdef OC_RES_BATCH_SUPPORT
+  if (is_batch &&
+      (oc_core_get_resource_by_index(OCF_RES, in->endpoint->device) ==
+       in->preparsed_request_obj->cur_resource) &&
+      !oc_discovery_batch_response_is_supported(in->endpoint)) {
+    OC_DBG(
+      "skip etag: insecure discovery batch interface requests are unsupported");
+    return BITMASK_CODE_OK;
+  }
+#endif /* OC_RES_BATCH_SUPPORT */
   uint64_t etag =
-    (in->iface_mask == OC_IF_B)
-      ? oc_ri_get_batch_etag(in->preparsed_request_obj->cur_resource,
-                             in->endpoint, in->endpoint->device)
-      : oc_ri_get_etag(in->preparsed_request_obj->cur_resource);
+    (is_batch) ? oc_ri_get_batch_etag(in->preparsed_request_obj->cur_resource,
+                                      in->endpoint, in->endpoint->device)
+               : oc_ri_get_etag(in->preparsed_request_obj->cur_resource);
   if (etag == OC_ETAG_UNINITIALIZED) {
     return bitmask_code;
   }

diff --git a/api/unittest/discovery/discovery.cpp b/api/unittest/discovery/discovery.cpp
@@ -28,6 +28,10 @@
 #include "tests/gtest/Resource.h"
 #include "tests/gtest/Utility.h"
 
+#ifdef OC_HAS_FEATURE_ETAG
+#include "api/oc_etag_internal.h"
+#endif /* OC_HAS_FEATURE_ETAG */
+
 #ifdef OC_COLLECTIONS
 #include "tests/gtest/Collection.h"
 #endif /* OC_COLLECTIONS */

diff --git a/api/unittest/discovery/discoverytest.cpp b/api/unittest/discovery/discoverytest.cpp
@@ -34,6 +34,7 @@
 #include "tests/gtest/Device.h"
 #include "tests/gtest/RepPool.h"
 #include "tests/gtest/Resource.h"
+#include "util/oc_features.h"
 #include "util/oc_macros_internal.h"
 
 #ifdef OC_SECURITY
@@ -328,6 +329,19 @@ TEST_F(TestDiscoveryWithServer, GetRequestBaseline)
 
 #ifdef OC_RES_BATCH_SUPPORT
 
+class TestBatchDiscoveryWithServer : public TestDiscoveryWithServer {
+public:
+  void TearDown() override
+  {
+#ifdef OC_HAS_FEATURE_ETAG
+    oc::IterateAllResources([](oc_resource_t *resource) {
+      oc_resource_set_etag(resource, oc_etag_get());
+    });
+#endif /* OC_HAS_FEATURE_ETAG */
+    TestDiscoveryWithServer::TearDown();
+  }
+};
+
 // batch interface
 // [
 //   {
@@ -338,7 +352,7 @@ TEST_F(TestDiscoveryWithServer, GetRequestBaseline)
 //   },
 //   ...
 // ]
-TEST_F(TestDiscoveryWithServer, GetRequestBatch)
+TEST_F(TestBatchDiscoveryWithServer, GetRequestBatch)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());
@@ -505,7 +519,7 @@ testBatchIncrementalChanges(
   testBatchIncrementalChanges(ep, &coapETag, query, expectedCode, expected);
 }
 
-TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_Single)
+TEST_F(TestBatchDiscoveryWithServer, GetRequestBatchIncremental_Single)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());
@@ -533,7 +547,7 @@ TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_Single)
 }
 
 // Invalid ETag0 should be ignored
-TEST_F(TestDiscoveryWithServer,
+TEST_F(TestBatchDiscoveryWithServer,
        GetRequestBatchIncremental_Single_InvalidIncrementalETag0)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
@@ -557,7 +571,7 @@ TEST_F(TestDiscoveryWithServer,
 
 // invalid ETags in the query should be ignored, but if a single valid ETag is
 // found then is should be used
-TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_InvalidETags)
+TEST_F(TestBatchDiscoveryWithServer, GetRequestBatchIncremental_InvalidETags)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());
@@ -644,7 +658,7 @@ TEST_F(TestDiscoveryWithServer,
 // Test with multiple etags in the query, sorted in descending order, so only
 // the first should trigger the iteration of resources and subsequent ETag
 // candidates should be skipped because they have a lower value
-TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_Multiple)
+TEST_F(TestBatchDiscoveryWithServer, GetRequestBatchIncremental_Multiple)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());
@@ -680,7 +694,8 @@ TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_Multiple)
 // Test with multiple etags in the query, sorted in ascending order, so all
 // candidates should trigger resources iteration and the last value should be
 // finally used as the ETag
-TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_MultipleAscending)
+TEST_F(TestBatchDiscoveryWithServer,
+       GetRequestBatchIncremental_MultipleAscending)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());
@@ -715,7 +730,7 @@ TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_MultipleAscending)
 
 /// Test will all batch resources etags in query, one of them is the latest
 /// updated thus we should receive a VALID response with empty payload
-TEST_F(TestDiscoveryWithServer, GetRequestBatchIncremental_AllAscending)
+TEST_F(TestBatchDiscoveryWithServer, GetRequestBatchIncremental_AllAscending)
 {
   auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
   ASSERT_TRUE(epOpt.has_value());

diff --git a/api/unittest/resourcetest.cpp b/api/unittest/resourcetest.cpp
@@ -387,6 +387,9 @@ TestResourceWithDevice::addDynamicResources()
   for (const auto &dr : dynResources) {
     oc_resource_t *res = oc::TestDevice::AddDynamicResource(dr, kDevice1ID);
     ASSERT_NE(nullptr, res);
+#ifdef OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM
+    oc_resource_set_access_in_RFOTM(res, true, OC_PERM_RETRIEVE);
+#endif /* OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM */
   }
 }
 
@@ -585,6 +588,8 @@ TEST_F(TestResourceWithDevice, IterateCollectionResources)
   ASSERT_TRUE(collection4.empty());
 }
 
+#if !defined(OC_SECURITY) || defined(OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM)
+
 // Resource that returns OC_IGNORE shouldn't return any payload
 TEST_F(TestResourceWithDevice, GetRequestIgnoredResource)
 {
@@ -608,6 +613,8 @@ TEST_F(TestResourceWithDevice, GetRequestIgnoredResource)
   ASSERT_TRUE(invoked);
 }
 
+#endif /* !OC_SECURITY || OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM */
+
 #endif /* OC_SERVER && OC_DYNAMIC_ALLOCATION */
 
 #if !defined(OC_SECURITY) || defined(OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM)