Updated discovery response when filter query does not match.
authorSachin Agrawal <sachin.agrawal@intel.com>
Wed, 22 Jul 2015 21:20:30 +0000 (14:20 -0700)
committerErich Keane <erich.keane@intel.com>
Thu, 23 Jul 2015 22:58:03 +0000 (22:58 +0000)
Iotivity should respond to discovery requests in below manner:
If query filter matching fails (or if Server does not have
any DISCOVERABLE resources) and discovery request is multicast,
it should NOT send any response.
But, if the discovery request is unicast in above scenario,
it should send an error(RESOURCE_NOT_FOUND - 404) response with
empty payload.

Currently, Iotivity stack sends a response in all error scenarios
with return code as OC_EH_OK (along with some payload) which has
various drawbacks, such as, causes un-necessary traffic in the network
and also wastes power on constrained devices.

Change-Id: Ibb3bedc7b8ae67b708b89e9ad747c031ab552cae
Signed-off-by: Sachin Agrawal <sachin.agrawal@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/1766
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Joseph Morrow <joseph.l.morrow@intel.com>
Reviewed-by: Erich Keane <erich.keane@intel.com>
resource/csdk/stack/samples/linux/secure/occlientbasicops.cpp
resource/csdk/stack/src/ocresource.c

index 92dbfdf..dd84fc3 100644 (file)
@@ -169,24 +169,28 @@ OCStackApplicationResult discoveryReqCB(void* ctx, OCDoHandle handle,
                 "Device =============> Discovered @ %s:%d",
                 clientResponse->devAddr.addr,
                 clientResponse->devAddr.port);
-        OC_LOG_PAYLOAD(INFO, TAG, clientResponse->payload);
-
-        ocConnType = clientResponse->connType;
 
-        if (parseClientResponse(clientResponse) != -1)
+        if (clientResponse->result == OC_STACK_OK)
         {
-            switch(TEST_CASE)
+            OC_LOG_PAYLOAD(INFO, TAG, clientResponse->payload);
+
+            ocConnType = clientResponse->connType;
+
+            if (parseClientResponse(clientResponse) != -1)
             {
-                case TEST_NON_CON_OP:
-                    InitGetRequest(OC_LOW_QOS);
-                    InitPutRequest(OC_LOW_QOS);
-                    //InitPostRequest(OC_LOW_QOS);
-                    break;
-                case TEST_CON_OP:
-                    InitGetRequest(OC_HIGH_QOS);
-                    InitPutRequest(OC_HIGH_QOS);
-                    //InitPostRequest(OC_HIGH_QOS);
-                    break;
+                switch(TEST_CASE)
+                {
+                    case TEST_NON_CON_OP:
+                        InitGetRequest(OC_LOW_QOS);
+                        InitPutRequest(OC_LOW_QOS);
+                        //InitPostRequest(OC_LOW_QOS);
+                        break;
+                    case TEST_CON_OP:
+                        InitGetRequest(OC_HIGH_QOS);
+                        InitPutRequest(OC_HIGH_QOS);
+                        //InitPostRequest(OC_HIGH_QOS);
+                        break;
+                }
             }
         }
     }
index 8a1d744..cab16d6 100644 (file)
@@ -515,11 +515,11 @@ static bool includeThisResourceInResponse(OCResource *resource,
 }
 
 OCStackResult SendNonPersistantDiscoveryResponse(OCServerRequest *request, OCResource *resource,
-                                OCPayload *discoveryPayload)
+                                OCPayload *discoveryPayload, OCEntityHandlerResult ehResult)
 {
     OCEntityHandlerResponse response = {};
 
-    response.ehResult = OC_EH_OK;
+    response.ehResult = ehResult;
     response.payload = discoveryPayload;
     response.persistentBufferFlag = 0;
     response.requestHandle = (OCRequestHandle) request;
@@ -536,99 +536,129 @@ static OCStackResult HandleVirtualResource (OCServerRequest *request, OCResource
     }
 
     OCStackResult discoveryResult = OC_STACK_ERROR;
+
+    bool bMulticast    = false;     // Was the discovery request a multicast request?
     OCPayload* payload = NULL;
-    char *filterOne = NULL;
-    char *filterTwo = NULL;
 
     OC_LOG(INFO, TAG, PCF("Entering HandleVirtualResource"));
 
     OCVirtualResources virtualUriInRequest = GetTypeOfVirtualURI (request->resourceUrl);
 
-
+    // Step 1: Generate the response to discovery request
     if (virtualUriInRequest == OC_WELL_KNOWN_URI)
     {
+        char *filterOne = NULL;
+        char *filterTwo = NULL;
+
         discoveryResult = getQueryParamsForFiltering (virtualUriInRequest, request->query,
-                                                            &filterOne, &filterTwo);
-        if (discoveryResult != OC_STACK_OK)
-        {
-            OC_LOG_V(ERROR, TAG, "Error (%d) validating query.\n", discoveryResult);
-            return discoveryResult;
-        }
-        payload = (OCPayload*)OCDiscoveryPayloadCreate();
+                &filterOne, &filterTwo);
 
-        if(!payload)
+        if (discoveryResult == OC_STACK_OK)
         {
-            return OC_STACK_NO_MEMORY;
-        }
+            payload = (OCPayload*)OCDiscoveryPayloadCreate();
 
-
-        for(;resource && discoveryResult == OC_STACK_OK; resource = resource->next)
-        {
-            if(includeThisResourceInResponse(resource, filterOne, filterTwo))
+            if(payload)
+            {
+                for(;resource && discoveryResult == OC_STACK_OK; resource = resource->next)
+                {
+                    if(includeThisResourceInResponse(resource, filterOne, filterTwo))
+                    {
+                        discoveryResult = BuildVirtualResourceResponse(resource,
+                                (OCDiscoveryPayload*)payload,
+                                &request->devAddr);
+                    }
+                }
+                // Set discoveryResult appropriately if no 'valid' resources are available.
+                if (((OCDiscoveryPayload*)payload)->resources == NULL)
+                {
+                    discoveryResult = OC_STACK_NO_RESOURCE;
+                }
+            }
+            else
             {
-                discoveryResult = BuildVirtualResourceResponse(resource,
-                    (OCDiscoveryPayload*)payload,
-                    &request->devAddr);
+                discoveryResult = OC_STACK_NO_MEMORY;
             }
         }
+        else
+        {
+            OC_LOG_V(ERROR, TAG, "Error (%d) parsing query.", discoveryResult);
+        }
     }
     else if (virtualUriInRequest == OC_DEVICE_URI)
     {
-            payload = (OCPayload*)OCDevicePayloadCreate(OC_RSRVD_DEVICE_URI,
-                        OCGetServerInstanceID(), savedDeviceInfo.deviceName,
-                        OC_SPEC_VERSION, OC_DATA_MODEL_VERSION);
-            if (!payload)
-            {
-                discoveryResult = OC_STACK_NO_MEMORY;
-            }
-            else
-            {
-                discoveryResult = OC_STACK_OK;
-            }
+        payload = (OCPayload*)OCDevicePayloadCreate(OC_RSRVD_DEVICE_URI,
+                OCGetServerInstanceID(), savedDeviceInfo.deviceName,
+                OC_SPEC_VERSION, OC_DATA_MODEL_VERSION);
+        if (!payload)
+        {
+            discoveryResult = OC_STACK_NO_MEMORY;
+        }
+        else
+        {
+            discoveryResult = OC_STACK_OK;
+        }
     }
     else if (virtualUriInRequest == OC_PLATFORM_URI)
     {
-            payload = (OCPayload*)OCPlatformPayloadCreate(
-                    OC_RSRVD_PLATFORM_URI,
-                    &savedPlatformInfo);
-            if (!payload)
-            {
-                discoveryResult = OC_STACK_NO_MEMORY;
-            }
-            else
-            {
-                discoveryResult = OC_STACK_OK;
-            }
+        payload = (OCPayload*)OCPlatformPayloadCreate(
+                OC_RSRVD_PLATFORM_URI,
+                &savedPlatformInfo);
+        if (!payload)
+        {
+            discoveryResult = OC_STACK_NO_MEMORY;
+        }
+        else
+        {
+            discoveryResult = OC_STACK_OK;
+        }
     }
 
+    /**
+     * Step 2: Send the discovery response
+     *
+     * Iotivity should respond to discovery requests in below manner:
+     * 1)If query filter matching fails and discovery request is multicast,
+     *   it should NOT send any response.
+     * 2)If query filter matching fails and discovery request is unicast,
+     *   it should send an error(RESOURCE_NOT_FOUND - 404) response.
+     * 3)If Server does not have any 'DISCOVERABLE' resources and discovery
+     *   request is multicast, it should NOT send any response.
+     * 4)If Server does not have any 'DISCOVERABLE' resources and discovery
+     *   request is unicast, it should send an error(RESOURCE_NOT_FOUND - 404) response.
+     */
+
     #ifdef WITH_PRESENCE
-    else
+    if ((virtualUriInRequest == OC_PRESENCE) &&
+        (resource->resourceProperties & OC_ACTIVE))
     {
-        if(resource->resourceProperties & OC_ACTIVE)
-        {
-            discoveryResult = SendPresenceNotification(resource->rsrcType,
-                                                OC_PRESENCE_TRIGGER_CHANGE);
-        }
+        // Presence uses observer notification api to respond via SendPresenceNotification.
+        SendPresenceNotification(resource->rsrcType, OC_PRESENCE_TRIGGER_CHANGE);
     }
+    else
     #endif
-
-    // Presence uses observer notification api to respond via SendPresenceNotification.
-    if (virtualUriInRequest != OC_PRESENCE)
     {
         if(discoveryResult == OC_STACK_OK)
         {
-            discoveryResult = SendNonPersistantDiscoveryResponse(request, resource,
-                                                        payload);
-            OCPayloadDestroy(payload);
+            SendNonPersistantDiscoveryResponse(request, resource, payload, OC_EH_OK);
+        }
+        else if(bMulticast == false)
+        {
+            OC_LOG_V(ERROR, TAG, "Sending a (%d) error to (%d)  \
+                discovery request", discoveryResult, virtualUriInRequest);
+            SendNonPersistantDiscoveryResponse(request, resource, NULL,
+                (discoveryResult == OC_STACK_NO_RESOURCE) ? OC_EH_RESOURCE_NOT_FOUND : OC_EH_ERROR);
         }
         else
         {
-            OC_LOG_V(ERROR, TAG, "Error (%d) building (%d)  discovery response. "\
-                        "Not responding to request.", discoveryResult, virtualUriInRequest);
+            // Ignoring the discovery request as per RFC 7252, Section #8.2
+            OC_LOG_V(INFO, TAG, "Silently ignoring the request since device does not have \
+                any useful data to send");
         }
     }
 
-    return discoveryResult;
+    OCPayloadDestroy(payload);
+
+    return OC_STACK_OK;
 }
 
 static OCStackResult