Re: [Patch] ShellPkg/Shell: Add double quotes to args with white space

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Ray,

This change does not impact the Argc/Argv list from the Shell
Parameters Protocol.  The double quotes are always stripped
for those and the entry point to the shell app still sees the
same input as before.  This change conforms to the shell 
specification section you reference. 

This change applied to the case when the Argc/Argv list is 
used to generate a new command line string from the Argc/Argv
list provided by the Shell Parameters Protocol.  The command 
line string is then passed to a function that re-parses the 
command line arguments into the Argc/Argv list in the Shell
Parameters Protocol.

I am seeing a real failure case when the ShellExecute()API
is used to execute a command line.  The Bugzilla issue
provided below contains sample code that reproduces the
issue.  The sample code is a request to edit a file whose
filename contains a space character.  In order to pass that
filename to the edit command, the name of the file must be 
surrounded by double quotes.

  Status = ShellExecute (
             &ImageHandle,
             L"edit \"a b.txt\"",
             FALSE,
             NULL,
             &ExecuteStatus
             );

Before this change, the following command line is
generated, and the edit command fails with an error
that there are too many parameters.

	edit a b.txt

After this change, the correct command line is generated
and the edit command correctly opens the file "a b.txt".

	edit "a b.txt"

What is the specific concern you have with this patch?

Best regards,

Mike


> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Sunday, January 8, 2017 10:17 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args with white
> space
> 
> Mike,
> According to the wording below copied from Shell spec, quotes are removed before
> saving
> To Argv.
> "Argv
> Points to an Argc-element array of points to null-terminated strings
> containing the command-line parameters. The first entry in the array is
> always the full file path of the executable. Any quotation marks that were
> used to preserve whitespace have been removed."
> 
> "Double-quotation marks that surround arguments are stripped before they are
> passed to the entry point of a shell application. For more information, see the
> Argv
> member of the EFI_SHELL_PARAMETERS_PROTOCOL."
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Michael Kinney
> > Sent: Sunday, January 8, 2017 4:26 AM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args with white
> > space
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=332
> >
> > When the ShellLib ShellExecute() API or the Shell Protocol Execute() API are
> > used to execute a command, the arguments are parsed to produce the
> > Argc/Argv list in the Shell Parameters Protocol and double quotes are
> > removed from arguments that are surrounded by double quotes.  This is the
> > required behavior of the Shell Parameters Protocol.
> >
> > The ProcessCommandLine() function in the shell implementation uses the
> > Argc/Argv list from the Shell Parameters Protocol to assemble a new
> > command line, but the double quotes that may have been originally present
> > for an argument are not preserved.
> >
> > ProcessCommandLine() is updated to check if an argument added to the
> > generated command line contains one or more white space characters, and if
> > it does, double quotes are added around the argument.
> >
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  ShellPkg/Application/Shell/Shell.c | 64
> > +++++++++++++++++++++++++++++++++-----
> >  1 file changed, 56 insertions(+), 8 deletions(-)
> >
> > diff --git a/ShellPkg/Application/Shell/Shell.c
> > b/ShellPkg/Application/Shell/Shell.c
> > index b39d81d..29e68db 100644
> > --- a/ShellPkg/Application/Shell/Shell.c
> > +++ b/ShellPkg/Application/Shell/Shell.c
> > @@ -1042,11 +1042,31 @@ ProcessCommandLine(
> >          continue;
> >        }
> >
> > -      ShellInfoObject.ShellInitSettings.FileName =
> > AllocateCopyPool(StrSize(CurrentArg), CurrentArg);
> > +      ShellInfoObject.ShellInitSettings.FileName = NULL;
> > +      Size = 0;
> > +      //
> > +      // If first argument contains a space, then add double quotes before the
> > argument
> > +      //
> > +      if (StrStr (CurrentArg, L" ") != NULL) {
> > +        StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size, L"\"",
> > 0);
> > +        if (ShellInfoObject.ShellInitSettings.FileName == NULL) {
> > +          return (EFI_OUT_OF_RESOURCES);
> > +        }
> > +      }
> > +      StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size,
> > + CurrentArg, 0);
> >        if (ShellInfoObject.ShellInitSettings.FileName == NULL) {
> >          return (EFI_OUT_OF_RESOURCES);
> >        }
> >        //
> > +      // If first argument contains a space, then add double quotes after the
> > argument
> > +      //
> > +      if (StrStr (CurrentArg, L" ") != NULL) {
> > +        StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size, L"\"",
> > 0);
> > +        if (ShellInfoObject.ShellInitSettings.FileName == NULL) {
> > +          return (EFI_OUT_OF_RESOURCES);
> > +        }
> > +      }
> > +      //
> >        // We found `file-name`.
> >        //
> >        ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoStartup = 1; @@ -
> > 1055,13 +1075,28 @@ ProcessCommandLine(
> >        // Add `file-name-options`
> >        for (Size = 0 ; LoopVar < gEfiShellParametersProtocol->Argc ; LoopVar++)
> {
> >          ASSERT((ShellInfoObject.ShellInitSettings.FileOptions == NULL && Size
> > == 0) || (ShellInfoObject.ShellInitSettings.FileOptions != NULL));
> > -        StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions,
> > -                    &Size,
> > -                    L" ",
> > -                    0);
> > -        if (ShellInfoObject.ShellInitSettings.FileOptions == NULL) {
> > -          SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName);
> > -          return (EFI_OUT_OF_RESOURCES);
> > +        //
> > +        // Add a space between arguments
> > +        //
> > +        if (ShellInfoObject.ShellInitSettings.FileOptions != NULL) {
> > +          StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, &Size,
> L"
> > ", 0);
> > +          if (ShellInfoObject.ShellInitSettings.FileOptions == NULL) {
> > +            SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName);
> > +            return (EFI_OUT_OF_RESOURCES);
> > +          }
> > +        }
> > +        //
> > +        // If an argumnent contains a space, then add double quotes before the
> > argument
> > +        //
> > +        if (StrStr (gEfiShellParametersProtocol->Argv[LoopVar], L" ") != NULL)
> {
> > +          StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions,
> > +                      &Size,
> > +                      L"\"",
> > +                      0);
> > +          if (ShellInfoObject.ShellInitSettings.FileOptions == NULL) {
> > +            SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName);
> > +            return (EFI_OUT_OF_RESOURCES);
> > +          }
> >          }
> >          StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions,
> >                      &Size,
> > @@ -1071,6 +1106,19 @@ ProcessCommandLine(
> >            SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName);
> >            return (EFI_OUT_OF_RESOURCES);
> >          }
> > +        //
> > +        // If an argumnent contains a space, then add double quotes after the
> > argument
> > +        //
> > +        if (StrStr (gEfiShellParametersProtocol->Argv[LoopVar], L" ") != NULL)
> {
> > +          StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions,
> > +                      &Size,
> > +                      L"\"",
> > +                      0);
> > +          if (ShellInfoObject.ShellInitSettings.FileOptions == NULL) {
> > +            SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName);
> > +            return (EFI_OUT_OF_RESOURCES);
> > +          }
> > +        }
> >        }
> >      }
> >    }
> > --
> > 2.6.3.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel