From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web09.2864.1610637367558331234 for ; Thu, 14 Jan 2021 07:16:07 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.43, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: /3FreB84NI1W92tfSuh2499ctzA1Il07G3r+725wLBXXJmdCkD3fPQmoY7EsvSkz0rSdS/EsIB 0i8oTiU98h5g== X-IronPort-AV: E=McAfee;i="6000,8403,9864"; a="263173201" X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="263173201" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jan 2021 07:16:06 -0800 IronPort-SDR: DeNXZGuCyHiN1qcIq2ueYQgQzmK84VJjylAFWxS5LxbCPKygIY2++cFmZhWBNTe5z0yA/R5bHk wD2kC9e4IheQ== X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="382298197" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.252.42.39]) ([10.252.42.39]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jan 2021 07:16:03 -0800 Subject: Re: [PATCH v2] NetworkPkg/DxeHttpLib: Migrate HTTP header manipulation APIs From: "Maciej Rabeda" To: Abner Chang , devel@edk2.groups.io Cc: Jiaxin Wu , Siyuan Fu , Fan Wang , Jiewen Yao , Nickle Wang , Peter O'Hanley References: <20210107065702.3523-1-abner.chang@hpe.com> Message-ID: <2bd36406-107d-eef9-3c41-ca5b179ef748@linux.intel.com> Date: Thu, 14 Jan 2021 16:16:01 +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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: pl Patch merged. PR: https://github.com/tianocore/edk2/pull/1355 Commit: https://github.com/tianocore/edk2/commit/40c4cd54213b78ef0daee2f4b186150d7ef63bb4 On 14-Jan-21 14:56, Rabeda, Maciej wrote: > Reviewed-by: Maciej Rabeda > > On 07-Jan-21 07:57, 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/HttpBootDxe/HttpBootSupport.c   | 130 -------------------- >>   NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c | 135 ++++++++++++++++++++- >>   5 files changed, 192 insertions(+), 189 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/HttpBootDxe/HttpBootSupport.c >> b/NetworkPkg/HttpBootDxe/HttpBootSupport.c >> index 93d9dfc464..37a95e031e 100644 >> --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c >> +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c >> @@ -491,136 +491,6 @@ Exit: >>       return Status; >>   } >> -/** >> -  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 ( >> -  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 >> -HttpBootFreeHeader ( >> -  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); >> -        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 >> -HttpBootSetHeader ( >> -  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; >> -} >>     /** >>     This function checks the HTTP(S) URI scheme. >> 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; >> +} >> + >