Fixed problems in URI parsing including memory corruption.

Fixed issues in CAParseURI that included

* Use of uint32_t instead of size_t.
  - Passing by address resulted in corruption on 64-bit builds and all
    others where the datatypes are different. This includes various
    embedded devices that have a size_t smaller than uint32_t.

* Miscalculation of offset would freeze parsing once a few path components
  or parameters were processed.
  - While walking parsed results list the buffer-check addition would fail
    and all subsequent path components or parameters (depending on each
    independently) would repeat the contents of a prior item. Thus the end
    of path components and/or parameter parts of the list would be filled
    with repeats of a single value instead of the correct latter values.

* Replaced duplicated code blocks with a single helper function.

* Added unit tests for issue. However the test cases won't be runnable
  until after the build machine issue with connectivity/test is addressed.

Change-Id: Ife3f327ae3bbda518d663bb0d6c5b4c0d8f98e9a
Signed-off-by: Jon A. Cruz <jonc@osg.samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/470
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: jihwan seo <jihwan.seo@samsung.com>
Reviewed-by: Joseph Morrow <joseph.l.morrow@intel.com>
Reviewed-by: Erich Keane <erich.keane@intel.com>

diff --git a/resource/csdk/connectivity/src/caprotocolmessage.c b/resource/csdk/connectivity/src/caprotocolmessage.c
index 07ef6fd..fbbe5d8 100644 (file)
--- a/resource/csdk/connectivity/src/caprotocolmessage.c
+++ b/resource/csdk/connectivity/src/caprotocolmessage.c
@@ -35,6 +35,19 @@
 static const char COAP_HEADER[] = "coap://[::]/";
 static uint32_t SEED = 0;
 
