Re: [edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

2020-07-02 Thread Vladimir Olovyannikov via groups.io
Hi Zhichao,

Thank you for reviewing the patch.
I will send v3 tomorrow addressing formatting and variable scope issues.

Thank you,
Vladimir
-Original Message-
From: Gao, Zhichao 
Sent: Wednesday, July 1, 2020 10:55 PM
To: devel@edk2.groups.io; vladimir.olovyanni...@broadcom.com
Cc: Samer El-Haj-Mahmoud ; Maciej Rabeda
; Wu, Jiaxin ; Fu,
Siyuan ; Ni, Ray ; Gao, Liming
; Nd 
Subject: RE: [edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add
HttpDynamicCommand

Hi,

Sorry for the delay. As I said in the V1, the patch required the
NetworkPkg maintainers' help to review the network connection
implementation part.

Some comments below.
1. for function RunHttp:
'''
UINTN  StartSize;
CHAR16 *Walker;
CHAR16 *VStr;
'''
The above variable is block scope which is strongly discouraged. See
CSS_2_1_Draft Section 5.4.1.1.

2. Some indentations need adjust:
  a) GetResponse function body
  b) "if (!gHttpError) {" section of GetResponse
  c) " ValueStr = ShellCommandLineGetValue (CheckPackage, L"-s");" the if
section below this statement

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Vladimir
> Olovyannikov via groups.io
> Sent: Tuesday, May 12, 2020 2:03 AM
> To: devel@edk2.groups.io
> Cc: Vladimir Olovyannikov ; Samer
El-
> Haj-Mahmoud ; Gao, Zhichao
> ; Maciej Rabeda ;
Wu,
> Jiaxin ; Fu, Siyuan ; Ni, Ray
> ; Gao, Liming ; Nd 
> Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
>
> Introduce an http client utilizing EDK2 HTTP protocol, to
> allow fast image downloading from http/https servers.
> HTTP download speed is usually faster than tftp.
> The client is based on the same approach as tftp dynamic command, and
> uses the same UEFI Shell command line parameters. This makes it easy
> integrating http into existing UEFI Shell scripts.
> Note that to enable HTTP download, feature Pcd
> gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
> be set to TRUE.
>
> Signed-off-by: Vladimir Olovyannikov

> Tested-By: Samer El-Haj-Mahmoud 
> Cc: Zhichao Gao 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Ray Ni 
> Cc: Liming Gao 
> Cc: Nd 
> ---
>  .../DynamicCommand/HttpDynamicCommand/Http.c  | 1701
> +
>  .../DynamicCommand/HttpDynamicCommand/Http.h  |   84 +
>  .../HttpDynamicCommand/Http.uni   |  113 ++
>  .../HttpDynamicCommand/HttpApp.c  |   53 +
>  .../HttpDynamicCommand/HttpApp.inf|   58 +
>  .../HttpDynamicCommand/HttpDynamicCommand.c   |  134 ++
>  .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
>  ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
>  ShellPkg/ShellPkg.dec |1 +
>  ShellPkg/ShellPkg.dsc |5 +
>  10 files changed, 2217 insertions(+)
>  create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
>
> diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> new file mode 100644
> index ..7238cc6a07cc
> --- /dev/null
> +++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> @@ -0,0 +1,1701 @@
> +/** @file
>
> +  The implementation for the 'http' Shell command.
>
> +
>
> +  Copyright (c) 2015, ARM Ltd. All rights reserved.
>
> +  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.

>
> +  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
>
> +  Copyright (c) 2020, Broadcom. All rights reserved. 
>
> +
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +**/
>
> +
>
> +#include "Http.h"
>
> +
>
> +#define IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH 32
>
> +EFI_HII_HANDLE   mHttpHiiHandle;
>
> +
>
> +/*
>
> +   Constant strings and definitions related to the message
>
> +   indicating the amount of progress in the dowloading of a HTTP file.
>
> +*/
>
> +
>
> +// Number of steps in the progression slider
>
> +#define HTTP_PROGRESS_SLIDER_STEPS  \
>
> +  ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) - 3)
>
> +
>
> +// Size in number of characters plus one (final zero) of the message to
>
> +// indicate

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

