From 8939bac9ef01afbcb8ffcd60373095e60064ef9c Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 24 Mar 2015 14:16:13 -0700 Subject: [PATCH] Fixed memory handling of HandleSingleResponse As identified in the RI code review, the options and token were leaked and had some interesting behavior if the options count was zero. This patch corrects those, and repairs some return values that were inconsistent and detected while chaising the memory around. Change-Id: Ica590fefc8702faa7108cc00642cbc2424f402ac Signed-off-by: Erich Keane Reviewed-on: https://gerrit.iotivity.org/gerrit/564 Tested-by: jenkins-iotivity Reviewed-by: Joseph Morrow --- resource/csdk/stack/src/ocserverrequest.c | 75 +++++++++++++++++-------------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c index 6c74095..30653d5 100644 --- a/resource/csdk/stack/src/ocserverrequest.c +++ b/resource/csdk/stack/src/ocserverrequest.c @@ -291,7 +291,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) OCStackResult result = OC_STACK_ERROR; CARemoteEndpoint_t responseEndpoint = {}; CAResponseInfo_t responseInfo = {}; - CAHeaderOption_t* optionsPointer; + CAHeaderOption_t* optionsPointer = NULL; OC_LOG_V(INFO, TAG, "Inside HandleSingleResponse: %s", ehResponse->payload); @@ -328,12 +328,8 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) break; } responseInfo.info.type = qualityOfServiceToMessageType(serverRequest->qos); - responseInfo.info.token = (CAToken_t) OCCalloc(1, CA_MAX_TOKEN_LEN + 1); - if (!responseInfo.info.token) - { - OC_LOG(FATAL, TAG, "Response Info Token is NULL"); - return result; - } + char token[CA_MAX_TOKEN_LEN + 1] = {}; + responseInfo.info.token = token; memcpy(responseInfo.info.token, serverRequest->requestToken, CA_MAX_TOKEN_LEN); if(serverRequest->observeResult == OC_STACK_OK) @@ -345,38 +341,48 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) responseInfo.info.numOptions = ehResponse->numSendVendorSpecificHeaderOptions; } - responseInfo.info.options = (CAHeaderOption_t *) - OCMalloc(sizeof(CAHeaderOption_t) * responseInfo.info.numOptions); + if(responseInfo.info.numOptions > 0) + { + responseInfo.info.options = (CAHeaderOption_t *) + OCCalloc(responseInfo.info.numOptions, + sizeof(CAHeaderOption_t)); - optionsPointer = responseInfo.info.options; + if(!responseInfo.info.options) + { + OC_LOG(FATAL, TAG, PCF("options is NULL")); + return OC_STACK_NO_MEMORY; + } - // TODO: This exposes CoAP specific details. At some point, this should be - // re-factored and handled in the CA layer. - if(serverRequest->observeResult == OC_STACK_OK) - { - 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)); - - // Point to the next header option before copying vender specific header options - optionsPointer += 1; - } + optionsPointer = responseInfo.info.options; - if (ehResponse->numSendVendorSpecificHeaderOptions) - { - memcpy(optionsPointer, ehResponse->sendVendorSpecificHeaderOptions, - sizeof(OCHeaderOption) * ehResponse->numSendVendorSpecificHeaderOptions); - } + // TODO: This exposes CoAP specific details. At some point, this should be + // re-factored and handled in the CA layer. + if(serverRequest->observeResult == OC_STACK_OK) + { + 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)); + + // Point to the next header option before copying vender specific header options + optionsPointer += 1; + } - // Allocate memory for the payload. - char *payload = (char *)OCCalloc(1, MAX_RESPONSE_LENGTH); - if(!payload) + if (ehResponse->numSendVendorSpecificHeaderOptions) + { + memcpy(optionsPointer, ehResponse->sendVendorSpecificHeaderOptions, + sizeof(OCHeaderOption) * + ehResponse->numSendVendorSpecificHeaderOptions); + } + } + else { - return OC_STACK_NO_MEMORY; + responseInfo.info.options = NULL; } + char payload[MAX_RESPONSE_LENGTH] = {}; + // Put the JSON prefix and suffix around the payload strcpy(payload, (const char *)OC_JSON_PREFIX); strcat(payload, (const char *)ehResponse->payload); @@ -404,7 +410,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) if(caResult != CA_STATUS_OK) { OC_LOG_V(ERROR, TAG, "CASendResponse failed on %s", connTypes[i]); - result = OC_STACK_ERROR; + result = CAResultToOCResult(caResult); } else { @@ -417,6 +423,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) if(caResult != CA_STATUS_OK) { OC_LOG(ERROR, TAG, PCF("CASendResponse failed")); + result = CAResultToOCResult(caResult); } else { @@ -424,7 +431,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) } #endif - OCFree(payload); + OCFree(responseInfo.info.options); //Delete the request FindAndDeleteServerRequest(serverRequest); return result; -- 2.7.4