+/**
+ * Helper that uses libcoap to parse either the path or the parameters of a URI
+ * and populate the supplied options list.
+ *
+ * @param str the input partial URI string (either path or query).
+ * @param length the length of the supplied partial URI.
+ * @param target the part of the URI to parse (either COAP_OPTION_URI_PATH
+ *        or COAP_OPTION_URI_QUERY).
+ * @param optlist options information.
+ */
+static void CAParseUriPartial(const unsigned char *str, size_t length,
+                              int target, coap_list_t **optlist);
+
 void CAGetRequestInfoFromPdu(const coap_pdu_t *pdu, CARequestInfo_t *outReqInfo, char *outUri,
                              uint32_t buflen)
 {
@@ -220,9 +233,9 @@ void CAParseURI(const char *uriInfo, coap_list_t **optlist)
     coap_uri_t uri;
     coap_split_uri((unsigned char *) uriInfo, strlen(uriInfo), &uri);
 
-    unsigned char portbuf[2];
     if (uri.port != COAP_DEFAULT_PORT)
     {
+        unsigned char portbuf[2];
         coap_insert(
                 optlist,
                 CACreateNewOptionNode(COAP_OPTION_URI_PORT,
@@ -230,68 +243,55 @@ void CAParseURI(const char *uriInfo, coap_list_t **optlist)
                 CAOrderOpts);
     }
 
-    unsigned char uriBuffer[CA_BUFSIZE] =
-    { 0 };
-    unsigned char *pBuf = uriBuffer;
-    uint32_t buflen;
-    int32_t res;
 
-    if (uri.path.length)
-    {
-        buflen = CA_BUFSIZE;
-        res = coap_split_path(uri.path.s, uri.path.length, pBuf, &buflen);
+    CAParseUriPartial(uri.path.s, uri.path.length, COAP_OPTION_URI_PATH, optlist);
 
-        if (res > 0)
-        {
-            uint32_t prevIdx = 0;
-            while (res--)
-            {
-                coap_insert(
-                        optlist,
-                        CACreateNewOptionNode(COAP_OPTION_URI_PATH, COAP_OPT_LENGTH(pBuf),
-                                              COAP_OPT_VALUE(pBuf)),
-                        CAOrderOpts);
+    CAParseUriPartial(uri.query.s, uri.query.length, COAP_OPTION_URI_QUERY, optlist);
 
-                uint32_t optSize = COAP_OPT_SIZE(pBuf);
-                uint32_t nextIdx = prevIdx + optSize;
-                if (nextIdx < buflen)
-                {
-                    pBuf += optSize;
-                    prevIdx += nextIdx;
-                }
-            }
-        }
-    }
+    OIC_LOG(DEBUG, TAG, "CAParseURI OUT");
+}
 
-    if (uri.query.length)
+void CAParseUriPartial(const unsigned char *str, size_t length,
+                       int target, coap_list_t **optlist)
+{
+    if ((target != COAP_OPTION_URI_PATH) && (target != COAP_OPTION_URI_QUERY))
     {
-        buflen = CA_BUFSIZE;
-        pBuf = uriBuffer;
-        res = coap_split_query(uri.query.s, uri.query.length, pBuf, &buflen);
+        // should never occur. Log just in case.
+        OIC_LOG(DEBUG, TAG, "Unexpected URI component.");
+    }
+    else if (str && length)
+    {
+        unsigned char uriBuffer[CA_BUFSIZE] = { 0 };
+        unsigned char *pBuf = uriBuffer;
+        size_t buflen = sizeof(uriBuffer);
+        int res = (target == COAP_OPTION_URI_PATH) ?
+            coap_split_path(str, length, pBuf, &buflen) :
+            coap_split_query(str, length, pBuf, &buflen);
 
         if (res > 0)
         {
-            uint32_t prevIdx = 0;
+            size_t prevIdx = 0;
             while (res--)
             {
                 coap_insert(
                         optlist,
-                        CACreateNewOptionNode(COAP_OPTION_URI_QUERY, COAP_OPT_LENGTH(pBuf),
+                        CACreateNewOptionNode(target, COAP_OPT_LENGTH(pBuf),
                                               COAP_OPT_VALUE(pBuf)),
                         CAOrderOpts);
 
-                uint32_t optSize = COAP_OPT_SIZE(pBuf);
-                uint32_t nextIdx = prevIdx + optSize;
-                if (nextIdx < buflen)
+                size_t optSize = COAP_OPT_SIZE(pBuf);
+                if ((prevIdx + optSize) < buflen)
                 {
                     pBuf += optSize;
-                    prevIdx += nextIdx;
+                    prevIdx += optSize;
                 }
             }
         }
+        else
+        {
+            OIC_LOG_V(DEBUG, TAG, "Problem parsing URI : %d for %d", res, target);
+        }
     }
-
-    OIC_LOG(DEBUG, TAG, "CAParseURI OUT");
 }
 
 void CAParseHeadOption(const uint32_t code, const CAInfo_t info, coap_list_t **optlist)

diff --git a/resource/csdk/connectivity/test/caprotocolmessagetest.cpp b/resource/csdk/connectivity/test/caprotocolmessagetest.cpp
new file mode 100644 (file)
index 0000000..12d6701
--- /dev/null
+++ b/resource/csdk/connectivity/test/caprotocolmessagetest.cpp
@@ -0,0 +1,187 @@
+//******************************************************************
+//
+// Copyright 2015 Samsung Electronics All Rights Reserved.
+//
+//-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+//-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
+
+#include <stdio.h>
+
+#include "gtest/gtest.h"
+
+#include "caprotocolmessage.h"
+
+namespace {
+
+static const char COAP_HEADER[] = "coap://[::]/";
+
+class CoAPOptionCase
+{
+public:
+    unsigned short key;
+    unsigned int length;
+    std::string dataStr; // data could be binary... for testing we'll use str
+};
+
+
+
+/**
+ * Helper to validate the state of CoAP URI parsing.
+ *
+ * @param cases array of expected parse results.
+ * @param numCases number of expected parse results.
+ * @param optlist parsed option list to verify.
+ */
+void verifyParsedOptions(CoAPOptionCase const *cases,
+                        size_t numCases,
+                        coap_list_t *optlist)
+{
+    size_t index = 0;
+    for (coap_list_t *opt = optlist; opt; opt = opt->next)
+    {
+        coap_option *option = (coap_option *) opt->data;
+        EXPECT_TRUE(option != NULL);
+        EXPECT_LT(index, numCases);
+        if (option && (index < numCases))
+        {
+            unsigned short key = COAP_OPTION_KEY(*option);
+            unsigned int length = COAP_OPTION_LENGTH(*option);
+            std::string dataStr((const char*)COAP_OPTION_DATA(*option), length);
+            // First validate the test case:
+            EXPECT_EQ(cases[index].length, cases[index].dataStr.length());
+
+            // Ensure data matches expected parsing
+            EXPECT_EQ(cases[index].key, key);
+            EXPECT_EQ(cases[index].length, length);
+            EXPECT_EQ(cases[index].dataStr, dataStr);
+        }
+
+        index++;
+    }
+    // Ensure we saw the proper number of parts:
+    EXPECT_EQ(numCases, index);
+}
+
+} // namespace
+
+TEST(CAProtocolMessage, CAParseURIBase)
+{
+    char sampleURI[] = "coap://[::]/oc/core?rt=core.sensor&if=core.mi.ll";
+
+    CoAPOptionCase cases[] = {
+        {COAP_OPTION_URI_PATH, 2, "oc"},
+        {COAP_OPTION_URI_PATH, 4, "core"},
+        {COAP_OPTION_URI_QUERY, 14, "rt=core.sensor"},
+        {COAP_OPTION_URI_QUERY, 13, "if=core.mi.ll"},
+    };
+    size_t numCases = sizeof(cases) / sizeof(cases[0]);
+
+
+    coap_list_t *optlist = NULL;
+    CAParseURI(sampleURI, &optlist);
+
+
+    verifyParsedOptions(cases, numCases, optlist);
+}
+
+// Try for multiple URI path components that still total less than 128
+TEST(CAProtocolMessage, CAParseURIManyPath)
+{
+    char sampleURI[] = "coap://[::]"
+        "/medium/a/b/c/d/e/f/g/h/i/j/"
+        "?rt=core.sensor&if=core.mi.ll";
+
+    CoAPOptionCase cases[] = {
+        {COAP_OPTION_URI_PATH, 6, "medium"},
+        {COAP_OPTION_URI_PATH, 1, "a"},
+        {COAP_OPTION_URI_PATH, 1, "b"},
+        {COAP_OPTION_URI_PATH, 1, "c"},
+        {COAP_OPTION_URI_PATH, 1, "d"},
+        {COAP_OPTION_URI_PATH, 1, "e"},
+        {COAP_OPTION_URI_PATH, 1, "f"},
+        {COAP_OPTION_URI_PATH, 1, "g"},
+        {COAP_OPTION_URI_PATH, 1, "h"},
+        {COAP_OPTION_URI_PATH, 1, "i"},
+        {COAP_OPTION_URI_PATH, 1, "j"},
+        {COAP_OPTION_URI_QUERY, 14, "rt=core.sensor"},
+        {COAP_OPTION_URI_QUERY, 13, "if=core.mi.ll"},
+    };
+    size_t numCases = sizeof(cases) / sizeof(cases[0]);
+
+
+    coap_list_t *optlist = NULL;
+    CAParseURI(sampleURI, &optlist);
+
+
+    verifyParsedOptions(cases, numCases, optlist);
+}
+
+// Try for multiple URI parameters that still total less than 128
+TEST(CAProtocolMessage, CAParseURIManyParams)
+{
+    char sampleURI[] = "coap://[::]/oc/core/"
+        "?rt=core.sensor&a=0&b=1&c=2&d=3&e=4&f=5&g=6&h=7&i=8&j=9";
+
+    CoAPOptionCase cases[] = {
+        {COAP_OPTION_URI_PATH, 2, "oc"},
+        {COAP_OPTION_URI_PATH, 4, "core"},
+        {COAP_OPTION_URI_QUERY, 14, "rt=core.sensor"},
+        {COAP_OPTION_URI_QUERY, 3, "a=0"},
+        {COAP_OPTION_URI_QUERY, 3, "b=1"},
+        {COAP_OPTION_URI_QUERY, 3, "c=2"},
+        {COAP_OPTION_URI_QUERY, 3, "d=3"},
+        {COAP_OPTION_URI_QUERY, 3, "e=4"},
+        {COAP_OPTION_URI_QUERY, 3, "f=5"},
+        {COAP_OPTION_URI_QUERY, 3, "g=6"},
+        {COAP_OPTION_URI_QUERY, 3, "h=7"},
+        {COAP_OPTION_URI_QUERY, 3, "i=8"},
+        {COAP_OPTION_URI_QUERY, 3, "j=9"},
+    };
+    size_t numCases = sizeof(cases) / sizeof(cases[0]);
+
+
+    coap_list_t *optlist = NULL;
+    CAParseURI(sampleURI, &optlist);
+
+
+    verifyParsedOptions(cases, numCases, optlist);
+}
+
+// Test that an initial long path component won't hide latter ones.
+TEST(CAProtocolMessage, CAParseURILongPath)
+{
+    char sampleURI[] = "coap://[::]/oc"
+        "123456789012345678901234567890123456789012345678901234567890"
+        "12345678901234567890123456789012345678901234567890"
+        "/core?rt=core.sensor&if=core.mi.ll";
+
+    CoAPOptionCase cases[] = {
+        {COAP_OPTION_URI_PATH, 112, "oc"
+        "123456789012345678901234567890123456789012345678901234567890"
+        "12345678901234567890123456789012345678901234567890"},
+        {COAP_OPTION_URI_PATH, 4, "core"},
+        {COAP_OPTION_URI_QUERY, 14, "rt=core.sensor"},
+        {COAP_OPTION_URI_QUERY, 13, "if=core.mi.ll"},
+    };
+    size_t numCases = sizeof(cases) / sizeof(cases[0]);
+
+
+    coap_list_t *optlist = NULL;
+    CAParseURI(sampleURI, &optlist);
+
+
+    verifyParsedOptions(cases, numCases, optlist);
+}