Fixes the observe option size to the legal max value of 3.
authorStephane Lejeune <stlejeun@cisco.com>
Wed, 2 Sep 2015 11:45:55 +0000 (13:45 +0200)
committerJon A. Cruz <jonc@osg.samsung.com>
Fri, 4 Sep 2015 18:29:23 +0000 (18:29 +0000)
The options that have variable byte length are now being
encoded to their minimal value. This patch also truncates the
option data to their maximum value allowed per the spec. One specific
example is the observer data option which receives 4 bytes of data from
the OC stack. When encoding in CoAP, the maximum value is 3 bytes.
Consequently, the CA layer which is aware of the CoAP encoding truncates
the value back to 3 bytes. Also when registering an observe, the data
length is now set to 0 using the CoAP var byte encoding which saves the
4 bytes that were just carying zero's before thsi fix.

Change-Id: I033969b407aa3426df8c85c3836206c9366fb0a2
Signed-off-by: Stephane Lejeune <stlejeun@cisco.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/2351
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Jon A. Cruz <jonc@osg.samsung.com>
resource/csdk/connectivity/inc/caprotocolmessage.h
resource/csdk/connectivity/lib/libcoap-4.1.1/encode.c
resource/csdk/connectivity/lib/libcoap-4.1.1/encode.h
resource/csdk/connectivity/lib/libcoap-4.1.1/option.c
resource/csdk/connectivity/lib/libcoap-4.1.1/option.h
resource/csdk/connectivity/src/caprotocolmessage.c
resource/csdk/stack/src/ocobserve.c
resource/csdk/stack/src/ocserverrequest.c
resource/csdk/stack/src/ocstack.c

index d97af0d..ae3e1c2 100644 (file)
@@ -151,13 +151,15 @@ uint32_t CAGetOptionCount(coap_opt_iterator_t opt_iter);
 
 /**
  * gets option data.
+ * @param[in]   key                  ID of the option
  * @param[in]   data                 data that is received.
  * @param[in]   length               length of the data.
  * @param[out]  option               result of the operation.
  * @param[in]   buflen               buffer length of the result.
  * @return  option count.
  */
-uint32_t CAGetOptionData(const uint8_t *data, uint32_t len, uint8_t *option, uint32_t buflen);
+uint32_t CAGetOptionData(uint16_t key, const uint8_t *data, uint32_t len,
+        uint8_t *option, uint32_t buflen);
 
 /**
  * extracts request information from received pdu.
index 86a5b89..a310008 100644 (file)
@@ -6,12 +6,19 @@
  * README for terms of use.
  */
 
+#include "config.h"
+
+#if defined(HAVE_ASSERT_H) && !defined(assert)
+# include <assert.h>
+#endif
+
 #ifndef NDEBUG
 #  include <stdio.h>
 #endif
 
 #include "config.h"
 #include "encode.h"
+#include "option.h"
 
 /* Carsten suggested this when fls() is not available: */
 int coap_fls(unsigned int i)
@@ -48,3 +55,16 @@ unsigned int coap_encode_var_bytes(unsigned char *buf, unsigned int val)
     return n;
 }
 
+bool coap_is_var_bytes(coap_option_def_t* def)
+{
+    assert (def);
+
+    if('u' == def->type)
+    {
+        return 1;
+    }
+    else
+    {
+      return 0;
+    }
+}
index b9b304e..b1b2e57 100644 (file)
@@ -17,6 +17,8 @@
 #endif
 #endif
 
+#include "option.h"
+
 #define Nn 8  /* duplicate definition of N if built on sky motes */
 #define E 4
 #define HIBIT (1 << (Nn - 1))
@@ -51,4 +53,10 @@ unsigned int coap_decode_var_bytes(unsigned char *buf, unsigned int len);
  */
 unsigned int coap_encode_var_bytes(unsigned char *buf, unsigned int val);
 
+/**
+ * Tests whether the option definition has a type that allows variable byte encoding.
+ * Returns true when supported, false when not supported.
+ */
+bool coap_is_var_bytes(coap_option_def_t* def);
+
 #endif /* _COAP_ENCODE_H_ */
index 3255020..9a52fb4 100644 (file)
@@ -18,6 +18,7 @@
 
 #include "option.h"
 #include "debug.h"
+#include "pdu.h"
 
 coap_opt_t *
 options_start(coap_pdu_t *pdu)
@@ -438,3 +439,42 @@ size_t coap_opt_encode(coap_opt_t *opt, size_t maxlen, unsigned short delta,
     return l + length;
 }
 
+static coap_option_def_t coap_option_def[] = {
+    { COAP_OPTION_IF_MATCH,       'o', 0,   8 },
+    { COAP_OPTION_URI_HOST,       's', 1, 255 },
+    { COAP_OPTION_ETAG,           'o', 1,   8 },
+    { COAP_OPTION_IF_NONE_MATCH,  'e', 0,   0 },
+    { COAP_OPTION_URI_PORT,       'u', 0,   2 },
+    { COAP_OPTION_LOCATION_PATH,  's', 0, 255 },
+    { COAP_OPTION_URI_PATH,       's', 0, 255 },
+    { COAP_OPTION_CONTENT_TYPE,   'u', 0,   2 },
+    { COAP_OPTION_MAXAGE,         'u', 0,   4 },
+    { COAP_OPTION_URI_QUERY,      's', 1, 255 },
+    { COAP_OPTION_ACCEPT,         'u', 0,   2 },
+    { COAP_OPTION_LOCATION_QUERY, 's', 0, 255 },
+    { COAP_OPTION_PROXY_URI,      's', 1,1034 },
+    { COAP_OPTION_PROXY_SCHEME,   's', 1, 255 },
+    { COAP_OPTION_SIZE1,          'u', 0,   4 },
+    { COAP_OPTION_SIZE2,          'u', 0,   4 },
+    { COAP_OPTION_OBSERVE,        'u', 0,   3 },
+    { COAP_OPTION_BLOCK2,         'u', 0,   3 },
+    { COAP_OPTION_BLOCK1,         'u', 0,   3 },
+};
+
+
+coap_option_def_t* coap_opt_def(unsigned short key)
+{
+    int i;
+
+    if (COAP_MAX_OPT < key)
+    {
+        return NULL;
+    }
+    for (i = 0; i < (int)(sizeof(coap_option_def)/sizeof(coap_option_def_t)); i++)
+    {
+        if (key == coap_option_def[i].key)
+            return &(coap_option_def[i]);
+    }
+    debug("coap_opt_def: add key:[%d] to coap_is_var_bytes", key);
+    return NULL;
+}
index 77c964c..cfdcfaf 100644 (file)
@@ -34,6 +34,18 @@ typedef struct
     unsigned char *value;
 } coap_option_t;
 
+
+/** Representation of the association between a CoAP option key and its
+ *  data type and valid data length ranges.
+ */
+typedef struct
+{
+    unsigned short key;     /**< The ID of the option the following values apply to. */
+    unsigned char type;     /**< The type of the option: u=uint, s=string, o=opaque. */
+    unsigned int min;       /**< The minimum number of bytes allowed for the option data */
+    unsigned int max;       /**< The maximum number of bytes allowed for the option data */
+} coap_option_def_t;
+
 /**
  * Parses the option pointed to by @p opt into @p result. This
  * function returns the number of bytes that have been parsed, or @c 0
@@ -312,6 +324,16 @@ unsigned short coap_opt_length(const coap_opt_t *opt);
  */
 unsigned char *coap_opt_value(coap_opt_t *opt);
 
+/**
+ * Returns a pointer to the coap option range definitions. @key
+ * must be a valid option ID. This function returns @c NULL if
+ * @p key is not a valid option ID.
+ *
+ * @param key The option ID whose definition should be returned.
+ * @return A pointer to the option definition.
+ */
+coap_option_def_t* coap_opt_def(unsigned short key);
+
 /** @deprecated { Use coap_opt_value() instead. } */
 #define COAP_OPT_VALUE(opt) coap_opt_value((coap_opt_t *)opt)
 
