[IOT-2421] Allow 0-length OCByteString. 55/21455/3
authorTodd Malsbary <todd.malsbary@intel.com>
Fri, 16 Jun 2017 16:55:23 +0000 (09:55 -0700)
committerDan Mihai <Daniel.Mihai@microsoft.com>
Thu, 20 Jul 2017 19:38:52 +0000 (19:38 +0000)
Bug: https://jira.iotivity.org/browse/IOT-2421
Change-Id: I7575340adb980facc5a9713886c194848c2c36d8
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/21455
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>
resource/csdk/stack/src/ocpayload.c
resource/csdk/stack/src/ocpayloadconvert.c
resource/csdk/stack/test/cbortests.cpp
resource/csdk/stack/test/stacktests.cpp

index 7eb6832..64839e7 100644 (file)
@@ -533,7 +533,7 @@ static bool OC_CALL OCRepPayloadSetProp(OCRepPayload* payload, const char* name,
                return val->str != NULL;
         case OCREP_PROP_BYTE_STRING:
                val->ocByteStr = *(OCByteString*)value;
-               return val->ocByteStr.bytes != NULL;
+               break;
         case OCREP_PROP_NULL:
                return val != NULL;
         case OCREP_PROP_ARRAY:
@@ -633,11 +633,6 @@ bool OC_CALL OCRepPayloadGetPropString(const OCRepPayload* payload, const char*
 
 bool OC_CALL OCRepPayloadSetPropByteString(OCRepPayload* payload, const char* name, OCByteString value)
 {
-    if (!value.bytes || !value.len)
-    {
-        return false;
-    }
-
     OCByteString ocByteStr = {NULL, 0};
     bool b = OCByteStringCopy(&ocByteStr, &value);
 
@@ -671,10 +666,17 @@ bool OC_CALL OCRepPayloadGetPropByteString(const OCRepPayload* payload, const ch
         return false;
     }
 
-    value->bytes = (uint8_t*)OICMalloc(val->ocByteStr.len * sizeof(uint8_t));
-    if (!value->bytes)
+    if (val->ocByteStr.len)
     {
-        return false;
+        value->bytes = (uint8_t*)OICMalloc(val->ocByteStr.len * sizeof(uint8_t));
+        if (!value->bytes)
+        {
+            return false;
+        }
+    }
+    else
+    {
+        value->bytes = NULL;
     }
     value->len = val->ocByteStr.len;
     memcpy(value->bytes, val->ocByteStr.bytes, value->len);
@@ -934,16 +936,19 @@ bool OC_CALL OCRepPayloadSetByteStringArray(OCRepPayload* payload, const char* n
 
     for (size_t i = 0; i < dimTotal; ++i)
     {
-        newArray[i].bytes = (uint8_t*)OICMalloc(array[i].len * sizeof(uint8_t));
-        if (NULL == newArray[i].bytes)
+        if (array[i].len)
         {
-            for (size_t j = 0; j < i; ++j)
+            newArray[i].bytes = (uint8_t*)OICMalloc(array[i].len * sizeof(uint8_t));
+            if (NULL == newArray[i].bytes)
             {
-                OICFree(newArray[j].bytes);
-            }
+                for (size_t j = 0; j < i; ++j)
+                {
+                    OICFree(newArray[j].bytes);
+                }
 
-            OICFree(newArray);
-            return false;
+                OICFree(newArray);
+                return false;
+            }
         }
         newArray[i].len = array[i].len;
         memcpy(newArray[i].bytes, array[i].bytes, newArray[i].len);
@@ -988,18 +993,21 @@ bool OC_CALL OCRepPayloadGetByteStringArray(const OCRepPayload* payload, const c
     for (size_t i = 0; i < dimTotal; ++i)
     {
         OCByteString* tmp = &(*array)[i];
-        tmp->bytes = (uint8_t*)OICMalloc(val->arr.ocByteStrArray[i].len * sizeof(uint8_t));
-        if (NULL == tmp->bytes)
+        if (val->arr.ocByteStrArray[i].len)
         {
-            for (size_t j = 0; j < i; ++j)
+            tmp->bytes = (uint8_t*)OICMalloc(val->arr.ocByteStrArray[i].len * sizeof(uint8_t));
+            if (NULL == tmp->bytes)
             {
-                OCByteString* temp = &(*array)[j];
-                OICFree(temp->bytes);
-            }
-            OICFree(*array);
-            *array = NULL;
+                for (size_t j = 0; j < i; ++j)
+                {
+                    OCByteString* temp = &(*array)[j];
+                    OICFree(temp->bytes);
+                }
+                OICFree(*array);
+                *array = NULL;
 
-            return false;
+                return false;
+            }
         }
         tmp->len = val->arr.ocByteStrArray[i].len;
         memcpy(tmp->bytes, val->arr.ocByteStrArray[i].bytes, tmp->len);
@@ -1561,9 +1569,12 @@ bool OC_CALL OCByteStringCopy(OCByteString* dest, const OCByteString* source)
     {
         OICFree(dest->bytes);
     }
-    dest->bytes = (uint8_t*)OICMalloc(source->len * sizeof(uint8_t));
-    VERIFY_PARAM_NON_NULL(TAG, dest->bytes, "Failed allocating memory");
-    memcpy(dest->bytes, source->bytes, source->len * sizeof(uint8_t));
+    if (source->len)
+    {
+        dest->bytes = (uint8_t*)OICMalloc(source->len * sizeof(uint8_t));
+        VERIFY_PARAM_NON_NULL(TAG, dest->bytes, "Failed allocating memory");
+        memcpy(dest->bytes, source->bytes, source->len * sizeof(uint8_t));
+    }
     dest->len = source->len;
     return true;
 
index b77dac5..629b055 100644 (file)
@@ -771,8 +771,8 @@ static int64_t OCConvertArrayItem(CborEncoder *array, const OCRepPayloadValueArr
             }
             break;
         case OCREP_PROP_BYTE_STRING:
-            err |= (!valArray->ocByteStrArray[index].len) ? cbor_encode_null(array) : cbor_encode_byte_string(array,
-                valArray->ocByteStrArray[index].bytes, valArray->ocByteStrArray[index].len);
+            err |= cbor_encode_byte_string(array, valArray->ocByteStrArray[index].bytes,
+                    valArray->ocByteStrArray[index].len);
             break;
         case OCREP_PROP_OBJECT:
             if (valArray->objArray != 0)
index 0d00426..1ac75b6 100644 (file)
@@ -175,8 +175,7 @@ TEST_F(CborByteStringTest, ByteStringArraySetGetTest )
     OICFree(quakedata_out);
 }
 
-
-TEST_F(CborByteStringTest, ByteStringArrayConvertParseTest )
+TEST_F(CborByteStringTest, ByteStringArrayConvertParseTest)
 {
     OCRepPayloadSetUri(payload_in, "/a/quake_sensor");
     OCRepPayloadSetPropInt(payload_in, "scale", 4);
@@ -234,6 +233,98 @@ TEST_F(CborByteStringTest, ByteStringArrayConvertParseTest )
     OCPayloadDestroy((OCPayload*)payload_out);
 }
 
+TEST_F(CborByteStringTest, EmptyByteStringConvertParseTest)
+{
+    OCByteString bytestring_in = {NULL, 0};
+
+    // Set ByteString in Payload
+    EXPECT_EQ(true, OCRepPayloadSetPropByteString(payload_in, "bytestring", bytestring_in));
+
+    // Convert OCPayload to CBOR
+    uint8_t *payload_cbor = NULL;
+    size_t payload_cbor_size = 0;
+    EXPECT_EQ(OC_STACK_OK, OCConvertPayload((OCPayload*) payload_in, OC_FORMAT_CBOR,
+            &payload_cbor, &payload_cbor_size));
+
+#ifdef CBOR_BIN_STRING_DEBUG
+    FILE *fp = fopen("emptybinstring.cbor", "wb+");
+    if (fp)
+    {
+        fwrite(payload_cbor, 1, payload_cbor_size, fp);
+        fclose(fp);
+    }
+#endif //CBOR_BIN_STRING_DEBUG
+
+    // Parse CBOR back to OCPayload
+    OCPayload* payload_out = NULL;
+    EXPECT_EQ(OC_STACK_OK, OCParsePayload(&payload_out, OC_FORMAT_CBOR, PAYLOAD_TYPE_REPRESENTATION,
+                 payload_cbor, payload_cbor_size));
+
+    OCByteString bytestring_out = {NULL, 0};
+    ASSERT_EQ(true, OCRepPayloadGetPropByteString((OCRepPayload*)payload_out, "bytestring", &bytestring_out));
+
+    // Compare input and output data
+    ASSERT_EQ((uint8_t*)NULL, bytestring_out.bytes);
+    EXPECT_EQ(bytestring_in.len, bytestring_out.len);
+
+    // Cleanup
+    OICFree(payload_cbor);
+    OICFree(bytestring_out.bytes);
+    OCPayloadDestroy((OCPayload*)payload_out);
+}
+
+TEST_F(CborByteStringTest, EmptyByteStringArrayConvertParseTest)
+{
+    size_t dimensions_in[MAX_REP_ARRAY_DEPTH] = { 3, 0, 0};
+
+    OCByteString bytestring_in[3] = {{NULL, 0},
+                                    {NULL, 0},
+                                    {NULL, 0}};
+
+    EXPECT_EQ(true, OCRepPayloadSetByteStringArray(payload_in, "bytestring",
+                bytestring_in, dimensions_in));
+
+    // Convert OCPayload to CBOR
+    uint8_t *payload_cbor = NULL;
+    size_t payload_cbor_size = 0;
+    EXPECT_EQ(OC_STACK_OK, OCConvertPayload((OCPayload*) payload_in, OC_FORMAT_CBOR,
+            &payload_cbor, &payload_cbor_size));
+#ifdef CBOR_BIN_STRING_DEBUG
+    FILE *fp = fopen("emptybinstringarr.cbor", "wb+");
+    if (fp)
+    {
+        fwrite(payload_cbor, 1, payload_cbor_size, fp);
+        fclose(fp);
+    }
+#endif //CBOR_BIN_STRING_DEBUG
+
+    // Parse CBOR back to OCPayload
+    OCPayload* payload_out = NULL;
+    EXPECT_EQ(OC_STACK_OK, OCParsePayload(&payload_out, OC_FORMAT_CBOR, PAYLOAD_TYPE_REPRESENTATION,
+                payload_cbor, payload_cbor_size));
+
+    OCByteString* bytestring_out = NULL;
+    size_t dimensions_out[MAX_REP_ARRAY_DEPTH] = {0};
+    ASSERT_EQ(true, OCRepPayloadGetByteStringArray((OCRepPayload*)payload_out, "bytestring",
+                &bytestring_out, dimensions_out));
+
+    for(size_t i = 0; i < dimensions_in[0]; i++)
+    {
+        EXPECT_EQ(bytestring_in[i].len, bytestring_out[i].len);
+        EXPECT_EQ(0, memcmp(bytestring_in[i].bytes, bytestring_out[i].bytes, bytestring_in[i].len));
+    }
+
+    // Cleanup
+    OICFree(payload_cbor);
+    for(size_t i = 0; i < dimensions_out[0]; i++)
+    {
+        OICFree(bytestring_out[i].bytes);
+    }
+    OICFree(bytestring_out);
+
+    OCPayloadDestroy((OCPayload*)payload_out);
+}
+
 TEST(CborHeterogeneousArrayTest, ConvertParseTest)
 {
     OCRepPayload *arr = OCRepPayloadCreate();
index 1435307..da4ee3d 100644 (file)
@@ -2437,6 +2437,45 @@ TEST(StackPayload, CloneByteString)
     OCRepPayloadDestroy(clone);
 }
 
+TEST(StackPayload, EmptyByteString)
+{
+    OCByteString value = { NULL, 0 };
+
+    OCByteString dest = { NULL, 0 };
+    EXPECT_TRUE(OCByteStringCopy(&dest, &value));
+    EXPECT_EQ(0, memcmp(&dest, &value, sizeof(OCByteString)));
+
+    OCRepPayload *payload = OCRepPayloadCreate();
+    ASSERT_TRUE(payload != NULL);
+
+    EXPECT_TRUE(OCRepPayloadSetPropByteString(payload, "value", value));
+    EXPECT_TRUE(OCRepPayloadGetPropByteString(payload, "value", &dest));
+    EXPECT_EQ(0, memcmp(&dest, &value, sizeof(OCByteString)));
+
+    OCByteString array[] = {
+        { NULL, 0 },
+        { NULL, 0 }
+    };
+    size_t dim[MAX_REP_ARRAY_DEPTH] = { 2, 0, 0 };
+    EXPECT_TRUE(OCRepPayloadSetByteStringArray(payload, "array", array, dim));
+    OCByteString *destArray = NULL;
+    size_t destDim[MAX_REP_ARRAY_DEPTH] = { 0 };
+    EXPECT_TRUE(OCRepPayloadGetByteStringArray(payload, "array", &destArray, destDim));
+    EXPECT_EQ(0, memcmp(destDim, dim, sizeof(destDim)));
+    size_t dimTotal = calcDimTotal(dim);
+    for (size_t i = 0; i < dimTotal; ++i)
+    {
+        EXPECT_EQ(0, memcmp(&destArray[i], &array[i], sizeof(OCByteString)));
+    }
+
+    for(size_t i = 0; i < dimTotal; i++)
+    {
+        OICFree(destArray[i].bytes);
+    }
+    OICFree(destArray);
+    OCRepPayloadDestroy(payload);
+}
+
 TEST(StackUri, Rfc6874_Noop_1)
 {
     char validIPv6Address[] = "FF01:0:0:0:0:0:0:FB";