Re: [PATCH v2 1/2] NetworkPkg: Add PCD to enable the HTTP connections switch

[Laszlo Ersek <lersek@redhat.com>]

On 01/17/17 04:33, Jiaxin Wu wrote:
> v2:
> * Rename the PCD to PcdAllowHttpConnections.
> * Refine the PCD descriptions.
> 
> If the value of PcdAllowHttpConnections is TRUE, HTTP connections is
> allowed. Both the "https://" and "http://" URI schemes are permitted.
> Otherwise, HTTP connections is denied. Only the "https://" URI scheme
> is permitted.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c  | 20 +++++++-
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c  | 81 ++++++++++++++++++++------------
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |  5 +-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c | 53 ++++++++++++++++++++-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.h | 17 ++++++-
>  NetworkPkg/HttpDxe/HttpDxe.inf           |  5 +-
>  NetworkPkg/HttpDxe/HttpImpl.c            | 12 ++++-
>  NetworkPkg/NetworkPkg.dec                |  8 +++-
>  8 files changed, 164 insertions(+), 37 deletions(-)

[snip]

> diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
> index 24d45f4..d51f816 100644
> --- a/NetworkPkg/NetworkPkg.dec
> +++ b/NetworkPkg/NetworkPkg.dec
> @@ -2,11 +2,11 @@
>  # Network Package.
>  #
>  # This package provides network modules that conform to UEFI 2.4 specification.
>  #
>  # (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>  #
>  # This program and the accompanying materials are licensed and made available under
>  # the terms and conditions of the BSD License which accompanies this distribution.
>  # The full text of the license may be found at
>  # http://opensource.org/licenses/bsd-license.php
> @@ -77,10 +77,16 @@
>  
>    ## Private Key's size.
>    # @Prompt Private Key's size.
>    gEfiNetworkPkgTokenSpaceGuid.PcdIpsecUefiCertificateKeySize|0x3d5|UINT32|0x00000006
>  
> +  ## Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
> +  # TRUE  - HTTP connections is allowed. Both the "https://" and "http://" URI schemes are permitted.
> +  # FALSE - HTTP connections is denied. Only the "https://" URI scheme is permitted.
> +  # @Prompt Indicates whether HTTP connections are permitted or not.
> +  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|FALSE|BOOLEAN|0x00000008
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>    ## IPv6 DHCP Unique Identifier (DUID) Type configuration (From RFCs 3315 and 6355).
>    # 01 = DUID Based on Link-layer Address Plus Time [DUID-LLT]
>    # 04 = UUID-Based DHCPv6 Unique Identifier (DUID-UUID)
>    # 02 = DUID Assigned by Vendor Based on Enterprise Number [DUID-EN] (not supported)
> 

Minor nit: I suggest replacing

  connections is

with

  connections are

in the commit message and in the code. (I count four instances in
total.) It can be done when you commit / push the patch.

Other than that, this looks good to me, from a client platform's
perspective.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo