Re: [PATCH v2] NetworkPkg/HttpDxe: Strip square brackets in IPv6 expressed HostName.

[Laszlo Ersek <lersek@redhat.com>]

On 08/02/18 03:20, Jiaxin Wu wrote:
> *v2: Optimize the patch by calculating AsciiStrSize() only once.
> 
> In URI, the colon (:) is used to terminate the HostName path before
> a port number. However, if HostName is expressed as IPv6 format, colon
> characters in IPv6 addresses will conflict with the colon before port
> number. To alleviate this conflict in URI, the IPv6 expressed HostName
> are enclosed in square brackets ([]). To record the real IPv6 HostName,
> square brackets should be stripped.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index 17deceb395..de48243982 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -403,14 +403,26 @@ EfiHttpRequest (
>      Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, &UrlParser);
>      if (EFI_ERROR (Status)) {
>        goto Error1;
>      }
>  
> -    HostName   = NULL;
> -    Status     = HttpUrlGetHostName (Url, UrlParser, &HostName);
> +    Status = HttpUrlGetHostName (Url, UrlParser, &HostName);
>      if (EFI_ERROR (Status)) {
> -     goto Error1;
> +      goto Error1;
> +    }
> +
> +    if (HttpInstance->LocalAddressIsIPv6) {
> +      HostNameSize = AsciiStrSize (HostName);
> +
> +      if (HostNameSize > 2 && HostName[0] == '[' && HostName[HostNameSize - 2] == ']') {
> +        //
> +        // HostName format is expressed as IPv6, so, remove '[' and ']'.
> +        //
> +        HostNameSize -= 2;
> +        CopyMem (HostName, HostName + 1, HostNameSize - 1);
> +        HostName[HostNameSize - 1] = '\0';
> +      }
>      }
>  
>      Status = HttpUrlGetPort (Url, UrlParser, &RemotePort);
>      if (EFI_ERROR (Status)) {
>        if (HttpInstance->UseHttps) {
> 

Looks good to me.

Thanks!
Laszlo