From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web08.6376.1609858115864579107 for ; Tue, 05 Jan 2021 06:48:35 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.115, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: uVhEt+nUR9h8azNpqBkJIJRPFivfY6swe5XdqB32tIjFXHqOhVGInLibDkgzCsReNdjw8Ze4IG CVT0ImpMHz0g== X-IronPort-AV: E=McAfee;i="6000,8403,9854"; a="176332204" X-IronPort-AV: E=Sophos;i="5.78,477,1599548400"; d="scan'208";a="176332204" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2021 06:48:34 -0800 IronPort-SDR: K0wuerb6NEvMg8YBI8Vk8SDTlaN89rA+nC7DYRFIf3GR4aqhvDFjGOt1U11pZ0JBWHkv+kY6hW rHdAnuc+sf9Q== X-IronPort-AV: E=Sophos;i="5.78,477,1599548400"; d="scan'208";a="378891865" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.252.38.78]) ([10.252.38.78]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2021 06:48:32 -0800 Subject: Re: [edk2-devel] [PATCH] NetworkPkg/DxeHttpLib: Migrate HTTP header manipulation APIs To: devel@edk2.groups.io, abner.chang@hpe.com Cc: Jiaxin Wu , Siyuan Fu , Fan Wang , Jiewen Yao , Nickle Wang , Peter O'Hanley References: <20201218023931.20445-1-abner.chang@hpe.com> From: "Maciej Rabeda" Message-ID: <34fdd782-b2b7-d64d-8852-9a792db3c023@linux.intel.com> Date: Tue, 5 Jan 2021 15:48:30 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20201218023931.20445-1-abner.chang@hpe.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: pl Hi Abner, Sorry for the late response, I was on vacation. One quick thing: I can see that the functions are simply being moved from one place to the other and renamed to HttpIo___(). The only issue I have is that this patch does not remove HttpBoot___() functions from HttpDxe/HttpBootSupport.c Upon removing those, I will give my approval :) Thanks, Maciej On 18-Dec-20 03:39, Abner Chang wrote: > Move HTTP header manipulation functions to DxeHttpLib from > HttpBootSupport.c. These general functions are used by both > Http BOOT and RedfishLib (patches will be sent later). > > Signed-off-by: Abner Chang > > Cc: Maciej Rabeda > Cc: Jiaxin Wu > Cc: Siyuan Fu > Cc: Fan Wang > Cc: Jiewen Yao > Cc: Nickle Wang > Cc: Peter O'Hanley > --- > NetworkPkg/HttpBootDxe/HttpBootSupport.h | 53 -------- > NetworkPkg/Include/Library/HttpLib.h | 53 ++++++++ > NetworkPkg/HttpBootDxe/HttpBootClient.c | 10 +- > NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c | 135 ++++++++++++++++++++- > 4 files changed, 192 insertions(+), 59 deletions(-) > > diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.h b/NetworkPkg/HttpBootDxe/HttpBootSupport.h > index 1a2d32dd5a..c2ac1b785a 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.h > +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.h > @@ -87,59 +87,6 @@ HttpBootPrintErrorMessage ( > EFI_HTTP_STATUS_CODE StatusCode > ); > > -// > -// A wrapper structure to hold the HTTP headers. > -// > -typedef struct { > - UINTN MaxHeaderCount; > - UINTN HeaderCount; > - EFI_HTTP_HEADER *Headers; > -} HTTP_IO_HEADER; > - > -/** > - Create a HTTP_IO_HEADER to hold the HTTP header items. > - > - @param[in] MaxHeaderCount The maximum number of HTTP header in this holder. > - > - @return A pointer of the HTTP header holder or NULL if failed. > - > -**/ > -HTTP_IO_HEADER * > -HttpBootCreateHeader ( > - IN UINTN MaxHeaderCount > - ); > - > -/** > - Destroy the HTTP_IO_HEADER and release the resources. > - > - @param[in] HttpIoHeader Point to the HTTP header holder to be destroyed. > - > -**/ > -VOID > -HttpBootFreeHeader ( > - IN HTTP_IO_HEADER *HttpIoHeader > - ); > - > -/** > - Set or update a HTTP header with the field name and corresponding value. > - > - @param[in] HttpIoHeader Point to the HTTP header holder. > - @param[in] FieldName Null terminated string which describes a field name. > - @param[in] FieldValue Null terminated string which describes the corresponding field value. > - > - @retval EFI_SUCCESS The HTTP header has been set or updated. > - @retval EFI_INVALID_PARAMETER Any input parameter is invalid. > - @retval EFI_OUT_OF_RESOURCES Insufficient resource to complete the operation. > - @retval Other Unexpected error happened. > - > -**/ > -EFI_STATUS > -HttpBootSetHeader ( > - IN HTTP_IO_HEADER *HttpIoHeader, > - IN CHAR8 *FieldName, > - IN CHAR8 *FieldValue > - ); > - > /** > Retrieve the host address using the EFI_DNS6_PROTOCOL. > > diff --git a/NetworkPkg/Include/Library/HttpLib.h b/NetworkPkg/Include/Library/HttpLib.h > index a906126b3d..2c3367fb01 100644 > --- a/NetworkPkg/Include/Library/HttpLib.h > +++ b/NetworkPkg/Include/Library/HttpLib.h > @@ -476,6 +476,59 @@ HttpIsValidHttpHeader ( > IN CHAR8 *FieldName > ); > > +// > +// A wrapper structure to hold the HTTP headers. > +// > +typedef struct { > + UINTN MaxHeaderCount; > + UINTN HeaderCount; > + EFI_HTTP_HEADER *Headers; > +} HTTP_IO_HEADER; > + > + > +/** > + Create a HTTP_IO_HEADER to hold the HTTP header items. > + > + @param[in] MaxHeaderCount The maximun number of HTTP header in this holder. > + > + @return A pointer of the HTTP header holder or NULL if failed. > + > +**/ > +HTTP_IO_HEADER * > +HttpIoCreateHeader ( > + UINTN MaxHeaderCount > + ); > + > +/** > + Destroy the HTTP_IO_HEADER and release the resources. > + > + @param[in] HttpIoHeader Point to the HTTP header holder to be destroyed. > + > +**/ > +VOID > +HttpIoFreeHeader ( > + IN HTTP_IO_HEADER *HttpIoHeader > + ); > + > +/** > + Set or update a HTTP header with the field name and corresponding value. > + > + @param[in] HttpIoHeader Point to the HTTP header holder. > + @param[in] FieldName Null terminated string which describes a field name. > + @param[in] FieldValue Null terminated string which describes the corresponding field value. > + > + @retval EFI_SUCCESS The HTTP header has been set or updated. > + @retval EFI_INVALID_PARAMETER Any input parameter is invalid. > + @retval EFI_OUT_OF_RESOURCES Insufficient resource to complete the operation. > + @retval Other Unexpected error happened. > + > +**/ > +EFI_STATUS > +HttpIoSetHeader ( > + IN HTTP_IO_HEADER *HttpIoHeader, > + IN CHAR8 *FieldName, > + IN CHAR8 *FieldValue > + ); > > #endif > > diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c > index 30ac15889b..8f21f7766e 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c > @@ -977,7 +977,7 @@ HttpBootGetBootFile ( > // Accept > // User-Agent > // > - HttpIoHeader = HttpBootCreateHeader (3); > + HttpIoHeader = HttpIoCreateHeader (3); > if (HttpIoHeader == NULL) { > Status = EFI_OUT_OF_RESOURCES; > goto ERROR_2; > @@ -995,7 +995,7 @@ HttpBootGetBootFile ( > if (EFI_ERROR (Status)) { > goto ERROR_3; > } > - Status = HttpBootSetHeader ( > + Status = HttpIoSetHeader ( > HttpIoHeader, > HTTP_HEADER_HOST, > HostName > @@ -1008,7 +1008,7 @@ HttpBootGetBootFile ( > // > // Add HTTP header field 2: Accept > // > - Status = HttpBootSetHeader ( > + Status = HttpIoSetHeader ( > HttpIoHeader, > HTTP_HEADER_ACCEPT, > "*/*" > @@ -1020,7 +1020,7 @@ HttpBootGetBootFile ( > // > // Add HTTP header field 3: User-Agent > // > - Status = HttpBootSetHeader ( > + Status = HttpIoSetHeader ( > HttpIoHeader, > HTTP_HEADER_USER_AGENT, > HTTP_USER_AGENT_EFI_HTTP_BOOT > @@ -1291,7 +1291,7 @@ ERROR_4: > FreePool (RequestData); > } > ERROR_3: > - HttpBootFreeHeader (HttpIoHeader); > + HttpIoFreeHeader (HttpIoHeader); > ERROR_2: > if (Cache != NULL) { > FreePool (Cache); > diff --git a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c > index daec1e0226..7354e0170f 100644 > --- a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c > +++ b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c > @@ -3,7 +3,7 @@ > It provides the helper routines to parse the HTTP message byte stream. > > Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> -(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +(C) Copyright 2016 - 2020 Hewlett Packard Enterprise Development LP
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -2095,3 +2095,136 @@ HttpIsValidHttpHeader ( > return TRUE; > } > > + > +/** > + Create a HTTP_IO_HEADER to hold the HTTP header items. > + > + @param[in] MaxHeaderCount The maximun number of HTTP header in this holder. > + > + @return A pointer of the HTTP header holder or NULL if failed. > + > +**/ > +HTTP_IO_HEADER * > +HttpIoCreateHeader ( > + UINTN MaxHeaderCount > + ) > +{ > + HTTP_IO_HEADER *HttpIoHeader; > + > + if (MaxHeaderCount == 0) { > + return NULL; > + } > + > + HttpIoHeader = AllocateZeroPool (sizeof (HTTP_IO_HEADER) + MaxHeaderCount * sizeof (EFI_HTTP_HEADER)); > + if (HttpIoHeader == NULL) { > + return NULL; > + } > + > + HttpIoHeader->MaxHeaderCount = MaxHeaderCount; > + HttpIoHeader->Headers = (EFI_HTTP_HEADER *) (HttpIoHeader + 1); > + > + return HttpIoHeader; > +} > + > +/** > + Destroy the HTTP_IO_HEADER and release the resources. > + > + @param[in] HttpIoHeader Point to the HTTP header holder to be destroyed. > + > +**/ > +VOID > +HttpIoFreeHeader ( > + IN HTTP_IO_HEADER *HttpIoHeader > + ) > +{ > + UINTN Index; > + > + if (HttpIoHeader != NULL) { > + if (HttpIoHeader->HeaderCount != 0) { > + for (Index = 0; Index < HttpIoHeader->HeaderCount; Index++) { > + FreePool (HttpIoHeader->Headers[Index].FieldName); > + ZeroMem (HttpIoHeader->Headers[Index].FieldValue, AsciiStrSize (HttpIoHeader->Headers[Index].FieldValue)); > + FreePool (HttpIoHeader->Headers[Index].FieldValue); > + } > + } > + FreePool (HttpIoHeader); > + } > +} > + > +/** > + Set or update a HTTP header with the field name and corresponding value. > + > + @param[in] HttpIoHeader Point to the HTTP header holder. > + @param[in] FieldName Null terminated string which describes a field name. > + @param[in] FieldValue Null terminated string which describes the corresponding field value. > + > + @retval EFI_SUCCESS The HTTP header has been set or updated. > + @retval EFI_INVALID_PARAMETER Any input parameter is invalid. > + @retval EFI_OUT_OF_RESOURCES Insufficient resource to complete the operation. > + @retval Other Unexpected error happened. > + > +**/ > +EFI_STATUS > +HttpIoSetHeader ( > + IN HTTP_IO_HEADER *HttpIoHeader, > + IN CHAR8 *FieldName, > + IN CHAR8 *FieldValue > + ) > +{ > + EFI_HTTP_HEADER *Header; > + UINTN StrSize; > + CHAR8 *NewFieldValue; > + > + if (HttpIoHeader == NULL || FieldName == NULL || FieldValue == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Header = HttpFindHeader (HttpIoHeader->HeaderCount, HttpIoHeader->Headers, FieldName); > + if (Header == NULL) { > + // > + // Add a new header. > + // > + if (HttpIoHeader->HeaderCount >= HttpIoHeader->MaxHeaderCount) { > + return EFI_OUT_OF_RESOURCES; > + } > + Header = &HttpIoHeader->Headers[HttpIoHeader->HeaderCount]; > + > + StrSize = AsciiStrSize (FieldName); > + Header->FieldName = AllocatePool (StrSize); > + if (Header->FieldName == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + CopyMem (Header->FieldName, FieldName, StrSize); > + Header->FieldName[StrSize -1] = '\0'; > + > + StrSize = AsciiStrSize (FieldValue); > + Header->FieldValue = AllocatePool (StrSize); > + if (Header->FieldValue == NULL) { > + FreePool (Header->FieldName); > + return EFI_OUT_OF_RESOURCES; > + } > + CopyMem (Header->FieldValue, FieldValue, StrSize); > + Header->FieldValue[StrSize -1] = '\0'; > + > + HttpIoHeader->HeaderCount++; > + } else { > + // > + // Update an existing one. > + // > + StrSize = AsciiStrSize (FieldValue); > + NewFieldValue = AllocatePool (StrSize); > + if (NewFieldValue == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + CopyMem (NewFieldValue, FieldValue, StrSize); > + NewFieldValue[StrSize -1] = '\0'; > + > + if (Header->FieldValue != NULL) { > + FreePool (Header->FieldValue); > + } > + Header->FieldValue = NewFieldValue; > + } > + > + return EFI_SUCCESS; > +} > +