[edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter check for the usage of HttpBootGetFileFromCache.

Jiaxin Wu posted 1 patch 6 years, 1 month ago
Failed in applying to current master (apply log)
NetworkPkg/HttpBootDxe/HttpBootClient.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
[edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter check for the usage of HttpBootGetFileFromCache.
Posted by Jiaxin Wu 6 years, 1 month ago
The patch is to fix the incorrect parameter check for the HttpBootGetFileFromCache().

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 15e0ab9d69..b93e63bb2f 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the boot file download function.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials are licensed and made available under 
 the terms and conditions of the BSD License that accompanies this distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.                                          
@@ -434,11 +434,11 @@ HttpBootDhcp6ExtractUriInfo (
       goto Error;
     }
   }
   
   //
-  // Extract the HTTP server Ip frome URL. This is used to Check route table 
+  // Extract the HTTP server Ip from URL. This is used to Check route table 
   // whether can send message to HTTP Server Ip through the GateWay.
   //
   Status = HttpUrlGetIp6 (
              Private->BootFileUri,
              Private->BootFileUriParser,
@@ -744,23 +744,22 @@ HttpBootGetFileFromCache (
   LIST_ENTRY                  *Entry2;
   HTTP_BOOT_CACHE_CONTENT     *Cache;
   HTTP_BOOT_ENTITY_DATA       *EntityData;
   UINTN                       CopyedSize;
   
-  if (Uri == NULL || BufferSize == 0 || Buffer == NULL || ImageType == NULL) {
+  if (Uri == NULL || BufferSize == NULL || Buffer == NULL || ImageType == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   NET_LIST_FOR_EACH (Entry, &Private->CacheList) {
     Cache = NET_LIST_USER_STRUCT (Entry, HTTP_BOOT_CACHE_CONTENT, Link);
     //
     // Compare the URI to see whether we already have a cache for this file.
     //
     if ((Cache->RequestData != NULL) &&
         (Cache->RequestData->Url != NULL) &&
-        (StrCmp (Uri, Cache->RequestData->Url) == 0)) 
-    {
+        (StrCmp (Uri, Cache->RequestData->Url) == 0)) {
       //
       // Hit in cache, record image type.
       //
       *ImageType  = Cache->ImageType;
 
@@ -945,11 +944,11 @@ HttpBootGetBootFile (
   Url = AllocatePool (UrlSize * sizeof (CHAR16));
   if (Url == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
   AsciiStrToUnicodeStrS (Private->BootFileUri, Url, UrlSize);
-  if (!HeaderOnly) {
+  if (!HeaderOnly && Buffer != NULL) {
     Status = HttpBootGetFileFromCache (Private, Url, BufferSize, Buffer, ImageType);
     if (Status != EFI_NOT_FOUND) {
       FreePool (Url);
       return Status;
     }
@@ -1127,11 +1126,11 @@ HttpBootGetBootFile (
   Context.Buffer     = Buffer;
   Context.BufferSize = *BufferSize;
   Context.Cache      = Cache;
   Context.Private    = Private;
   Status = HttpInitMsgParser (
-             HeaderOnly? HttpMethodHead : HttpMethodGet,
+             HeaderOnly ? HttpMethodHead : HttpMethodGet,
              ResponseData->Response.StatusCode,
              ResponseData->HeaderCount,
              ResponseData->Headers,
              HttpBootGetBootFileCallback,
              (VOID*) &Context,
-- 
2.16.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter check for the usage of HttpBootGetFileFromCache.
Posted by Ye, Ting 6 years, 1 month ago
Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Thursday, March 1, 2018 2:30 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter check for the usage of HttpBootGetFileFromCache.

The patch is to fix the incorrect parameter check for the HttpBootGetFileFromCache().

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 15e0ab9d69..b93e63bb2f 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the boot file download function.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  This program and the accompanying materials are licensed and made available under  the terms and conditions of the BSD License that accompanies this distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.                                          
@@ -434,11 +434,11 @@ HttpBootDhcp6ExtractUriInfo (
       goto Error;
     }
   }
   
   //
-  // Extract the HTTP server Ip frome URL. This is used to Check route table 
+  // Extract the HTTP server Ip from URL. This is used to Check route 
+ table
   // whether can send message to HTTP Server Ip through the GateWay.
   //
   Status = HttpUrlGetIp6 (
              Private->BootFileUri,
              Private->BootFileUriParser, @@ -744,23 +744,22 @@ HttpBootGetFileFromCache (
   LIST_ENTRY                  *Entry2;
   HTTP_BOOT_CACHE_CONTENT     *Cache;
   HTTP_BOOT_ENTITY_DATA       *EntityData;
   UINTN                       CopyedSize;
   
-  if (Uri == NULL || BufferSize == 0 || Buffer == NULL || ImageType == NULL) {
+  if (Uri == NULL || BufferSize == NULL || Buffer == NULL || ImageType 
+ == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   NET_LIST_FOR_EACH (Entry, &Private->CacheList) {
     Cache = NET_LIST_USER_STRUCT (Entry, HTTP_BOOT_CACHE_CONTENT, Link);
     //
     // Compare the URI to see whether we already have a cache for this file.
     //
     if ((Cache->RequestData != NULL) &&
         (Cache->RequestData->Url != NULL) &&
-        (StrCmp (Uri, Cache->RequestData->Url) == 0)) 
-    {
+        (StrCmp (Uri, Cache->RequestData->Url) == 0)) {
       //
       // Hit in cache, record image type.
       //
       *ImageType  = Cache->ImageType;
 
@@ -945,11 +944,11 @@ HttpBootGetBootFile (
   Url = AllocatePool (UrlSize * sizeof (CHAR16));
   if (Url == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
   AsciiStrToUnicodeStrS (Private->BootFileUri, Url, UrlSize);
-  if (!HeaderOnly) {
+  if (!HeaderOnly && Buffer != NULL) {
     Status = HttpBootGetFileFromCache (Private, Url, BufferSize, Buffer, ImageType);
     if (Status != EFI_NOT_FOUND) {
       FreePool (Url);
       return Status;
     }
@@ -1127,11 +1126,11 @@ HttpBootGetBootFile (
   Context.Buffer     = Buffer;
   Context.BufferSize = *BufferSize;
   Context.Cache      = Cache;
   Context.Private    = Private;
   Status = HttpInitMsgParser (
-             HeaderOnly? HttpMethodHead : HttpMethodGet,
+             HeaderOnly ? HttpMethodHead : HttpMethodGet,
              ResponseData->Response.StatusCode,
              ResponseData->HeaderCount,
              ResponseData->Headers,
              HttpBootGetBootFileCallback,
              (VOID*) &Context,
--
2.16.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter check for the usage of HttpBootGetFileFromCache.
Posted by Fu, Siyuan 6 years, 1 month ago
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, March 1, 2018 2:30 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] NetworkPkg/HttpBootDxe: Correct the parameter
> check for the usage of HttpBootGetFileFromCache.
> 
> The patch is to fix the incorrect parameter check for the
> HttpBootGetFileFromCache().
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 15e0ab9d69..b93e63bb2f 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Implementation of the boot file download function.
> 
> -Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
>  http://opensource.org/licenses/bsd-license.php.
> @@ -434,11 +434,11 @@ HttpBootDhcp6ExtractUriInfo (
>        goto Error;
>      }
>    }
> 
>    //
> -  // Extract the HTTP server Ip frome URL. This is used to Check route
> table
> +  // Extract the HTTP server Ip from URL. This is used to Check route
> table
>    // whether can send message to HTTP Server Ip through the GateWay.
>    //
>    Status = HttpUrlGetIp6 (
>               Private->BootFileUri,
>               Private->BootFileUriParser,
> @@ -744,23 +744,22 @@ HttpBootGetFileFromCache (
>    LIST_ENTRY                  *Entry2;
>    HTTP_BOOT_CACHE_CONTENT     *Cache;
>    HTTP_BOOT_ENTITY_DATA       *EntityData;
>    UINTN                       CopyedSize;
> 
> -  if (Uri == NULL || BufferSize == 0 || Buffer == NULL || ImageType ==
> NULL) {
> +  if (Uri == NULL || BufferSize == NULL || Buffer == NULL || ImageType ==
> NULL) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
>    NET_LIST_FOR_EACH (Entry, &Private->CacheList) {
>      Cache = NET_LIST_USER_STRUCT (Entry, HTTP_BOOT_CACHE_CONTENT, Link);
>      //
>      // Compare the URI to see whether we already have a cache for this
> file.
>      //
>      if ((Cache->RequestData != NULL) &&
>          (Cache->RequestData->Url != NULL) &&
> -        (StrCmp (Uri, Cache->RequestData->Url) == 0))
> -    {
> +        (StrCmp (Uri, Cache->RequestData->Url) == 0)) {
>        //
>        // Hit in cache, record image type.
>        //
>        *ImageType  = Cache->ImageType;
> 
> @@ -945,11 +944,11 @@ HttpBootGetBootFile (
>    Url = AllocatePool (UrlSize * sizeof (CHAR16));
>    if (Url == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
>    AsciiStrToUnicodeStrS (Private->BootFileUri, Url, UrlSize);
> -  if (!HeaderOnly) {
> +  if (!HeaderOnly && Buffer != NULL) {
>      Status = HttpBootGetFileFromCache (Private, Url, BufferSize, Buffer,
> ImageType);
>      if (Status != EFI_NOT_FOUND) {
>        FreePool (Url);
>        return Status;
>      }
> @@ -1127,11 +1126,11 @@ HttpBootGetBootFile (
>    Context.Buffer     = Buffer;
>    Context.BufferSize = *BufferSize;
>    Context.Cache      = Cache;
>    Context.Private    = Private;
>    Status = HttpInitMsgParser (
> -             HeaderOnly? HttpMethodHead : HttpMethodGet,
> +             HeaderOnly ? HttpMethodHead : HttpMethodGet,
>               ResponseData->Response.StatusCode,
>               ResponseData->HeaderCount,
>               ResponseData->Headers,
>               HttpBootGetBootFileCallback,
>               (VOID*) &Context,
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] NetworkPkg/HttpBootDxe: Fix the incorrect error message output.
Posted by Jiaxin Wu 6 years, 1 month ago
For IPv6 case, if one invalid URL returned from DHCP server, HttpBootDxe
driver could not retrieve the URL host address from DNS server. In such a
case, the error message should be printed as:
  Error: Could not retrieve the host address from DNS server.
Instead of:
  Error: Could not discover the boot information for DHCP server.
Then, we can still output as following:
  Error: Could not retrieve NBP file size from HTTP server.

Besides, currently implementation in HttpBootLoadFile will always output
error message even the HTTP process is correct.

This patch is to fix above issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c |  1 +
 NetworkPkg/HttpBootDxe/HttpBootImpl.c   | 37 ++++++++++++++++++---------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index b93e63bb2f..1d1e47008d 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -472,10 +472,11 @@ HttpBootDhcp6ExtractUriInfo (
     }
     
     Status = HttpBootDns (Private, HostNameStr, &IpAddr);
     FreePool (HostNameStr);
     if (EFI_ERROR (Status)) {
+      AsciiPrint ("\n  Error: Could not retrieve the host address from DNS server.\n");
       goto Error;
     }  
   } 
   
   CopyMem (&Private->ServerIp.v6, &IpAddr, sizeof (EFI_IPv6_ADDRESS));
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 16c1207bf8..a0fd934ec4 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -1,9 +1,9 @@
 /** @file
   The implementation of EFI_LOAD_FILE_PROTOCOL for UEFI HTTP boot.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials are licensed and made available under 
 the terms and conditions of the BSD License that accompanies this distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.                                          
@@ -329,11 +329,11 @@ HttpBootLoadFile (
     //
     // Parse the cached offer to get the boot file URL first.
     //
     Status = HttpBootDiscoverBootInfo (Private);
     if (EFI_ERROR (Status)) {
-      AsciiPrint ("\n  Error: Could not discover the boot information for DHCP server.\n");
+      AsciiPrint ("\n  Error: Could not retrieve NBP file size from HTTP server.\n");
       goto ON_EXIT;
     }
   }
 
   if (!Private->HttpCreated) {
@@ -398,26 +398,29 @@ HttpBootLoadFile (
              ImageType
              );
   
 ON_EXIT:
   HttpBootUninstallCallback (Private);
-
-  if (Status == EFI_ACCESS_DENIED) {
-    AsciiPrint ("\n  Error: Could not establish connection with HTTP server.\n");
-  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
-    AsciiPrint ("\n  Error: Buffer size is smaller than the requested file.\n");
-  } else if (Status == EFI_OUT_OF_RESOURCES) {
-    AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
-  } else if (Status == EFI_DEVICE_ERROR) {
-    AsciiPrint ("\n  Error: Network device error.\n");
-  } else if (Status == EFI_TIMEOUT) {
-    AsciiPrint ("\n  Error: Server response timeout.\n");
-  } else if (Status == EFI_ABORTED) {
-    AsciiPrint ("\n  Error: Remote boot cancelled.\n");
-  } else if (Status != EFI_BUFFER_TOO_SMALL) {
-    AsciiPrint ("\n  Error: Unexpected network error.\n");
+  
+  if (EFI_ERROR (Status)) {
+    if (Status == EFI_ACCESS_DENIED) {
+      AsciiPrint ("\n  Error: Could not establish connection with HTTP server.\n");
+    } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
+      AsciiPrint ("\n  Error: Buffer size is smaller than the requested file.\n");
+    } else if (Status == EFI_OUT_OF_RESOURCES) {
+      AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
+    } else if (Status == EFI_DEVICE_ERROR) {
+      AsciiPrint ("\n  Error: Network device error.\n");
+    } else if (Status == EFI_TIMEOUT) {
+      AsciiPrint ("\n  Error: Server response timeout.\n");
+    } else if (Status == EFI_ABORTED) {
+      AsciiPrint ("\n  Error: Remote boot cancelled.\n");
+    } else if (Status != EFI_BUFFER_TOO_SMALL) {
+      AsciiPrint ("\n  Error: Unexpected network error.\n");
+    }
   }
+  
   return Status;
 }
 
 /**
   Disable the use of UEFI HTTP boot function.
-- 
2.16.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/HttpBootDxe: Fix the incorrect error message output.
Posted by Fu, Siyuan 6 years, 1 month ago

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, March 1, 2018 2:30 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] NetworkPkg/HttpBootDxe: Fix the incorrect error
> message output.
> 
> For IPv6 case, if one invalid URL returned from DHCP server, HttpBootDxe
> driver could not retrieve the URL host address from DNS server. In such a
> case, the error message should be printed as:
>   Error: Could not retrieve the host address from DNS server.
> Instead of:
>   Error: Could not discover the boot information for DHCP server.
> Then, we can still output as following:
>   Error: Could not retrieve NBP file size from HTTP server.
> 
> Besides, currently implementation in HttpBootLoadFile will always output
> error message even the HTTP process is correct.
> 
> This patch is to fix above issue.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c |  1 +
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c   | 37 ++++++++++++++++++----------
> -----
>  2 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index b93e63bb2f..1d1e47008d 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -472,10 +472,11 @@ HttpBootDhcp6ExtractUriInfo (
>      }
> 
>      Status = HttpBootDns (Private, HostNameStr, &IpAddr);
>      FreePool (HostNameStr);
>      if (EFI_ERROR (Status)) {
> +      AsciiPrint ("\n  Error: Could not retrieve the host address from
> DNS server.\n");
>        goto Error;
>      }
>    }
> 
>    CopyMem (&Private->ServerIp.v6, &IpAddr, sizeof (EFI_IPv6_ADDRESS));
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> index 16c1207bf8..a0fd934ec4 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> @@ -1,9 +1,9 @@
>  /** @file
>    The implementation of EFI_LOAD_FILE_PROTOCOL for UEFI HTTP boot.
> 
> -Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
>  http://opensource.org/licenses/bsd-license.php.
> @@ -329,11 +329,11 @@ HttpBootLoadFile (
>      //
>      // Parse the cached offer to get the boot file URL first.
>      //
>      Status = HttpBootDiscoverBootInfo (Private);
>      if (EFI_ERROR (Status)) {
> -      AsciiPrint ("\n  Error: Could not discover the boot information for
> DHCP server.\n");
> +      AsciiPrint ("\n  Error: Could not retrieve NBP file size from HTTP
> server.\n");
>        goto ON_EXIT;
>      }
>    }
> 
>    if (!Private->HttpCreated) {
> @@ -398,26 +398,29 @@ HttpBootLoadFile (
>               ImageType
>               );
> 
>  ON_EXIT:
>    HttpBootUninstallCallback (Private);
> -
> -  if (Status == EFI_ACCESS_DENIED) {
> -    AsciiPrint ("\n  Error: Could not establish connection with HTTP
> server.\n");
> -  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
> -    AsciiPrint ("\n  Error: Buffer size is smaller than the requested
> file.\n");
> -  } else if (Status == EFI_OUT_OF_RESOURCES) {
> -    AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
> -  } else if (Status == EFI_DEVICE_ERROR) {
> -    AsciiPrint ("\n  Error: Network device error.\n");
> -  } else if (Status == EFI_TIMEOUT) {
> -    AsciiPrint ("\n  Error: Server response timeout.\n");
> -  } else if (Status == EFI_ABORTED) {
> -    AsciiPrint ("\n  Error: Remote boot cancelled.\n");
> -  } else if (Status != EFI_BUFFER_TOO_SMALL) {
> -    AsciiPrint ("\n  Error: Unexpected network error.\n");
> +
> +  if (EFI_ERROR (Status)) {
> +    if (Status == EFI_ACCESS_DENIED) {
> +      AsciiPrint ("\n  Error: Could not establish connection with HTTP
> server.\n");
> +    } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
> +      AsciiPrint ("\n  Error: Buffer size is smaller than the requested
> file.\n");
> +    } else if (Status == EFI_OUT_OF_RESOURCES) {
> +      AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
> +    } else if (Status == EFI_DEVICE_ERROR) {
> +      AsciiPrint ("\n  Error: Network device error.\n");
> +    } else if (Status == EFI_TIMEOUT) {
> +      AsciiPrint ("\n  Error: Server response timeout.\n");
> +    } else if (Status == EFI_ABORTED) {
> +      AsciiPrint ("\n  Error: Remote boot cancelled.\n");
> +    } else if (Status != EFI_BUFFER_TOO_SMALL) {
> +      AsciiPrint ("\n  Error: Unexpected network error.\n");
> +    }
>    }
> +
>    return Status;
>  }
> 
>  /**
>    Disable the use of UEFI HTTP boot function.
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/HttpBootDxe: Fix the incorrect error message output.
Posted by Ye, Ting 6 years, 1 month ago
Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Thursday, March 1, 2018 2:30 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [edk2] [Patch] NetworkPkg/HttpBootDxe: Fix the incorrect error message output.

For IPv6 case, if one invalid URL returned from DHCP server, HttpBootDxe driver could not retrieve the URL host address from DNS server. In such a case, the error message should be printed as:
  Error: Could not retrieve the host address from DNS server.
Instead of:
  Error: Could not discover the boot information for DHCP server.
Then, we can still output as following:
  Error: Could not retrieve NBP file size from HTTP server.

Besides, currently implementation in HttpBootLoadFile will always output error message even the HTTP process is correct.

This patch is to fix above issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c |  1 +
 NetworkPkg/HttpBootDxe/HttpBootImpl.c   | 37 ++++++++++++++++++---------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index b93e63bb2f..1d1e47008d 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -472,10 +472,11 @@ HttpBootDhcp6ExtractUriInfo (
     }
     
     Status = HttpBootDns (Private, HostNameStr, &IpAddr);
     FreePool (HostNameStr);
     if (EFI_ERROR (Status)) {
+      AsciiPrint ("\n  Error: Could not retrieve the host address from 
+ DNS server.\n");
       goto Error;
     }  
   } 
   
   CopyMem (&Private->ServerIp.v6, &IpAddr, sizeof (EFI_IPv6_ADDRESS)); diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 16c1207bf8..a0fd934ec4 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -1,9 +1,9 @@
 /** @file
   The implementation of EFI_LOAD_FILE_PROTOCOL for UEFI HTTP boot.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  This program and the accompanying materials are licensed and made available under  the terms and conditions of the BSD License that accompanies this distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.                                          
@@ -329,11 +329,11 @@ HttpBootLoadFile (
     //
     // Parse the cached offer to get the boot file URL first.
     //
     Status = HttpBootDiscoverBootInfo (Private);
     if (EFI_ERROR (Status)) {
-      AsciiPrint ("\n  Error: Could not discover the boot information for DHCP server.\n");
+      AsciiPrint ("\n  Error: Could not retrieve NBP file size from 
+ HTTP server.\n");
       goto ON_EXIT;
     }
   }
 
   if (!Private->HttpCreated) {
@@ -398,26 +398,29 @@ HttpBootLoadFile (
              ImageType
              );
   
 ON_EXIT:
   HttpBootUninstallCallback (Private);
-
-  if (Status == EFI_ACCESS_DENIED) {
-    AsciiPrint ("\n  Error: Could not establish connection with HTTP server.\n");
-  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
-    AsciiPrint ("\n  Error: Buffer size is smaller than the requested file.\n");
-  } else if (Status == EFI_OUT_OF_RESOURCES) {
-    AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
-  } else if (Status == EFI_DEVICE_ERROR) {
-    AsciiPrint ("\n  Error: Network device error.\n");
-  } else if (Status == EFI_TIMEOUT) {
-    AsciiPrint ("\n  Error: Server response timeout.\n");
-  } else if (Status == EFI_ABORTED) {
-    AsciiPrint ("\n  Error: Remote boot cancelled.\n");
-  } else if (Status != EFI_BUFFER_TOO_SMALL) {
-    AsciiPrint ("\n  Error: Unexpected network error.\n");
+  
+  if (EFI_ERROR (Status)) {
+    if (Status == EFI_ACCESS_DENIED) {
+      AsciiPrint ("\n  Error: Could not establish connection with HTTP server.\n");
+    } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
+      AsciiPrint ("\n  Error: Buffer size is smaller than the requested file.\n");
+    } else if (Status == EFI_OUT_OF_RESOURCES) {
+      AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
+    } else if (Status == EFI_DEVICE_ERROR) {
+      AsciiPrint ("\n  Error: Network device error.\n");
+    } else if (Status == EFI_TIMEOUT) {
+      AsciiPrint ("\n  Error: Server response timeout.\n");
+    } else if (Status == EFI_ABORTED) {
+      AsciiPrint ("\n  Error: Remote boot cancelled.\n");
+    } else if (Status != EFI_BUFFER_TOO_SMALL) {
+      AsciiPrint ("\n  Error: Unexpected network error.\n");
+    }
   }
+  
   return Status;
 }
 
 /**
   Disable the use of UEFI HTTP boot function.
--
2.16.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel