From dd718cdfa823418f66c8581cd3b9592276a9fb0b Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 9 Feb 2015 17:29:14 -0800 Subject: [PATCH] Clean up TODOs in OCUtilities The Escape String mechanism is no longer used anywhere, so it is dead code. The QueryParam parsing was only used in 1 place and the initial non 'temp patch' function was all dead code. This fix removes both sets of dead code and cleans up the temp-patch code to be a little more efficient. Change-Id: I0777f5d5795362195e35bdffc82a6eed55850a91 Signed-off-by: Erich Keane Reviewed-on: https://gerrit.iotivity.org/gerrit/320 Tested-by: jenkins-iotivity Reviewed-by: Patrick Lankswert --- resource/include/OCUtilities.h | 18 ++--- resource/src/InProcServerWrapper.cpp | 6 +- resource/src/OCUtilities.cpp | 132 +++-------------------------------- 3 files changed, 19 insertions(+), 137 deletions(-) diff --git a/resource/include/OCUtilities.h b/resource/include/OCUtilities.h index 1c083df..82cccd9 100644 --- a/resource/include/OCUtilities.h +++ b/resource/include/OCUtilities.h @@ -35,20 +35,14 @@ namespace OC { typedef std::map QueryParamsKeyVal; /* - * @brief Helper function to get query parameter from a URI - * @remarks Its okay to return a copy of the container.\ - * The size is not expected to be huge. - * @remarks Temporary: The URI must strictly have\ - * coap as the protocol in the fully qualified URI\ - * e.g., coap://1.2.3.4:5657/foo?bar=0) - * @remarks If a separate class for URI parser is needed,\ - * please talk to Erich Keane. - * @todo If more URI elements need to be parsed,\ - * please move the common parsing logic to a - * different function + * @brief helper function that parses the query parameters component + * of a URI into a key-value map. This function expects the uri + * parameter to contain the query parameters component of a URI + * (everything after the '?', excluding anything anchors). + * + * Note that output will not perform URL decoding */ QueryParamsKeyVal getQueryParams(const std::string& uri); - } } diff --git a/resource/src/InProcServerWrapper.cpp b/resource/src/InProcServerWrapper.cpp index d60f613..8b48b37 100644 --- a/resource/src/InProcServerWrapper.cpp +++ b/resource/src/InProcServerWrapper.cpp @@ -70,9 +70,9 @@ void formResourceRequest(OCEntityHandlerFlag flag, { if(entityHandlerRequest->query) { - std::string querystr(reinterpret_cast(entityHandlerRequest->query)); - - OC::Utilities::QueryParamsKeyVal qp = OC::Utilities::getQueryParams(querystr); + OC::Utilities::QueryParamsKeyVal qp = + OC::Utilities::getQueryParams( + reinterpret_cast(entityHandlerRequest->query)); if(qp.size() > 0) { diff --git a/resource/src/OCUtilities.cpp b/resource/src/OCUtilities.cpp index 530f673..93d61b3 100644 --- a/resource/src/OCUtilities.cpp +++ b/resource/src/OCUtilities.cpp @@ -19,151 +19,39 @@ //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= #include - -#include "OCUtilities.h" +#include #include - #include #include #include -extern "C" { -#include // libcoap -#include // libcoap -} - -namespace OC { - - // Helper function to escape special character. - std::string escapeString(const std::string& value) - { - std::ostringstream stringStream; - for (const char& c : value) - { - switch (c) - { - case '\\': stringStream << "\\\\"; - break; - case '"': stringStream << "\\\""; - break; - case '/': stringStream << "\\/"; - break; - case '\b': stringStream << "\\b"; - break; - case '\f': stringStream << "\\f"; - break; - case '\n': stringStream << "\\n"; - break; - case '\r': stringStream << "\\r"; - break; - case '\t': stringStream << "\\t"; - break; - default: stringStream << c; - break; - } - } - return stringStream.str(); - } -} -// [TODO] remove this function -// it seems that the C stack is parsing and giving out the query separately. -// the entire URI need not be parsed -static OC::Utilities::QueryParamsKeyVal tempPatch(const std::string& _uri) +OC::Utilities::QueryParamsKeyVal OC::Utilities::getQueryParams(const std::string& uri) { OC::Utilities::QueryParamsKeyVal qp; - if(_uri.empty()) + if(uri.empty()) { return qp; } std::vector queryparams; - boost::split(queryparams, _uri, boost::is_any_of("&")); + boost::split(queryparams, uri, [](const char c){return c=='&';}, + boost::token_compress_on); for(std::string& it: queryparams) { - std::vector keyval; - boost::split(keyval, it, boost::is_any_of("=")); - if(2 == keyval.size()) - { - qp[keyval.at(0)] = keyval.at(1); - } - } - - return qp; -} - -// implementation can be split into two functions if needed -// uses do{}while(0) to avoid returning from multiple locations -OC::Utilities::QueryParamsKeyVal OC::Utilities::getQueryParams(const std::string& _uri) -{ - - // this is a temporary fix. [TODO] remove this after permanent fix - return tempPatch(_uri); - - OC::Utilities::QueryParamsKeyVal qp; - unsigned char *bufptr = nullptr; // don't delete via bufptr - unsigned char *bufptrToDelete = nullptr; // bufptr may be incremented. need this one to keep track. - do // while(0) - { - if(_uri.empty()) - { - break; - } - - coap_uri_t coapuri = {{0}}; - unsigned char* uristr = reinterpret_cast(const_cast(_uri.c_str())); + auto index = it.find('='); - if(coap_split_uri(uristr, _uri.length(), &coapuri) < 0) + if(index == std::string::npos) { - break; + qp[it] = ""; } - - size_t buflen = 2048; // this is big enough buffer. [TODO] may want to downsize it. I have seen that the size may have to be greater than coap.query.length, which is counterintuitve but there may be a bug in coap uri parser. - bufptrToDelete = bufptr = new (std::nothrow) unsigned char[buflen](); // why heap? will need it for incrementing the pointer in the logic below - - if(!bufptr) + else { - break; - } - - int segments = -1; - if((segments = coap_split_query(coapuri.query.s, coapuri.query.length, bufptr, &buflen)) < 0) - { - break; - } - - // coap uri parser has weird api. its not straighforward to understand what the coap function calls below do. - // coap uri parser lacks ability to split the key value pair in query params. that will be done in getQueryParams() function - std::vector queryparams; - while(segments--) - { - queryparams.push_back(std::string (reinterpret_cast(coap_opt_value(bufptr)), coap_opt_length(bufptr))); - bufptr += coap_opt_size(bufptr); - } - - if(queryparams.empty()) - { - break; - } - - //[TODO] use foreach - for(std::string& it : queryparams) - { - std::vector keyval; - boost::split(keyval, it, boost::is_any_of("=")); - if(2 == keyval.size()) - { - qp[keyval.at(0)] = keyval.at(1); - } + qp[it.substr(0, index)] = it.substr(index + 1); } } - while(0); - if(bufptrToDelete) - { - delete [] bufptrToDelete; - } return qp; } -- 2.7.4