On 08/01/18 03:54, Jiaxin Wu wrote:
> 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...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin...@intel.com>
> ---
> NetworkPkg/HttpDxe/HttpImpl.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index 17deceb395..e05ee9344b 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -403,14 +403,25 @@ 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 && AsciiStrSize (HostName) > 2 &&
> + HostName[0] == '[' && *(HostName + (AsciiStrSize (HostName) - 2)) ==
> ']') {
> + //
> + // HostName format is expressed as IPv6, so, remove '[' and ']'.
> + //
> + HostNameSize = AsciiStrSize (HostName) - 2;
> +
> + CopyMem (HostName, HostName + 1, HostNameSize - 1);
> +
> + *(HostName + HostNameSize - 1) = '\0';
> }
>
> Status = HttpUrlGetPort (Url, UrlParser, &RemotePort);
> if (EFI_ERROR (Status)) {
> if (HttpInstance->UseHttps) {
>
There are a number of expressions of the form
*(HostName + Offset)
which could be rewritten more idiomatically as
HostName[Offset]
In addition, I think the code could be optimized by calculating
AsciiStrSize() only once:
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';
}
}
Under my proposal, if the inner condition fails, then "HostNameSize"
will be set as a "side effect", but I don't think that's a problem.
Anyway, the patch seems technically correct; if you don't want to submit
a v2, I'm fine with this variant too:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
I have just one request for the subject line, before you push the patch:
please replace "stripped" with "strip".
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