index 8435bdd..ec48e71 100644 (file)
@@ -526,8 +526,30 @@ coap_list_t *CACreateNewOptionNode(uint16_t key, uint32_t length, const char *da
     memset(option, 0, sizeof(coap_option) + length + 1);
 
     COAP_OPTION_KEY(*option) = key;
-    COAP_OPTION_LENGTH(*option) = length;
-    memcpy(COAP_OPTION_DATA(*option), data, length);
+
+    coap_option_def_t* def = coap_opt_def(key);
+    if (NULL != def && coap_is_var_bytes(def))
+    {
+       if (length > def->max)
+        {
+            // make sure we shrink the value so it fits the coap option definition
+            // by truncating the value, disregard the leading bytes.
+            OIC_LOG_V(DEBUG, TAG, "Option [%d] data size [%d] shrunk to [%d]",
+                    def->key, length, def->max);
+            data = &(data[length-def->max]);
+            length = def->max;
+        }
+        // Shrink the encoding length to a minimum size for coap
+        // options that support variable length encoding.
+         COAP_OPTION_LENGTH(*option) = coap_encode_var_bytes(
+                COAP_OPTION_DATA(*option),
+                coap_decode_var_bytes((unsigned char *)data, length));
+    }
+    else
+    {
+        COAP_OPTION_LENGTH(*option) = length;
+        memcpy(COAP_OPTION_DATA(*option), data, length);
+    }
 
     /* we can pass NULL here as delete function since option is released automatically  */
     coap_list_t *node = coap_new_listnode(option, NULL);
@@ -630,11 +652,12 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, uint32_t *outCode, CAInfo_t *
     while ((option = coap_option_next(&opt_iter)))
     {
         char buf[COAP_MAX_PDU_SIZE] = {0};
-        if (CAGetOptionData((uint8_t *)(COAP_OPT_VALUE(option)),
-                            COAP_OPT_LENGTH(option), (uint8_t *)buf, sizeof(buf)))
+        uint32_t bufLength =
+            CAGetOptionData(opt_iter.type, (uint8_t *)(COAP_OPT_VALUE(option)),
+                    COAP_OPT_LENGTH(option), (uint8_t *)buf, sizeof(buf));
+        if (bufLength)
         {
             OIC_LOG_V(DEBUG, TAG, "COAP URI element : %s", buf);
-            uint32_t bufLength = strlen(buf);
             if (COAP_OPTION_URI_PATH == opt_iter.type || COAP_OPTION_URI_QUERY == opt_iter.type)
             {
                 if (false == isfirstsetflag)
@@ -916,11 +939,12 @@ void CADestroyInfo(CAInfo_t *info)
     OIC_LOG(DEBUG, TAG, "OUT");
 }
 
-uint32_t CAGetOptionData(const uint8_t *data, uint32_t len, uint8_t *option, uint32_t buflen)
+uint32_t CAGetOptionData(uint16_t key, const uint8_t *data, uint32_t len,
+        uint8_t *option, uint32_t buflen)
 {
-    if (0 == buflen || 0 == len)
+    if (0 == buflen)
     {
-        OIC_LOG(ERROR, TAG, "len 0");
+        OIC_LOG(ERROR, TAG, "buflen 0");
         return 0;
     }
 
@@ -936,8 +960,17 @@ uint32_t CAGetOptionData(const uint8_t *data, uint32_t len, uint8_t *option, uin
         return 0;
     }
 
-    memcpy(option, data, len);
-    option[len] = '\0';
+    coap_option_def_t* def = coap_opt_def(key);
+    if(NULL != def && coap_is_var_bytes(def) && 0 == len) {
+        // A 0 length option is permitted in CoAP but the
+        // rest or the stack is unaware of variable byte encoding
+        // should remain that way so a 0 byte of length 1 is inserted.
+        len = 1;
+        option[0]=0;
+    } else {
+        memcpy(option, data, len);
+        option[len] = '\0';
+    }
 
     return len;
 }
index 0297707..72a3e7c 100644 (file)
@@ -347,7 +347,7 @@ OCStackResult GenerateObserverId (OCObservationId *observationId)
         resObs = GetObserverUsingId (*observationId);
     } while (NULL != resObs);
 
-    OC_LOG_V(INFO, TAG, "Generated bservation ID is %u", *observationId);
+    OC_LOG_V(INFO, TAG, "GeneratedObservation ID is %u", *observationId);
 
     return OC_STACK_OK;
 exit:
@@ -533,7 +533,7 @@ CreateObserveHeaderOption (CAHeaderOption_t **caHdrOpt,
     }
     tmpHdrOpt[0].protocolID = CA_COAP_ID;
     tmpHdrOpt[0].optionID = COAP_OPTION_OBSERVE;
-    tmpHdrOpt[0].optionLength = sizeof(uint32_t);
+    tmpHdrOpt[0].optionLength = sizeof(uint8_t);
     tmpHdrOpt[0].optionData[0] = observeFlag;
     for (uint8_t i = 0; i < numOptions; i++)
     {
index f54d0e2..12d0618 100644 (file)
@@ -487,8 +487,14 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
             responseInfo.info.options[0].protocolID = CA_COAP_ID;
             responseInfo.info.options[0].optionID = COAP_OPTION_OBSERVE;
             responseInfo.info.options[0].optionLength = sizeof(uint32_t);
-            memcpy(responseInfo.info.options[0].optionData,
-                    &(serverRequest->observationOption), sizeof(uint32_t));
+            uint8_t* observationData = (uint8_t*)responseInfo.info.options[0].optionData;
+            uint32_t observationOption= serverRequest->observationOption;
+            size_t i;
+            for (i=sizeof(uint32_t); i; --i)
+            {
+                observationData[i-1] = observationOption & 0xFF;
+                observationOption >>=8;
+            }
 
             // Point to the next header option before copying vender specific header options
             optionsPointer += 1;
index 9bede93..7026d7d 100644 (file)
@@ -1059,8 +1059,18 @@ void HandleCAResponses(const CAEndpoint_t* endPoint, const CAResponseInfo_t* res
                 //First option always with option ID is COAP_OPTION_OBSERVE if it is available.
                 if(responseInfo->info.options[0].optionID == COAP_OPTION_OBSERVE)
                 {
-                    memcpy (&(response.sequenceNumber),
-                            &(responseInfo->info.options[0].optionData), sizeof(uint32_t));
+                    size_t i;
+                    uint32_t observationOption;
+                    uint8_t* optionData = (uint8_t*)responseInfo->info.options[0].optionData;
+                    for (observationOption=0, i=0;
+                            i<sizeof(uint32_t) && i<responseInfo->info.options[0].optionLength;
+                            i++)
+                    {
+                        observationOption =
+                            (observationOption << 8) | optionData[i];
+                    }
+                    response.sequenceNumber = observationOption;
+
                     response.numRcvdVendorSpecificHeaderOptions = responseInfo->info.numOptions - 1;
                     start = 1;
                 }