From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web10.1351.1603909662269087177 for ; Wed, 28 Oct 2020 11:27:42 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.88, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: oaJP88K47FN84IhbryKe/t0mrkYET/kkyOHRQ1WbmN8uTqOKDz9fsOg+TdW9LMOP3zGmpv9+EV gOMmx+mF1trg== X-IronPort-AV: E=McAfee;i="6000,8403,9788"; a="186080205" X-IronPort-AV: E=Sophos;i="5.77,427,1596524400"; d="scan'208";a="186080205" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Oct 2020 11:27:41 -0700 IronPort-SDR: Bb8LzXLvoGJ8Mv0ujWRNw1Zus9EhytzsZDCyM7F7MH529S7TGG7KSz+vqKeV6Hmu6+AZltJN+K ku6f3u5Bg9wg== X-IronPort-AV: E=Sophos;i="5.77,427,1596524400"; d="scan'208";a="536339513" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.249.42.246]) ([10.249.42.246]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Oct 2020 11:27:38 -0700 Subject: Re: [edk2-devel] [DxeHttpIoLib PATCH V2 1/3] NetworkPkg/Library: Implementation of Http IO Helper Library To: devel@edk2.groups.io, abner.chang@hpe.com Cc: Jiaxin Wu , Siyuan Fu , "Wang, Nickle (HPS SW)" References: <20201020023848.3015-1-abner.chang@hpe.com> <20201020023848.3015-2-abner.chang@hpe.com> <036e09c1-d1c8-8d8b-44fb-7f252ccf1bad@linux.intel.com> From: "Maciej Rabeda" Message-ID: <909b9d7b-9018-fe0a-a9f7-f6477403f84d@linux.intel.com> Date: Wed, 28 Oct 2020 19:27:33 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: pl Thanks for patching spelling problems in MdePkg header. I will send more comments on v3. Added one here regarding chunk transfer security. Thanks, Maciej On 26-Oct-20 07:53, Abner Chang wrote: > Thanks for reviewing this Maciej. > > Most of comments are addressed. I also submitted a BZ for the wrong spelling of CHUNK_TRNASFER_*, the patch is already sent. > V3 of this patch set is also sent to the mailing list. > > Other feedbacks are in below, > >> -----Original Message----- >> From: Rabeda, Maciej [mailto:maciej.rabeda@linux.intel.com] >> Sent: Saturday, October 24, 2020 12:25 AM >> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) >> >> Cc: Jiaxin Wu ; Siyuan Fu ; >> Wang, Nickle (HPS SW) >> Subject: Re: [edk2-devel] [DxeHttpIoLib PATCH V2 1/3] NetworkPkg/Library: >> Implementation of Http IO Helper Library >> >> Hi Abner, >> >> I understand the concept of moving HTTP I/O layer outside HttpBootDxe and >> into a separate library and I approve it. >> However, there are some things that have to addressed/fixed/explained >> here. >> See comments within the patch. >> >> On 20-Oct-20 04:38, Abner Chang wrote: >>> Add HTTP IO helper library which could be sued by HTTP applications >>> such as HTTP Boot, Redfish HTTP REST EX driver instance and etc. >>> >>> Signed-off-by: Abner Chang >>> >>> Cc: Maciej Rabeda >>> Cc: Jiaxin Wu >>> Cc: Siyuan Fu >>> Cc: Nickle Wang >>> --- >>> NetworkPkg/Include/Library/HttpIoLib.h | 328 +++++++ >>> .../Library/DxeHttpIoLib/DxeHttpIoLib.c | 809 ++++++++++++++++++ >>> .../Library/DxeHttpIoLib/DxeHttpIoLib.inf | 43 + >>> .../Library/DxeHttpIoLib/DxeHttpIoLib.uni | 13 + >>> 4 files changed, 1193 insertions(+) >>> create mode 100644 NetworkPkg/Include/Library/HttpIoLib.h >>> create mode 100644 NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.c >>> create mode 100644 NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.inf >>> create mode 100644 NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.uni >>> >>> diff --git a/NetworkPkg/Include/Library/HttpIoLib.h >>> b/NetworkPkg/Include/Library/HttpIoLib.h >>> new file mode 100644 >>> index 0000000000..8f3804ca42 >>> --- /dev/null >>> +++ b/NetworkPkg/Include/Library/HttpIoLib.h >>> @@ -0,0 +1,328 @@ >>> +/** @file >>> + HttpIoLib.h. >>> + >>> +(C) Copyright 2020 Hewlett-Packard Development Company, L.P.
>>> +SPDX-License-Identifier: BSD-2-Clause-Patent >>> + >>> +**/ >>> + >>> +#ifndef HTTP_IO_LIB_H_ >>> +#define HTTP_IO_LIB_H_ >>> + >>> +#include >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +#define HTTP_IO_MAX_SEND_PAYLOAD 1024 >>> +#define HTTP_IO_CHUNK_SIZE_STRING_LEN 50 >>> +#define HTTP_IO_CHUNKED_TRANSFER_CODING_DATA_LENGTH 256 >>> + >>> +/// >>> +/// HTTP_IO_CALLBACK_EVENT >>> +/// >>> +typedef enum { >>> + HttpIoRequest, >>> + HttpIoResponse >>> +} HTTP_IO_CALLBACK_EVENT; >>> + >>> +/** >>> + HttpIo Callback function which will be invoked when specified >> HTTP_IO_CALLBACK_EVENT happened. >>> + >>> + @param[in] EventType Indicate the Event type that occurs in the >> current callback. >>> + @param[in] Message HTTP message which will be send to, or just >> received from HTTP server. >>> + @param[in] Context The Callback Context pointer. >>> + >>> + @retval EFI_SUCCESS Tells the HttpIo to continue the HTTP process. >>> + @retval Others Tells the HttpIo to abort the current HTTP process. >>> +**/ >>> +typedef >>> +EFI_STATUS >>> +(EFIAPI * HTTP_IO_CALLBACK) ( >>> + IN HTTP_IO_CALLBACK_EVENT EventType, >>> + IN EFI_HTTP_MESSAGE *Message, >>> + IN VOID *Context >>> + ); >>> + >>> +/// >>> +/// A wrapper structure to hold the received HTTP response data. >>> +/// >>> +typedef struct { >>> + EFI_HTTP_RESPONSE_DATA Response; >>> + UINTN HeaderCount; >>> + EFI_HTTP_HEADER *Headers; >>> + UINTN BodyLength; >>> + CHAR8 *Body; >>> + EFI_STATUS Status; >>> +} HTTP_IO_RESPONSE_DATA; >>> + >>> +/// >>> +/// HTTP_IO configuration data for IPv4 /// typedef struct { >>> + EFI_HTTP_VERSION HttpVersion; >>> + UINT32 RequestTimeOut; ///< In milliseconds. >>> + UINT32 ResponseTimeOut; ///< In milliseconds. >>> + BOOLEAN UseDefaultAddress; >>> + EFI_IPv4_ADDRESS LocalIp; >>> + EFI_IPv4_ADDRESS SubnetMask; >>> + UINT16 LocalPort; >>> +} HTTP4_IO_CONFIG_DATA; >>> + >>> +/// >>> +/// HTTP_IO configuration data for IPv6 /// typedef struct { >>> + EFI_HTTP_VERSION HttpVersion; >>> + UINT32 RequestTimeOut; ///< In milliseconds. >>> + BOOLEAN UseDefaultAddress; >>> + EFI_IPv6_ADDRESS LocalIp; >>> + UINT16 LocalPort; >>> +} HTTP6_IO_CONFIG_DATA; >>> + >>> +/// >>> +/// HTTP_IO configuration >>> +/// >>> +typedef union { >>> + HTTP4_IO_CONFIG_DATA Config4; >>> + HTTP6_IO_CONFIG_DATA Config6; >>> +} HTTP_IO_CONFIG_DATA; >>> + >>> +/// >>> +/// HTTP_IO wrapper of the EFI HTTP service. >>> +/// >>> +typedef struct { >>> + UINT8 IpVersion; >>> + EFI_HANDLE Image; >>> + EFI_HANDLE Controller; >>> + EFI_HANDLE Handle; >>> + >>> + EFI_HTTP_PROTOCOL *Http; >>> + >>> + HTTP_IO_CALLBACK Callback; >>> + VOID *Context; >>> + >>> + EFI_HTTP_TOKEN ReqToken; >>> + EFI_HTTP_MESSAGE ReqMessage; >>> + EFI_HTTP_TOKEN RspToken; >>> + EFI_HTTP_MESSAGE RspMessage; >>> + >>> + BOOLEAN IsTxDone; >>> + BOOLEAN IsRxDone; >>> + >>> + EFI_EVENT TimeoutEvent; >>> + UINT32 Timeout; >>> +} HTTP_IO; >>> + >>> +/// >>> +/// Process code of HTTP chunk transfer. >>> +/// >>> +typedef enum { >>> + HttpIoSendChunkNone = 0, >>> + HttpIoSendChunkHeaderZeroContent, >>> + HttpIoSendChunkContent, >>> + HttpIoSendChunkEndChunk, >>> + HttpIoSendChunkFinish >>> +} HTTP_IO_SEND_CHUNK_PROCESS; >>> + >>> +/// >>> +/// Process code of HTTP non chunk transfer. >>> +/// >>> +typedef enum { >>> + HttpIoSendNonChunkNone = 0, >>> + HttpIoSendNonChunkHeaderZeroContent, >>> + HttpIoSendNonChunkContent, >>> + HttpIoSendNonChunkFinish >>> +} HTTP_IO_SEND_NON_CHUNK_PROCESS; >>> + >>> +/// >>> +/// Chunk links for HTTP chunked transfer coding. >>> +/// >>> +typedef struct { >>> + LIST_ENTRY NextChunk; >>> + UINTN Length; >>> + CHAR8 *Data; >>> +} HTTP_IO_CHUNKS; >>> + >>> +/** >>> + Notify the callback function when an event is triggered. >>> + >>> + @param[in] Context The opaque parameter to the function. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +HttpIoNotifyDpc ( >>> + IN VOID *Context >>> + ); >>> + >>> +/** >>> + Request HttpIoNotifyDpc as a DPC at TPL_CALLBACK. >>> + >>> + @param[in] Event The event signaled. >>> + @param[in] Context The opaque parameter to the function. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +HttpIoNotify ( >>> + IN EFI_EVENT Event, >>> + IN VOID *Context >>> + ); >>> + >>> +/** >>> + Destroy the HTTP_IO and release the resources. >>> + >>> + @param[in] HttpIo The HTTP_IO which wraps the HTTP service to be >> destroyed. >>> + >>> +**/ >>> +VOID >>> +HttpIoDestroyIo ( >>> + IN HTTP_IO *HttpIo >>> + ); >>> + >>> +/** >>> + Create a HTTP_IO to access the HTTP service. It will create and >>> +configure >>> + a HTTP child handle. >>> + >>> + @param[in] Image The handle of the driver image. >>> + @param[in] Controller The handle of the controller. >>> + @param[in] IpVersion IP_VERSION_4 or IP_VERSION_6. >>> + @param[in] ConfigData The HTTP_IO configuration data. >>> + @param[in] Callback Callback function which will be invoked when >> specified >>> + HTTP_IO_CALLBACK_EVENT happened. >>> + @param[in] Context The Context data which will be passed to the >> Callback function. >>> + @param[out] HttpIo The HTTP_IO. >>> + >>> + @retval EFI_SUCCESS The HTTP_IO is created and configured. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_UNSUPPORTED One or more of the control options are >> not >>> + supported in the implementation. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval Others Failed to create the HTTP_IO or configure it. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoCreateIo ( >>> + IN EFI_HANDLE Image, >>> + IN EFI_HANDLE Controller, >>> + IN UINT8 IpVersion, >>> + IN HTTP_IO_CONFIG_DATA *ConfigData, >>> + IN HTTP_IO_CALLBACK Callback, >>> + IN VOID *Context, >>> + OUT HTTP_IO *HttpIo >>> + ); >>> + >>> +/** >>> + Synchronously send a HTTP REQUEST message to the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] Request A pointer to storage such data as URL and >> HTTP method. >>> + @param[in] HeaderCount Number of HTTP header structures in >> Headers list. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[in] BodyLength Length in bytes of the HTTP body. >>> + @param[in] Body Body associated with the HTTP request. >>> + >>> + @retval EFI_SUCCESS The HTTP request is transmitted. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_DEVICE_ERROR An unexpected network or system error >> occurred. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoSendRequest ( >>> + IN HTTP_IO *HttpIo, >>> + IN EFI_HTTP_REQUEST_DATA *Request, OPTIONAL >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, OPTIONAL >>> + IN UINTN BodyLength, >>> + IN VOID *Body OPTIONAL >>> + ); >>> + >>> +/** >>> + Synchronously receive a HTTP RESPONSE message from the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] RecvMsgHeader TRUE to receive a new HTTP response >> (from message header). >>> + FALSE to continue receive the previous response message. >>> + @param[out] ResponseData Point to a wrapper of the received >> response data. >>> + >>> + @retval EFI_SUCCESS The HTTP response is received. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_DEVICE_ERROR An unexpected network or system error >> occurred. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoRecvResponse ( >>> + IN HTTP_IO *HttpIo, >>> + IN BOOLEAN RecvMsgHeader, >>> + OUT HTTP_IO_RESPONSE_DATA *ResponseData >>> + ); >>> + >>> +/** >>> + Get the value of the content length if there is a "Content-Length" header. >>> + >>> + @param[in] HeaderCount Number of HTTP header structures in >> Headers. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[out] ContentLength Pointer to save the value of the content >> length. >>> + >>> + @retval EFI_SUCCESS Successfully get the content length. >>> + @retval EFI_NOT_FOUND No "Content-Length" header in the >> Headers. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoGetContentLength ( >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, >>> + OUT UINTN *ContentLength >>> + ); >>> + >>> +/** >>> + Synchronously receive a HTTP RESPONSE message from the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] HeaderCount Number of headers in Headers. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[out] ChunkListHead A pointer to receivce list head of chunked >> data. >>> + Caller has to release memory of ChunkListHead >>> + and all list entries. >>> + @param[out] ContentLength Total content length >>> + >>> + @retval EFI_SUCCESS The HTTP chunked transfer is received. >>> + @retval EFI_NOT_FOUND No chunked transfer coding header found. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_INVALID_PARAMETER Improper parameters. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoGetChunkedTransferContent ( >>> + IN HTTP_IO *HttpIo, >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, >>> + OUT LIST_ENTRY **ChunkListHead, >>> + OUT UINTN *ContentLength >>> + ); >>> + >>> +/** >>> + Send HTTP request in chunks. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] SendChunkProcess Pointer to current chunk process >> status. >>> + @param[out] RequestMessage Request to send. >>> + >>> + @retval EFI_SUCCESS Successfully to send chunk data according to >> SendChunkProcess. >>> + @retval Other Other errors. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoSendChunkedTransfer ( >>> + IN HTTP_IO *HttpIo, >>> + IN HTTP_IO_SEND_CHUNK_PROCESS *SendChunkProcess, >>> + IN EFI_HTTP_MESSAGE *RequestMessage >>> +); >>> +#endif >>> diff --git a/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.c >>> b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.c >>> new file mode 100644 >>> index 0000000000..fbce86ea5c >>> --- /dev/null >>> +++ b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.c >>> @@ -0,0 +1,809 @@ >>> +/** @file >>> + Http IO Helper Library. >>> + >>> + (C) Copyright 2020 Hewlett-Packard Development Company, L.P.
>>> + SPDX-License-Identifier: BSD-2-Clause-Patent **/ >>> + >>> +#include >>> + >>> +#include >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include #include >>> + #include >>> + >>> +/** >>> + Notify the callback function when an event is triggered. >>> + >>> + @param[in] Context The opaque parameter to the function. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +HttpIoNotifyDpc ( >>> + IN VOID *Context >>> + ) >>> +{ >>> + *((BOOLEAN *) Context) = TRUE; >>> +} >>> + >>> +/** >>> + Request HttpIoNotifyDpc as a DPC at TPL_CALLBACK. >>> + >>> + @param[in] Event The event signaled. >>> + @param[in] Context The opaque parameter to the function. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +HttpIoNotify ( >>> + IN EFI_EVENT Event, >>> + IN VOID *Context >>> + ) >>> +{ >>> + // >>> + // Request HttpIoNotifyDpc as a DPC at TPL_CALLBACK >>> + // >>> + QueueDpc (TPL_CALLBACK, HttpIoNotifyDpc, Context); } >>> + >>> +/** >>> + Destroy the HTTP_IO and release the resources. >>> + >>> + @param[in] HttpIo The HTTP_IO which wraps the HTTP service to be >> destroyed. >>> + >>> +**/ >>> +VOID >>> +HttpIoDestroyIo ( >>> + IN HTTP_IO *HttpIo >>> + ) >>> +{ >>> + EFI_HTTP_PROTOCOL *Http; >>> + EFI_EVENT Event; >>> + >>> + if (HttpIo == NULL) { >>> + return; >>> + } >>> + >>> + Event = HttpIo->ReqToken.Event; >>> + if (Event != NULL) { >>> + gBS->CloseEvent (Event); >>> + } >>> + >>> + Event = HttpIo->RspToken.Event; >>> + if (Event != NULL) { >>> + gBS->CloseEvent (Event); >>> + } >>> + >>> + Event = HttpIo->TimeoutEvent; >>> + if (Event != NULL) { >>> + gBS->CloseEvent (Event); >>> + } >>> + >>> + Http = HttpIo->Http; >>> + if (Http != NULL) { >>> + Http->Configure (Http, NULL); >>> + gBS->CloseProtocol ( >>> + HttpIo->Handle, >>> + &gEfiHttpProtocolGuid, >>> + HttpIo->Image, >>> + HttpIo->Controller >>> + ); >>> + } >>> + >>> + NetLibDestroyServiceChild ( >>> + HttpIo->Controller, >>> + HttpIo->Image, >>> + &gEfiHttpServiceBindingProtocolGuid, >>> + HttpIo->Handle >>> + ); >>> +} >>> + >>> +/** >>> + Create a HTTP_IO to access the HTTP service. It will create and >>> +configure >>> + a HTTP child handle. >>> + >>> + @param[in] Image The handle of the driver image. >>> + @param[in] Controller The handle of the controller. >>> + @param[in] IpVersion IP_VERSION_4 or IP_VERSION_6. >>> + @param[in] ConfigData The HTTP_IO configuration data , >>> + NULL means not to configure the HTTP child. >>> + @param[in] Callback Callback function which will be invoked when >> specified >>> + HTTP_IO_CALLBACK_EVENT happened. >>> + @param[in] Context The Context data which will be passed to the >> Callback function. >>> + @param[out] HttpIo The HTTP_IO. >>> + >>> + @retval EFI_SUCCESS The HTTP_IO is created and configured. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_UNSUPPORTED One or more of the control options are >> not >>> + supported in the implementation. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval Others Failed to create the HTTP_IO or configure it. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoCreateIo ( >>> + IN EFI_HANDLE Image, >>> + IN EFI_HANDLE Controller, >>> + IN UINT8 IpVersion, >>> + IN HTTP_IO_CONFIG_DATA *ConfigData, OPTIONAL >>> + IN HTTP_IO_CALLBACK Callback, >>> + IN VOID *Context, >>> + OUT HTTP_IO *HttpIo >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_HTTP_CONFIG_DATA HttpConfigData; >>> + EFI_HTTPv4_ACCESS_POINT Http4AccessPoint; >>> + EFI_HTTPv6_ACCESS_POINT Http6AccessPoint; >>> + EFI_HTTP_PROTOCOL *Http; >>> + EFI_EVENT Event; >>> + >>> + if ((Image == NULL) || (Controller == NULL) || (HttpIo == NULL)) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + if (IpVersion != IP_VERSION_4 && IpVersion != IP_VERSION_6) { >>> + return EFI_UNSUPPORTED; >>> + } >>> + >>> + ZeroMem (HttpIo, sizeof (HTTP_IO)); ZeroMem (&HttpConfigData, >>> + sizeof (EFI_HTTP_CONFIG_DATA)); >>> + >>> + // >>> + // Create the HTTP child instance and get the HTTP protocol. >>> + // >>> + Status = NetLibCreateServiceChild ( >>> + Controller, >>> + Image, >>> + &gEfiHttpServiceBindingProtocolGuid, >>> + &HttpIo->Handle >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + >>> + Status = gBS->OpenProtocol ( >>> + HttpIo->Handle, >>> + &gEfiHttpProtocolGuid, >>> + (VOID **) &Http, >>> + Image, >>> + Controller, >>> + EFI_OPEN_PROTOCOL_BY_DRIVER >>> + ); >>> + if (EFI_ERROR (Status) || (Http == NULL)) { >>> + goto ON_ERROR; >>> + } >>> + >>> + // >>> + // Init the configuration data and configure the HTTP child. >>> + // >>> + HttpIo->Image = Image; >>> + HttpIo->Controller = Controller; >>> + HttpIo->IpVersion = IpVersion; >>> + HttpIo->Http = Http; >>> + HttpIo->Callback = Callback; >>> + HttpIo->Context = Context; >>> + >>> + if (ConfigData != NULL) { >>> + if (HttpIo->IpVersion == IP_VERSION_4) { >>> + HttpConfigData.LocalAddressIsIPv6 = FALSE; >>> + HttpConfigData.HttpVersion = ConfigData->Config4.HttpVersion; >>> + HttpConfigData.TimeOutMillisec = ConfigData- >>> Config4.RequestTimeOut; >>> + >>> + Http4AccessPoint.UseDefaultAddress = ConfigData- >>> Config4.UseDefaultAddress; >>> + Http4AccessPoint.LocalPort = ConfigData->Config4.LocalPort; >>> + IP4_COPY_ADDRESS (&Http4AccessPoint.LocalAddress, &ConfigData- >>> Config4.LocalIp); >>> + IP4_COPY_ADDRESS (&Http4AccessPoint.LocalSubnet, &ConfigData- >>> Config4.SubnetMask); >>> + HttpConfigData.AccessPoint.IPv4Node = &Http4AccessPoint; >>> + } else { >>> + HttpConfigData.LocalAddressIsIPv6 = TRUE; >>> + HttpConfigData.HttpVersion = ConfigData->Config6.HttpVersion; >>> + HttpConfigData.TimeOutMillisec = ConfigData- >>> Config6.RequestTimeOut; >>> + >>> + Http6AccessPoint.LocalPort = ConfigData->Config6.LocalPort; >>> + IP6_COPY_ADDRESS (&Http6AccessPoint.LocalAddress, &ConfigData- >>> Config6.LocalIp); >>> + HttpConfigData.AccessPoint.IPv6Node = &Http6AccessPoint; >>> + } >>> + >>> + Status = Http->Configure (Http, &HttpConfigData); >>> + if (EFI_ERROR (Status)) { >>> + goto ON_ERROR; >>> + } >>> + } >>> + >>> + // >>> + // Create events for variuos asynchronous operations. >>> + // >>> + Status = gBS->CreateEvent ( >>> + EVT_NOTIFY_SIGNAL, >>> + TPL_NOTIFY, >>> + HttpIoNotify, >>> + &HttpIo->IsTxDone, >>> + &Event >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + goto ON_ERROR; >>> + } >>> + HttpIo->ReqToken.Event = Event; >>> + HttpIo->ReqToken.Message = &HttpIo->ReqMessage; >>> + >>> + Status = gBS->CreateEvent ( >>> + EVT_NOTIFY_SIGNAL, >>> + TPL_NOTIFY, >>> + HttpIoNotify, >>> + &HttpIo->IsRxDone, >>> + &Event >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + goto ON_ERROR; >>> + } >>> + HttpIo->RspToken.Event = Event; >>> + HttpIo->RspToken.Message = &HttpIo->RspMessage; >>> + >>> + // >>> + // Create TimeoutEvent for response // Status = gBS->CreateEvent >>> + ( >>> + EVT_TIMER, >>> + TPL_CALLBACK, >>> + NULL, >>> + NULL, >>> + &Event >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + goto ON_ERROR; >>> + } >>> + HttpIo->TimeoutEvent = Event; >>> + return EFI_SUCCESS; >>> + >>> +ON_ERROR: >>> + HttpIoDestroyIo (HttpIo); >>> + >>> + return Status; >>> +} >>> + >>> +/** >>> + Synchronously send a HTTP REQUEST message to the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] Request A pointer to storage such data as URL and >> HTTP method. >>> + @param[in] HeaderCount Number of HTTP header structures in >> Headers list. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[in] BodyLength Length in bytes of the HTTP body. >>> + @param[in] Body Body associated with the HTTP request. >>> + >>> + @retval EFI_SUCCESS The HTTP request is trasmitted. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_DEVICE_ERROR An unexpected network or system error >> occurred. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoSendRequest ( >>> + IN HTTP_IO *HttpIo, >>> + IN EFI_HTTP_REQUEST_DATA *Request, >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, >>> + IN UINTN BodyLength, >>> + IN VOID *Body >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_HTTP_PROTOCOL *Http; >>> + >>> + if (HttpIo == NULL || HttpIo->Http == NULL) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + HttpIo->ReqToken.Status = EFI_NOT_READY; >>> + HttpIo->ReqToken.Message->Data.Request = Request; >>> + HttpIo->ReqToken.Message->HeaderCount = HeaderCount; >>> + HttpIo->ReqToken.Message->Headers = Headers; >>> + HttpIo->ReqToken.Message->BodyLength = BodyLength; >>> + HttpIo->ReqToken.Message->Body = Body; >>> + >>> + if (HttpIo->Callback != NULL) { >>> + Status = HttpIo->Callback ( >>> + HttpIoRequest, >>> + HttpIo->ReqToken.Message, >>> + HttpIo->Context >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + } >>> + >>> + // >>> + // Queue the request token to HTTP instances. >>> + // >>> + Http = HttpIo->Http; >>> + HttpIo->IsTxDone = FALSE; >>> + Status = Http->Request ( >>> + Http, >>> + &HttpIo->ReqToken >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + >>> + // >>> + // Poll the network until transmit finish. >>> + // >>> + while (!HttpIo->IsTxDone) { >>> + Http->Poll (Http); >>> + } >>> + >>> + return HttpIo->ReqToken.Status; >>> +} >>> + >>> +/** >>> + Synchronously receive a HTTP RESPONSE message from the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] RecvMsgHeader TRUE to receive a new HTTP response >> (from message header). >>> + FALSE to continue receive the previous response message. >>> + @param[out] ResponseData Point to a wrapper of the received >> response data. >>> + >>> + @retval EFI_SUCCESS The HTTP response is received. >>> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_DEVICE_ERROR An unexpected network or system error >> occurred. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoRecvResponse ( >>> + IN HTTP_IO *HttpIo, >>> + IN BOOLEAN RecvMsgHeader, >>> + OUT HTTP_IO_RESPONSE_DATA *ResponseData >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_HTTP_PROTOCOL *Http; >>> + >>> + if (HttpIo == NULL || HttpIo->Http == NULL || ResponseData == NULL) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + // >>> + // Create new TimeoutEvent for response // if >>> + (HttpIo->TimeoutEvent != NULL) { >>> + gBS->CloseEvent (HttpIo->TimeoutEvent); >>> + HttpIo->TimeoutEvent = NULL; >>> + } >>> + >>> + Status = gBS->CreateEvent ( >>> + EVT_TIMER, >>> + TPL_CALLBACK, >>> + NULL, >>> + NULL, >>> + &HttpIo->TimeoutEvent >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >> Is it necessary to recreate the event? Wouldn't this be enough to reset it? > No, we don’t have to recreate this event. >> gBS->SetTimer (HttpIo->TimeoutEvent, TimerCancel, 0); > I made some changes in this function. >>> + >>> + // >>> + // Start the timer, and wait Timeout seconds to receive the header >> packet. >>> + // >>> + Status = gBS->SetTimer (HttpIo->TimeoutEvent, TimerRelative, >>> + HttpIo->Timeout * TICKS_PER_MS); if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + >>> + // >>> + // Queue the response token to HTTP instances. >>> + // >>> + HttpIo->RspToken.Status = EFI_NOT_READY; if (RecvMsgHeader) { >>> + HttpIo->RspToken.Message->Data.Response = >>> + &ResponseData->Response; } else { >>> + HttpIo->RspToken.Message->Data.Response = NULL; } >>> + HttpIo->RspToken.Message->HeaderCount = 0; >>> + HttpIo->RspToken.Message->Headers = NULL; >>> + HttpIo->RspToken.Message->BodyLength = ResponseData- >>> BodyLength; >>> + HttpIo->RspToken.Message->Body = ResponseData->Body; >>> + >>> + Http = HttpIo->Http; >>> + HttpIo->IsRxDone = FALSE; >>> + Status = Http->Response ( >>> + Http, >>> + &HttpIo->RspToken >>> + ); >>> + >>> + if (EFI_ERROR (Status)) { >>> + gBS->SetTimer (HttpIo->TimeoutEvent, TimerCancel, 0); >>> + return Status; >>> + } >>> + >>> + // >>> + // Poll the network until receive finish. >>> + // >>> + while (!HttpIo->IsRxDone && ((HttpIo->TimeoutEvent == NULL) || >> EFI_ERROR (gBS->CheckEvent (HttpIo->TimeoutEvent)))) { >>> + Http->Poll (Http); >>> + } >>> + >>> + gBS->SetTimer (HttpIo->TimeoutEvent, TimerCancel, 0); >>> + >>> + if (!HttpIo->IsRxDone) { >>> + // >>> + // Timeout occurs, cancel the response token. >>> + // >>> + Http->Cancel (Http, &HttpIo->RspToken); >>> + >>> + Status = EFI_TIMEOUT; >>> + >>> + return Status; >>> + } else { >>> + HttpIo->IsRxDone = FALSE; >>> + } >>> + >>> + if ((HttpIo->Callback != NULL) && >>> + (HttpIo->RspToken.Status == EFI_SUCCESS || HttpIo- >>> RspToken.Status == EFI_HTTP_ERROR)) { >>> + Status = HttpIo->Callback ( >>> + HttpIoResponse, >>> + HttpIo->RspToken.Message, >>> + HttpIo->Context >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + } >>> + >>> + // >>> + // Store the received data into the wrapper. >>> + // >>> + ResponseData->Status = HttpIo->RspToken.Status; >>> + ResponseData->HeaderCount = HttpIo->RspToken.Message- >>> HeaderCount; >>> + ResponseData->Headers = HttpIo->RspToken.Message->Headers; >>> + ResponseData->BodyLength = HttpIo->RspToken.Message- >>> BodyLength; >>> + >>> + return Status; >>> +} >>> + >>> +/** >>> + Get the value of the content length if there is a "Content-Length" header. >>> + >>> + @param[in] HeaderCount Number of HTTP header structures in >> Headers. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[out] ContentLength Pointer to save the value of the content >> length. >>> + >>> + @retval EFI_SUCCESS Successfully get the content length. >>> + @retval EFI_NOT_FOUND No "Content-Length" header in the >> Headers. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoGetContentLength ( >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, >>> + OUT UINTN *ContentLength >>> + ) >>> +{ >>> + EFI_HTTP_HEADER *Header; >>> + >>> + Header = HttpFindHeader (HeaderCount, Headers, >>> + HTTP_HEADER_CONTENT_LENGTH); if (Header == NULL) { >>> + return EFI_NOT_FOUND; >>> + } >>> + >>> + return AsciiStrDecimalToUintnS (Header->FieldValue, (CHAR8 **) >>> +NULL, ContentLength); } >>> +/** >>> + Send HTTP request in chunks. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] SendChunkProcess Pointer to current chunk process >> status. >>> + @param[in] RequestMessage Request to send. >>> + >>> + @retval EFI_SUCCESS Successfully to send chunk data according to >> SendChunkProcess. >>> + @retval Other Other errors. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoSendChunkedTransfer ( >>> + IN HTTP_IO *HttpIo, >>> + IN HTTP_IO_SEND_CHUNK_PROCESS *SendChunkProcess, >>> + IN EFI_HTTP_MESSAGE *RequestMessage >>> +) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_HTTP_HEADER *NewHeaders; >>> + EFI_HTTP_HEADER *ContentLengthHeader; >>> + UINTN AddNewHeader; >>> + UINTN HeaderCount; >>> + CHAR8 *MessageBody; >>> + UINTN MessageBodyLength; >>> + CHAR8 ChunkLengthStr [HTTP_IO_CHUNK_SIZE_STRING_LEN]; >>> + EFI_HTTP_REQUEST_DATA *SentRequestData; >> This is a new function and I already see a difference in the coding standard. >> Please align variable names like in previous functions. > Done > >>> + >>> + AddNewHeader = 0; >>> + NewHeaders = NULL; >>> + MessageBody = NULL; >>> + ContentLengthHeader = NULL; >>> + MessageBodyLength = 0; >> Alignment of assignments. > Done > >>> + >>> + switch (*SendChunkProcess) { >>> + case HttpIoSendChunkHeaderZeroContent: >> Case spacing: 2 spaces. Applies throughout new code. >>> + ContentLengthHeader = >>> + HttpFindHeader(RequestMessage->HeaderCount, RequestMessage- >>> Headers, >>> + HTTP_HEADER_CONTENT_LENGTH); >> Spacing after function call name. This applies from this point until the end of >> patch. > Done >>> + if (ContentLengthHeader == NULL) { >>> + AddNewHeader = 1; >>> + } >>> + >>> + NewHeaders = AllocateZeroPool((RequestMessage->HeaderCount + >>> + AddNewHeader) * sizeof(EFI_HTTP_HEADER)); >> Lack of NULL pointer test for NewHeaders (out of memory case). >>> + CopyMem ((VOID*)NewHeaders, (VOID *)RequestMessage->Headers, >> RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER)); >>> + if (AddNewHeader == 0) { >>> + // >>> + // Override content-length to Transfer-Encoding. >>> + // >>> + ContentLengthHeader = HttpFindHeader (RequestMessage- >>> HeaderCount, NewHeaders, HTTP_HEADER_CONTENT_LENGTH); >>> + ContentLengthHeader->FieldName = NULL; >>> + ContentLengthHeader->FieldValue = NULL; >>> + } else { >>> + ContentLengthHeader = NewHeaders + RequestMessage- >>> HeaderCount; >>> + } >>> + HttpSetFieldNameAndValue(ContentLengthHeader, >> HTTP_HEADER_TRANSFER_ENCODING, >> HTTP_HEADER_TRANSFER_ENCODING_CHUNKED); >>> + HeaderCount = RequestMessage->HeaderCount + AddNewHeader; >> Move HeaderCount assignment before NewHeaders allocation. Use >> HeaderCount as argument for NewHeaders allocation. >>> + MessageBodyLength = 0; >>> + MessageBody = NULL; >>> + SentRequestData = RequestMessage->Data.Request; >>> + break; >>> + >>> + case HttpIoSendChunkContent: >>> + HeaderCount = 0; >>> + NewHeaders = NULL; >>> + SentRequestData = NULL; >>> + if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) { >>> + MessageBodyLength = HTTP_IO_MAX_SEND_PAYLOAD; >>> + } else { >>> + MessageBodyLength = RequestMessage->BodyLength; >>> + } >>> + AsciiSPrint (ChunkLengthStr, HTTP_IO_CHUNK_SIZE_STRING_LEN, >>> + "%x%c%c", MessageBodyLength, CHUNKED_TRNASFER_CODING_CR, >>> + CHUNKED_TRNASFER_CODING_LF); >> Line too long, break down in accordance to coding standard. > Done > >>> + MessageBody = AllocatePool (AsciiStrLen (ChunkLengthStr) + >> MessageBodyLength + 2); >>> + if (MessageBody == NULL) { >>> + DEBUG((DEBUG_ERROR, "Not enough memory for chunk >> transfer\n")); >>> + return EFI_OUT_OF_RESOURCES; >>> + } >>> + CopyMem (MessageBody, ChunkLengthStr, AsciiStrLen >> (ChunkLengthStr)); >>> + CopyMem (MessageBody + AsciiStrLen (ChunkLengthStr), >> RequestMessage->Body, MessageBodyLength); >>> + *(MessageBody + AsciiStrLen (ChunkLengthStr) + MessageBodyLength) >> = CHUNKED_TRNASFER_CODING_CR; >>> + *(MessageBody + AsciiStrLen (ChunkLengthStr) + MessageBodyLength >> + 1) = CHUNKED_TRNASFER_CODING_LF; >>> + RequestMessage->BodyLength -= MessageBodyLength; >>> + RequestMessage->Body = (VOID *)((CHAR8 *)RequestMessage- >>> Body + MessageBodyLength); >>> + MessageBodyLength += (AsciiStrLen (ChunkLengthStr) + 2); >> This block could use some spacing and comments to improve readability. > Done > >>> + if (RequestMessage->BodyLength == 0) { >>> + *SendChunkProcess = HttpIoSendChunkEndChunk; >>> + } >>> + break; >>> + >>> + case HttpIoSendChunkEndChunk: >>> + HeaderCount = 0; >>> + NewHeaders = NULL; >>> + SentRequestData = NULL; >>> + AsciiSPrint (ChunkLengthStr, HTTP_IO_CHUNK_SIZE_STRING_LEN, >> "0%c%c%c%c", >>> + CHUNKED_TRNASFER_CODING_CR, >> CHUNKED_TRNASFER_CODING_LF, >>> + CHUNKED_TRNASFER_CODING_CR, >> CHUNKED_TRNASFER_CODING_LF >>> + ); >> Please break function calls in accordance with coding standard. > Done >>> + MessageBody = AllocatePool (AsciiStrLen(ChunkLengthStr)); >>> + if (MessageBody == NULL) { >>> + DEBUG((DEBUG_ERROR, "Not enough memory for the end chunk >> transfer\n")); >>> + return EFI_OUT_OF_RESOURCES; >>> + } >>> + CopyMem (MessageBody, ChunkLengthStr, AsciiStrLen >> (ChunkLengthStr)); >>> + MessageBodyLength = AsciiStrLen(ChunkLengthStr); >>> + *SendChunkProcess = HttpIoSendChunkFinish; >>> + break; >>> + >>> + default: >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + Status = HttpIoSendRequest( >>> + HttpIo, >>> + SentRequestData, >>> + HeaderCount, >>> + NewHeaders, >>> + MessageBodyLength, >>> + MessageBody >>> + ); >>> + if (ContentLengthHeader != NULL) { >>> + if (ContentLengthHeader->FieldName != NULL) { >>> + FreePool(ContentLengthHeader->FieldName); >>> + } >>> + if (ContentLengthHeader->FieldValue != NULL) { >>> + FreePool(ContentLengthHeader->FieldValue); >>> + } >>> + ContentLengthHeader = NULL; >> NULL assignments are not necessary here as we are exiting the function and >> those variables will not be used. > Right, fixed. >>> + } >>> + if (NewHeaders != NULL) { >>> + FreePool(NewHeaders); >>> + NewHeaders = NULL; >>> + } >>> + if (MessageBody != NULL) { >>> + FreePool(MessageBody); >>> + MessageBody = NULL; >>> + } >>> + return Status; >>> +} >>> + >>> +/** >>> + Synchronously receive a HTTP RESPONSE message from the server. >>> + >>> + @param[in] HttpIo The HttpIo wrapping the HTTP service. >>> + @param[in] HeaderCount Number of headers in Headers. >>> + @param[in] Headers Array containing list of HTTP headers. >>> + @param[out] ChunkListHead A pointer to receivce list head of chunked >> data. >> Spelling: receive. > Fixed. > >>> + Caller has to release memory of ChunkListHead >>> + and all list entries. >>> + @param[out] ContentLength Total content length >>> + >>> + @retval EFI_SUCCESS The HTTP chunked transfer is received. >>> + @retval EFI_NOT_FOUND No chunked transfer coding header found. >>> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >>> + @retval EFI_INVALID_PARAMETER Improper parameters. >>> + @retval Others Other errors as indicated. >>> + >>> +**/ >>> +EFI_STATUS >>> +HttpIoGetChunkedTransferContent ( >>> + IN HTTP_IO *HttpIo, >>> + IN UINTN HeaderCount, >>> + IN EFI_HTTP_HEADER *Headers, >>> + OUT LIST_ENTRY **ChunkListHead, >>> + OUT UINTN *ContentLength >>> + ) >>> +{ >>> + EFI_HTTP_HEADER *Header; >>> + CHAR8 ChunkSizeAscii[256]; >>> + EFI_STATUS Status; >>> + UINTN Index; >>> + HTTP_IO_RESPONSE_DATA ResponseData; >>> + UINTN TotalLength; >>> + LIST_ENTRY *HttpChunks; >>> + HTTP_IO_CHUNKS *ThisChunk; >> Variable alignment. >>> + LIST_ENTRY *ThisListEntry; >>> + >>> + if (ChunkListHead == NULL || ContentLength == NULL) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + *ContentLength = 0; >>> + Header = HttpFindHeader (HeaderCount, Headers, >>> + HTTP_HEADER_TRANSFER_ENCODING); if (Header == NULL) { >>> + return EFI_NOT_FOUND; >>> + } >>> + if (AsciiStrCmp (Header->FieldValue, >>> + HTTP_HEADER_TRANSFER_ENCODING_CHUNKED) == 0) { >> Please do a negation comparison instead and return. It reduces one level of >> indentation. > Done >>> + // >>> + // Loop to get all chunks. >>> + // >> Please avoid empty single-line comments when starting and ending >> comment block. >>> + TotalLength = 0; >>> + HttpChunks = (LIST_ENTRY *)AllocateZeroPool (sizeof (LIST_ENTRY)); >>> + if (HttpChunks == NULL) { >>> + Status = EFI_OUT_OF_RESOURCES; >>> + goto DeleterChunks; >>> + } >>> + InitializeListHead (HttpChunks); >> Add a bit of spacing above to improve readability. > Fixed >>> + DEBUG ((DEBUG_INFO, " Chunked transfer\n")); >>> + while (TRUE) { >>> + ZeroMem((VOID *)&ResponseData, >> sizeof(HTTP_IO_RESPONSE_DATA)); >>> + ResponseData.BodyLength = >> HTTP_IO_CHUNKED_TRANSFER_CODING_DATA_LENGTH; >>> + ResponseData.Body = ChunkSizeAscii; >>> + Status = HttpIoRecvResponse ( >>> + HttpIo, >>> + FALSE, >>> + &ResponseData >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >> Returning here creates memory leak risk - chunks previously received will not >> be freed. > Fixed >>> + // >>> + // Decoding Chunked Transfer Coding. >>> + // Only decode chunk-size and last chunk. >>> + // >>> + DEBUG ((DEBUG_INFO, " Chunk HTTP Response StatusCode - %d\n", >> ResponseData.Response.StatusCode)); >>> + // >>> + // Break if this is last chunk. >>> + // >>> + if (ChunkSizeAscii [0] == CHUNKED_TRNASFER_CODING_LAST_CHUNK) >> { >>> + Status = EFI_SUCCESS; >>> + DEBUG ((DEBUG_INFO, " Last chunk\n")); >>> + ThisChunk = (HTTP_IO_CHUNKS *)AllocateZeroPool (sizeof >> (HTTP_IO_CHUNKS)); >>> + if (ThisChunk == NULL) { >>> + Status = EFI_OUT_OF_RESOURCES; >>> + goto DeleterChunks; >>> + } >>> + InitializeListHead (&ThisChunk->NextChunk); >>> + ThisChunk->Length = ResponseData.BodyLength - 1 - 2; // Minus >> sizeof '0' and CRLF. >>> + ThisChunk->Data = (CHAR8 *)AllocatePool (ThisChunk->Length); >>> + if (ThisChunk->Data == NULL) { >>> + FreePool ((UINT8 *)ThisChunk); >>> + Status = EFI_OUT_OF_RESOURCES; >>> + goto DeleterChunks; >>> + } >>> + CopyMem ((UINT8 *)ThisChunk->Data, (UINT8 *)ResponseData.Body >> + 1, ThisChunk->Length); >>> + TotalLength += ThisChunk->Length; >>> + InsertTailList (HttpChunks, &ThisChunk->NextChunk); >>> + break; >>> + } >>> + >>> + // >>> + // Get the chunk length >>> + // >>> + Index = 0; >>> + while ((ChunkSizeAscii [Index] != >> CHUNKED_TRNASFER_CODING_EXTENSION_SAPERATOR) && >>> + (ChunkSizeAscii [Index] != >>> + (CHAR8)CHUNKED_TRNASFER_CODING_CR) && >> Spelling errors in macros above. > Oops. BZ is submitted for this typo in http11.h. > https://bugzilla.tianocore.org/show_bug.cgi?id=3019 > Patch is set to the mailing list for review. > > >>> + (Index != >> HTTP_IO_CHUNKED_TRANSFER_CODING_DATA_LENGTH)) { >>> + Index ++; >>> + }; >>> + if (Index == HTTP_IO_CHUNKED_TRANSFER_CODING_DATA_LENGTH) >> { >>> + return EFI_NOT_FOUND; >>> + } >> Again - memory leak. > Fixed > >>> + ChunkSizeAscii[Index] = 0; >>> + AsciiStrHexToUintnS (ChunkSizeAscii, NULL, ContentLength); >>> + DEBUG ((DEBUG_INFO, " Length of this chunk %d\n", >> *ContentLength)); >>> + // >>> + // Receive the data; >>> + // >>> + ThisChunk = (HTTP_IO_CHUNKS *)AllocateZeroPool (sizeof >> (HTTP_IO_CHUNKS)); >>> + if (ThisChunk == NULL) { >>> + Status = EFI_OUT_OF_RESOURCES; >>> + goto DeleterChunks; >>> + } >>> + ResponseData.BodyLength = *ContentLength; >>> + ResponseData.Body = (CHAR8 *)AllocatePool (*ContentLength); >>> + if (ResponseData.Body == NULL) { >>> + FreePool (ThisChunk); >>> + Status = EFI_OUT_OF_RESOURCES; >>> + goto DeleterChunks; >>> + } >>> + InitializeListHead (&ThisChunk->NextChunk); >>> + ThisChunk->Length = *ContentLength; >>> + ThisChunk->Data = ResponseData.Body; >>> + InsertTailList (HttpChunks, &ThisChunk->NextChunk); >>> + Status = HttpIoRecvResponse ( >>> + HttpIo, >>> + FALSE, >>> + &ResponseData >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + goto DeleterChunks; >>> + } >>> + // >>> + // Read CRLF >>> + // >>> + ZeroMem((VOID *)&ResponseData, >> sizeof(HTTP_IO_RESPONSE_DATA)); >>> + ResponseData.BodyLength = 2; >>> + ResponseData.Body = ChunkSizeAscii; >>> + Status = HttpIoRecvResponse ( >>> + HttpIo, >>> + FALSE, >>> + &ResponseData >>> + ); >>> + if (EFI_ERROR (Status)) { >>> + goto DeleterChunks; >>> + } >>> + >>> + TotalLength += *ContentLength; >> The following code requires some kind of protection against receiving an >> infinite number of chunks. >> Currently I could just not send a last chunk and wait for the platform to crash >> or/and run out of memory :) >> > Good point! However, I can't think of any good way to prevent from this happens. This depends on the reliability of content provider and how the web service handles chunk transfer, right? > From system firmware viewpoint, we would connect to a trusted content source for either HTTP boot image and Redfish service resource. > Do you think this concern is valid for the real practice? > > Or give it a limited size of payload? Like the size larger than 1G could be considered as an ill content? What would be the largest payload that you would like to use chunk-transfer for? I see that the chunked transfer is not being currently used in HTTP Boot, so we should stick to current use case -  Redfish. > > >>> + }; >>> + *ContentLength = TotalLength; >>> + *ChunkListHead = HttpChunks; >>> + DEBUG ((DEBUG_INFO, " Total of lengh of chunks :%d\n", >> TotalLength)); >>> + return EFI_SUCCESS; >>> + } else { >>> + return EFI_NOT_FOUND; >>> + } >>> +DeleterChunks:; >> Name this label ExitDeleteChunks and remove the comma. >>> + while (!IsListEmpty (HttpChunks)) { >>> + ThisListEntry = GetFirstNode (HttpChunks); >>> + RemoveEntryList (ThisListEntry); >>> + }; >>> + return Status; >>> +} >>> diff --git a/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.inf >>> b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.inf >>> new file mode 100644 >>> index 0000000000..a02b409547 >>> --- /dev/null >>> +++ b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.inf >>> @@ -0,0 +1,43 @@ >>> +## @file >>> +# This library instance provides HTTP IO helper functions. >>> +# >>> +# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
# >>> +SPDX-License-Identifier: BSD-2-Clause-Patent # ## >>> + >>> +[Defines] >>> + INF_VERSION = 0x0001001b >>> + BASE_NAME = DxeHttpIoLib >>> + MODULE_UNI_FILE = DxeHttpIoLib.uni >>> + FILE_GUID = 50B198F8-7986-4F51-A857-CFE4643D59F3 >>> + MODULE_TYPE = DXE_DRIVER >>> + VERSION_STRING = 1.0 >>> + LIBRARY_CLASS = HttpIoLib| DXE_CORE DXE_DRIVER >> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION >> UEFI_DRIVER >>> + >>> +# >>> +# The following information is for reference only and not required by the >> build tools. >>> +# >>> +# VALID_ARCHITECTURES = IA32 X64 EBC ARM AARM64 RISCV64 >>> +# >>> + >>> +[Sources] >>> + DxeHttpIoLib.c >>> + >>> +[Packages] >>> + MdePkg/MdePkg.dec >>> + NetworkPkg/NetworkPkg.dec >>> + >>> + >>> +[LibraryClasses] >>> + BaseLib >>> + BaseMemoryLib >>> + DebugLib >>> + DpcLib >>> + MemoryAllocationLib >>> + PrintLib >>> + UefiBootServicesTableLib >>> + >>> +[Protocols] >>> + gEfiHttpProtocolGuid ## SOMETIMES_CONSUMES >>> + >>> diff --git a/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.uni >>> b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.uni >>> new file mode 100644 >>> index 0000000000..d419b1a49d >>> --- /dev/null >>> +++ b/NetworkPkg/Library/DxeHttpIoLib/DxeHttpIoLib.uni >>> @@ -0,0 +1,13 @@ >>> +// /** @file >>> +// The library instance provides HTTP IO helper functions. >>> +// >>> +// (C) Copyright 2020 Hewlett Packard Enterprise Development LP
>>> +// SPDX-License-Identifier: BSD-2-Clause-Patent // // **/ >>> + >>> + >>> +#string STR_MODULE_ABSTRACT #language en-US "HTTP IO Helper >> Library" >>> + >>> +#string STR_MODULE_DESCRIPTION #language en-US "The library >> instance provides HTTP IO helper functions." >>> + > > > > >