discovery: allow to ignore resource in batch notification

api/oc_discovery.c

@@ -223,7 +223,7 @@ encode_resource(CborEncoder *links, const oc_resource_t *resource,
     return false;
   }
 
-  oc_rep_start_object(links, link);
+  oc_rep_begin_object(links, link);
 
   // rel
   if (oc_core_get_resource_by_index(OCF_RES, resource->device) == resource) {
@@ -412,7 +412,7 @@ process_oic_1_1_resource(oc_resource_t *resource, oc_request_t *request,
     return false;
   }
 
-  oc_rep_start_object(links, res);
+  oc_rep_begin_object(links, res);
 
   // uri
   oc_rep_set_text_string_v1(res, href, oc_string(resource->uri),
@@ -502,7 +502,7 @@ process_oic_1_1_device_object(CborEncoder *device, oc_request_t *request,
   char uuid[OC_UUID_LEN];
   oc_uuid_to_str(oc_core_get_device_id(device_num), uuid, OC_ARRAY_SIZE(uuid));
 
-  oc_rep_start_object(device, links);
+  oc_rep_begin_object(device, links);
   oc_rep_set_text_string_v1(links, di, uuid,
                             oc_strnlen(uuid, OC_ARRAY_SIZE(uuid)));
 
@@ -700,6 +700,38 @@ discovery_process_batch_response_write_href(char *buffer, size_t buffer_size,
   buffer[to_write] = '\0';
 }
 
+static bool
+discovery_should_ignore_resource_response(
+  coap_status_t response_code, ptrdiff_t payload_start, size_t payload_size,
+  bool is_collection, bool is_notification, const oc_resource_t *resource)
+{
+  (void)resource;
+  if (response_code == CLEAR_TRANSACTION) {
+    OC_DBG("discovery%s: ignoring resource(%s) with CLEAR_TRANSACTION",
+           is_notification ? " notification" : "", oc_string(resource->uri));
+    return true;
+  }
+#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
+  if (is_notification && response_code == CONTENT_2_05) {
+    const uint8_t *payload = oc_rep_get_encoder_buf() + payload_start;
+    if (payload_size == 0 ||
+        (!is_collection && payload_size == 2 &&
+         oc_rep_encoded_payload_is_empty_object(oc_rep_encoder_get_type(),
+                                                payload, payload_size))) {
+      OC_DBG("discovery notification: ignoring empty resource(%s)",
+             oc_string(resource->uri));
+      return true;
+    }
+  }
+#else  /* !OC_DISCOVERY_RESOURCE_OBSERVABLE */
+  (void)payload_start;
+  (void)payload_size;
+  (void)is_collection;
+  (void)is_notification;
+#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
+  return false;
+}
+
 typedef bool (*discovery_process_batch_response_filter_t)(const oc_resource_t *,
                                                           void *);
 
@@ -715,15 +747,13 @@ discovery_process_batch_response(
     return false;
   }
 
-#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
   struct
   {
     CborEncoder global;
     CborEncoder links;
   } prev;
   memcpy(&prev.links, encoder, sizeof(CborEncoder));
   memcpy(&prev.global, oc_rep_get_encoder(), sizeof(CborEncoder));
-#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
 
   oc_request_t rest_request;
   memset(&rest_request, 0, sizeof(oc_request_t));
@@ -737,7 +767,7 @@ discovery_process_batch_response(
   rest_request.origin = endpoint;
   rest_request.method = OC_GET;
 
-  oc_rep_start_object(encoder, links_obj);
+  oc_rep_begin_object(encoder, links_obj);
 
   char href[OC_MAX_OCF_URI_SIZE] = { 0 };
   discovery_process_batch_response_write_href(href, OC_MAX_OCF_URI_SIZE,
@@ -779,24 +809,13 @@ discovery_process_batch_response(
     payload_size = total_payload_end - total_payload_start;
   }
 
-#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
-  if (is_notification && response_buffer.code == CONTENT_2_05) {
-    const uint8_t *payload =
-      oc_rep_get_encoder_buf() + (ptrdiff_t)total_payload_start;
-    if (payload_size == 0 ||
-        (!is_collection && payload_size == 2 &&
-         oc_rep_encoded_payload_is_empty_object(oc_rep_encoder_get_type(),
-                                                payload, payload_size))) {
-      OC_DBG("ignoring empty resource(%s)", oc_string(resource->uri));
-      memcpy(encoder, &prev.links, sizeof(CborEncoder));
-      memcpy(oc_rep_get_encoder(), &prev.global, sizeof(CborEncoder));
-      return false;
-    }
+  if (discovery_should_ignore_resource_response(
+        response_buffer.code, (ptrdiff_t)total_payload_start, payload_size,
+        is_collection, is_notification, resource)) {
+    memcpy(encoder, &prev.links, sizeof(CborEncoder));
+    memcpy(oc_rep_get_encoder(), &prev.global, sizeof(CborEncoder));
+    return false;
   }
-#else  /* !OC_DISCOVERY_RESOURCE_OBSERVABLE */
-  (void)is_notification;
-  (void)is_collection;
-#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
 
   if (payload_size == 0) {
     oc_rep_start_root_object();
@@ -1072,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)
 {
@@ -1084,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;
     }

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.
  *

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_BAD_REQUEST;
+  }
+#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;
   }

api/oc_server_api.c

@@ -139,6 +139,7 @@ oc_send_response_internal(oc_request_t *request, oc_status_t response_code,
 {
   int status_code = oc_status_code(response_code);
   if (status_code < 0) {
+    OC_ERR("could not send response: invalid response code");
     request->response->response_buffer->code = CLEAR_TRANSACTION;
     return false;
   }
@@ -190,7 +191,7 @@ oc_send_response_with_callback(oc_request_t *request, oc_status_t response_code,
   bool canTruncate = request->method != OC_GET || response_code != OC_STATUS_OK;
   if (!oc_send_response_internal(request, response_code, content_format,
                                  response_length(canTruncate), trigger_cb)) {
-    OC_ERR("could not send response: invalid response code");
+    return;
   }
 }
 

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 */
@@ -337,6 +341,14 @@ TestDiscoveryWithServer::onGetEmptyDynamicResource(oc_request_t *request,
   oc_send_response(request, OC_STATUS_OK);
 }
 
+void
+TestDiscoveryWithServer::onGetIgnoredDynamicResource(oc_request_t *request,
+                                                     oc_interface_mask_t,
+                                                     void *)
+{
+  oc_send_response(request, OC_IGNORE);
+}
+
 void
 TestDiscoveryWithServer::addDynamicResources()
 {
@@ -353,6 +365,9 @@ TestDiscoveryWithServer::addDynamicResources()
   oc::DynamicResourceHandler handlers3{};
   handlers3.onGet = onGetEmptyDynamicResource;
 
+  oc::DynamicResourceHandler handlers4{};
+  handlers4.onGet = onGetIgnoredDynamicResource;
+
   std::vector<oc::DynamicResourceToAdd> dynResources = {
     oc::makeDynamicResourceToAdd("Dynamic Resource 1",
                                  std::string(kDynamicURI1),
@@ -366,6 +381,10 @@ TestDiscoveryWithServer::addDynamicResources()
       "Dynamic Resource 3", std::string(kDynamicURI3),
       { "oic.d.observable", "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
       handlers3, OC_DISCOVERABLE | OC_OBSERVABLE),
+    oc::makeDynamicResourceToAdd(
+      "Dynamic Resource 4", std::string(kDynamicURIIgnored),
+      { "oic.d.ignored", "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R }, handlers4,
+      OC_DISCOVERABLE | OC_OBSERVABLE),
   };
   for (const auto &dr : dynResources) {
     oc_resource_t *res = oc::TestDevice::AddDynamicResource(dr, kDeviceID);
@@ -401,7 +420,6 @@ TestDiscoveryWithServer::addColletions()
   dynamicResources[std::string(kColDynamicURI1)] = { 404 };
   handlers1.onGet = onGetDynamicResource;
   handlers1.onGetData = &dynamicResources[std::string(kColDynamicURI1)];
-
   auto dr1 = oc::makeDynamicResourceToAdd(
     "Collection Resource 1", std::string(kColDynamicURI1),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -416,7 +434,6 @@ TestDiscoveryWithServer::addColletions()
   dynamicResources[std::string(kColDynamicURI2)] = { 1 };
   handlers2.onGet = onGetDynamicResource;
   handlers2.onGetData = &dynamicResources[std::string(kColDynamicURI2)];
-
   auto dr2 = oc::makeDynamicResourceToAdd(
     "Collection Resource 2", std::string(kColDynamicURI2),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -429,7 +446,6 @@ TestDiscoveryWithServer::addColletions()
 
   oc::DynamicResourceHandler handlers3{};
   handlers3.onGet = onGetEmptyDynamicResource;
-
   auto dr3 = oc::makeDynamicResourceToAdd(
     "Collection Resource 3", std::string(kColDynamicURI3),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -530,6 +546,10 @@ TestDiscoveryWithServer::getBatchResources(const oc_endpoint_t *endpoint)
   oc_resources_iterate(
     kDeviceID, true, true, true, true,
     [](oc_resource_t *resource, void *data) {
+      if (std::string(kDynamicURIIgnored) == oc_string(resource->uri)) {
+        // resource that returns OC_IGNORE shouldn't be in the batch payload
+        return true;
+      }
       if (auto *br = static_cast<batch_resources_t *>(data);
           oc_discovery_resource_is_in_batch_response(resource, br->endpoint,
                                                      true)) {

api/unittest/discovery/discovery.h

@@ -110,12 +110,12 @@ class TestDiscoveryWithServer : public ::testing::Test {
   void SetUp() override;
   void TearDown() override;
 
-  static void onGetDynamicResource(oc_request_t *request,
-                                   oc_interface_mask_t interface,
+  static void onGetDynamicResource(oc_request_t *request, oc_interface_mask_t,
                                    void *user_data);
   static void onGetEmptyDynamicResource(oc_request_t *request,
-                                        oc_interface_mask_t interface,
-                                        void *user_data);
+                                        oc_interface_mask_t, void *);
+  static void onGetIgnoredDynamicResource(oc_request_t *request,
+                                          oc_interface_mask_t, void *);
 
   static void addDynamicResources();
 #ifdef OC_COLLECTIONS
@@ -154,6 +154,7 @@ constexpr size_t kDeviceID{ 0 };
 constexpr std::string_view kDynamicURI1 = "/dyn/discoverable";
 constexpr std::string_view kDynamicURI2 = "/dyn/undiscoverable";
 constexpr std::string_view kDynamicURI3 = "/dyn/empty";
+constexpr std::string_view kDynamicURIIgnored = "/dyn/ignored";
 
 #ifdef OC_COLLECTIONS
 

api/unittest/discovery/discoveryobservetest.cpp

@@ -418,7 +418,7 @@ TEST_F(TestDiscoveryWithServer, ObserveBatchWithResourceAdded)
   verifyBatchPayload(obd.batch, &ep);
 }
 
-TEST_F(TestDiscoveryWithServer, ObserveBatchIgnoreEmptyChange)
+TEST_F(TestDiscoveryWithServer, ObserveBatchSkipEmptyResourceChange)
 {
   ASSERT_TRUE(oc_get_con_res_announced());
 
@@ -451,6 +451,40 @@ TEST_F(TestDiscoveryWithServer, ObserveBatchIgnoreEmptyChange)
   EXPECT_TRUE(obd.batch.empty());
 }
 
+TEST_F(TestDiscoveryWithServer, ObserveBatchSkipIgnoreResourceChange)
+{
+  ASSERT_TRUE(oc_get_con_res_announced());
+
+  auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
+  ASSERT_TRUE(epOpt.has_value());
+  auto ep = std::move(*epOpt);
+
+  observeBatchData obd{};
+  ASSERT_TRUE(oc_do_observe(OCF_RES_URI, &ep, "if=" OC_IF_B_STR, onBatchObserve,
+                            HIGH_QOS, &obd));
+  oc::TestDevice::PoolEventsMsV1(1s);
+  EXPECT_EQ(OC_COAP_OPTION_OBSERVE_REGISTER, obd.observe);
+  ASSERT_FALSE(obd.batch.empty());
+  // all resources should be in the first payload
+  verifyBatchPayload(obd.batch, &ep);
+  obd.observe = 0;
+  obd.batch.clear();
+
+  auto *res = oc_ri_get_app_resource_by_uri(
+    kDynamicURIIgnored.data(), kDynamicURIIgnored.size(), kDeviceID);
+  ASSERT_NE(nullptr, res);
+  // notification with payload from resource that returns OC_IGNORE should be
+  // ignored
+  oc_notify_resource_changed(res);
+  int repeats = 0;
+  while (obd.observe == 0 && repeats < 30) {
+    oc::TestDevice::PoolEventsMsV1(10ms);
+    ++repeats;
+  }
+  EXPECT_EQ(0, obd.observe);
+  EXPECT_TRUE(obd.batch.empty());
+}
+
 TEST_F(TestDiscoveryWithServer, ObserveBatchWithEmptyPayload)
 {
   // TODO: remove resource from payload -> make it undiscoverable in runtime