2020-07-01 Thread Gao, Zhichao
Hi,

Sorry for the delay. As I said in the V1, the patch required the NetworkPkg 
maintainers' help to review the network connection implementation part.

Some comments below.
1. for function RunHttp: 
'''
UINTN  StartSize;
CHAR16 *Walker;
CHAR16 *VStr;
'''
The above variable is block scope which is strongly discouraged. See 
CSS_2_1_Draft Section 5.4.1.1.

2. Some indentations need adjust:
  a) GetResponse function body
  b) "if (!gHttpError) {" section of GetResponse
  c) " ValueStr = ShellCommandLineGetValue (CheckPackage, L"-s");" the if 
section below this statement

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Vladimir
> Olovyannikov via groups.io
> Sent: Tuesday, May 12, 2020 2:03 AM
> To: devel@edk2.groups.io
> Cc: Vladimir Olovyannikov ; Samer El-
> Haj-Mahmoud ; Gao, Zhichao
> ; Maciej Rabeda ; Wu,
> Jiaxin ; Fu, Siyuan ; Ni, Ray
> ; Gao, Liming ; Nd 
> Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
> 
> Introduce an http client utilizing EDK2 HTTP protocol, to
> allow fast image downloading from http/https servers.
> HTTP download speed is usually faster than tftp.
> The client is based on the same approach as tftp dynamic command, and
> uses the same UEFI Shell command line parameters. This makes it easy
> integrating http into existing UEFI Shell scripts.
> Note that to enable HTTP download, feature Pcd
> gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
> be set to TRUE.
> 
> Signed-off-by: Vladimir Olovyannikov 
> Tested-By: Samer El-Haj-Mahmoud 
> Cc: Zhichao Gao 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Ray Ni 
> Cc: Liming Gao 
> Cc: Nd 
> ---
>  .../DynamicCommand/HttpDynamicCommand/Http.c  | 1701
> +
>  .../DynamicCommand/HttpDynamicCommand/Http.h  |   84 +
>  .../HttpDynamicCommand/Http.uni   |  113 ++
>  .../HttpDynamicCommand/HttpApp.c  |   53 +
>  .../HttpDynamicCommand/HttpApp.inf|   58 +
>  .../HttpDynamicCommand/HttpDynamicCommand.c   |  134 ++
>  .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
>  ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
>  ShellPkg/ShellPkg.dec |1 +
>  ShellPkg/ShellPkg.dsc |5 +
>  10 files changed, 2217 insertions(+)
>  create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> 
> diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> new file mode 100644
> index ..7238cc6a07cc
> --- /dev/null
> +++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> @@ -0,0 +1,1701 @@
> +/** @file
> 
> +  The implementation for the 'http' Shell command.
> 
> +
> 
> +  Copyright (c) 2015, ARM Ltd. All rights reserved.
> 
> +  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved. 
> 
> +  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> 
> +  Copyright (c) 2020, Broadcom. All rights reserved. 
> 
> +
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +**/
> 
> +
> 
> +#include "Http.h"
> 
> +
> 
> +#define IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH 32
> 
> +EFI_HII_HANDLE   mHttpHiiHandle;
> 
> +
> 
> +/*
> 
> +   Constant strings and definitions related to the message
> 
> +   indicating the amount of progress in the dowloading of a HTTP file.
> 
> +*/
> 
> +
> 
> +// Number of steps in the progression slider
> 
> +#define HTTP_PROGRESS_SLIDER_STEPS  \
> 
> +  ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) - 3)
> 
> +
> 
> +// Size in number of characters plus one (final zero) of the message to
> 
> +// indicate the progress of an HTTP download. The format is "[(progress 
> slider:
> 
> +// 40 characters)] (nb of KBytes downloaded so far: 7 characters) Kb". There
> 
> +// are thus the number of characters in HTTP_PROGR_FRAME[] plus 11
> characters
> 
> +// (2 // spaces, "Kb" and seven characters for the number of KBytes).
> 
> +#define HTTP_PROGRESS_MESSAGE_SIZE  \
> 
> +  ((sizeof (HT

[edk2-devel] [PATCH v2 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

2020-05-11 Thread Vladimir Olovyannikov via groups.io
Introduce an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
be set to TRUE.

Signed-off-by: Vladimir Olovyannikov 
Tested-By: Samer El-Haj-Mahmoud 
Cc: Zhichao Gao 
Cc: Maciej Rabeda 
Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Cc: Ray Ni 
Cc: Liming Gao 
Cc: Nd 
---
 .../DynamicCommand/HttpDynamicCommand/Http.c  | 1701 +
 .../DynamicCommand/HttpDynamicCommand/Http.h  |   84 +
 .../HttpDynamicCommand/Http.uni   |  113 ++
 .../HttpDynamicCommand/HttpApp.c  |   53 +
 .../HttpDynamicCommand/HttpApp.inf|   58 +
 .../HttpDynamicCommand/HttpDynamicCommand.c   |  134 ++
 .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
 ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
 ShellPkg/ShellPkg.dec |1 +
 ShellPkg/ShellPkg.dsc |5 +
 10 files changed, 2217 insertions(+)
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf

diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c 
b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
new file mode 100644
index ..7238cc6a07cc
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
@@ -0,0 +1,1701 @@
+/** @file
+  The implementation for the 'http' Shell command.
+
+  Copyright (c) 2015, ARM Ltd. All rights reserved.
+  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved. 
+  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2020, Broadcom. All rights reserved. 
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "Http.h"
+
+#define IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH 32
+EFI_HII_HANDLE   mHttpHiiHandle;
+
+/*
+   Constant strings and definitions related to the message
+   indicating the amount of progress in the dowloading of a HTTP file.
+*/
+
+// Number of steps in the progression slider
+#define HTTP_PROGRESS_SLIDER_STEPS  \
+  ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) - 3)
+
+// Size in number of characters plus one (final zero) of the message to
+// indicate the progress of an HTTP download. The format is "[(progress slider:
+// 40 characters)] (nb of KBytes downloaded so far: 7 characters) Kb". There
+// are thus the number of characters in HTTP_PROGR_FRAME[] plus 11 characters
+// (2 // spaces, "Kb" and seven characters for the number of KBytes).
+#define HTTP_PROGRESS_MESSAGE_SIZE  \
+  ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) + 12)
+
+//
+// Buffer size. Note that larger buffer does not mean better speed!
+//
+#define DEFAULT_BUF_SIZE  SIZE_32KB
+#define MAX_BUF_SIZE  SIZE_4MB
+
+#define MIN_PARAM_COUNT   2
+#define MAX_PARAM_COUNT   4
+
+#define TIMER_MAX_TIMEOUT_S   10
+
+// File name to use when URI ends with "/"
+#define DEFAULT_HTML_FILE L"index.html"
+#define DEFAULT_HTTP_PROTOL"http"
+
+// String to delete the HTTP progress message to be able to update it :
+// (HTTP_PROGRESS_MESSAGE_SIZE-1) '\b'
+#define HTTP_PROGRESS_DEL \
+  L"\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\
+\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b"
+
+#define HTTP_KB  L"\b\b\b\b\b\b\b\b\b\b"
+// Frame for the progression slider
+#define HTTP_PROGR_FRAME L"[]"
+
+// String descriptions for server errors
+STATIC CONST CHAR16 *ErrStatusDesc[] =
+{
+  L"400 Bad Request",
+  L"401 Unauthorized",
+  L"402 Payment required",
+  L"403 Forbidden",
+  L"404 Not Found",
+  L"405 Method not allowed",
+  L"406 Not acceptable",
+  L"407 Proxy authentication required",
+  L"408 Request time out",
+  L"409 Conflict",
+  L"410 Gone",
+  L"411 Length required",
+  L"412 Precondition failed",
+  L"413 Request entity too large",
+  L"414 Request URI to large",
+  L"415 Unsupported media type",
+  L"416 Requested range not satisfied",
+  L"417 Expectation failed",
+  L"500 Internal server error",
+  L"501 Not implemented",
+  L"502 Bad gateway",
+  L"503 Service unavailable",
+  L"504 Gateway timeout",
+  L"505 HTTP version not supported"
+};
+
+// Local File Handle
+STATIC SHELL_FILE_HANDLE   mFileHandle = NULL;
+
+// Path of the local file,