Reviewed-by: Jaben Carsey <jaben.car...@intel.com>
> -----Original Message-----
> From: Qiu, Shumin
> Sent: Friday, November 06, 2015 12:06 AM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin <shumin....@intel.com>; Carsey, Jaben
> <jaben.car...@intel.com>
> Subject: [PATCH] ShellPkg: Don't strip positional parameters of quotation
> marks.
> Importance: High
>
> Per Shell SPEC 2.1 'Double-quotation marks that surround arguments are not
> stripped in positional parameters'. This patch makes Shell implementation to
> follow SPEC.
>
> Cc: Jaben Carsey <jaben.car...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin <shumin....@intel.com>
> ---
> ShellPkg/Application/Shell/Shell.c | 30 ++++++-------
> ShellPkg/Application/Shell/Shell.h | 8 ----
> .../Application/Shell/ShellParametersProtocol.c | 52 ++++++++++++++++---
> ---
> .../Application/Shell/ShellParametersProtocol.h | 36 ++++++++++-----
> ShellPkg/Application/Shell/ShellProtocol.c | 2 +-
> 5 files changed, 80 insertions(+), 48 deletions(-)
>
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index cb9d969..8af6647 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1863,7 +1863,7 @@ IsValidSplit(
> return (EFI_OUT_OF_RESOURCES);
> }
> TempWalker = (CHAR16*)Temp;
> - if (!EFI_ERROR(GetNextParameter(&TempWalker, &FirstParameter,
> StrSize(CmdLine)))) {
> + if (!EFI_ERROR(GetNextParameter(&TempWalker, &FirstParameter,
> StrSize(CmdLine), TRUE))) {
> if (GetOperationType(FirstParameter) == Unknown_Invalid) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_NOT_FOUND),
> ShellInfoObject.HiiHandle, FirstParameter);
> SetLastError(SHELL_NOT_FOUND);
> @@ -2029,7 +2029,7 @@ DoHelpUpdate(
>
> Walker = *CmdLine;
> while(Walker != NULL && *Walker != CHAR_NULL) {
> - if (!EFI_ERROR(GetNextParameter(&Walker, &CurrentParameter,
> StrSize(*CmdLine)))) {
> + if (!EFI_ERROR(GetNextParameter(&Walker, &CurrentParameter,
> StrSize(*CmdLine), TRUE))) {
> if (StrStr(CurrentParameter, L"-?") == CurrentParameter) {
> CurrentParameter[0] = L' ';
> CurrentParameter[1] = L' ';
> @@ -2173,7 +2173,7 @@ RunInternalCommand(
> //
> // get the argc and argv updated for internal commands
> //
> - Status = UpdateArgcArgv(ParamProtocol, NewCmdLine, &Argv, &Argc);
> + Status = UpdateArgcArgv(ParamProtocol, NewCmdLine, Internal_Command,
> &Argv, &Argc);
> if (!EFI_ERROR(Status)) {
> //
> // Run the internal command.
> @@ -2520,7 +2520,7 @@ RunShellCommand(
> return (EFI_OUT_OF_RESOURCES);
> }
> TempWalker = CleanOriginal;
> - if (!EFI_ERROR(GetNextParameter(&TempWalker, &FirstParameter,
> StrSize(CleanOriginal)))) {
> + if (!EFI_ERROR(GetNextParameter(&TempWalker, &FirstParameter,
> StrSize(CleanOriginal), TRUE))) {
> //
> // Depending on the first parameter we change the behavior
> //
> @@ -2767,34 +2767,34 @@ RunScriptFileHandle (
> if (NewScriptFile->Argv != NULL) {
> switch (NewScriptFile->Argc) {
> default:
> - Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%9", NewScriptFile->Argv[9], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%9", NewScriptFile->Argv[9], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 9:
> - Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%8", NewScriptFile->Argv[8], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%8", NewScriptFile->Argv[8], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 8:
> - Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%7", NewScriptFile->Argv[7], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%7", NewScriptFile->Argv[7], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 7:
> - Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%6", NewScriptFile->Argv[6], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%6", NewScriptFile->Argv[6], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 6:
> - Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%5", NewScriptFile->Argv[5], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%5", NewScriptFile->Argv[5], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 5:
> - Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%4", NewScriptFile->Argv[4], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%4", NewScriptFile->Argv[4], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 4:
> - Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%3", NewScriptFile->Argv[3], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%3", NewScriptFile->Argv[3], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 3:
> - Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%2", NewScriptFile->Argv[2], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%2", NewScriptFile->Argv[2], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 2:
> - Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%1", NewScriptFile->Argv[1], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine2, CommandLine,
> PrintBuffSize, L"%1", NewScriptFile->Argv[1], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> case 1:
> - Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%0", NewScriptFile->Argv[0], FALSE, TRUE);
> + Status = ShellCopySearchAndReplace(CommandLine, CommandLine2,
> PrintBuffSize, L"%0", NewScriptFile->Argv[0], FALSE, FALSE);
> ASSERT_EFI_ERROR(Status);
> break;
> case 0:
> @@ -2944,7 +2944,7 @@ RunScriptFile (
> //
> // get the argc and argv updated for scripts
> //
> - Status = UpdateArgcArgv(ParamProtocol, CmdLine, &Argv, &Argc);
> + Status = UpdateArgcArgv(ParamProtocol, CmdLine, Script_File_Name,
> &Argv, &Argc);
> if (!EFI_ERROR(Status)) {
>
> if (Handle == NULL) {
> diff --git a/ShellPkg/Application/Shell/Shell.h
> b/ShellPkg/Application/Shell/Shell.h
> index 7c87651..5726320 100644
> --- a/ShellPkg/Application/Shell/Shell.h
> +++ b/ShellPkg/Application/Shell/Shell.h
> @@ -124,14 +124,6 @@ typedef struct {
>
> extern SHELL_INFO ShellInfoObject;
>
> -typedef enum {
> - Internal_Command,
> - Script_File_Name,
> - Efi_Application,
> - File_Sys_Change,
> - Unknown_Invalid
> -} SHELL_OPERATION_TYPES;
> -
> /**
> Converts the command line to it's post-processed form. this replaces
> variables and alias' per UEFI Shell spec.
>
> diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c
> b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> index 9c502f8..56dd792 100644
> --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> @@ -74,10 +74,12 @@ FindEndOfParameter(
>
> This will also remove all remaining ^ characters after processing.
>
> - @param[in, out] Walker pointer to string of command line. Adjusted
> to
> - reminaing command line on return
> - @param[in, out] TempParameter pointer to string of command line item
> extracted.
> - @param[in] Length buffer size of TempParameter.
> + @param[in, out] Walker pointer to string of command line.
> Adjusted
> to
> + reminaing command line on return
> + @param[in, out] TempParameter pointer to string of command line item
> extracted.
> + @param[in] Length buffer size of TempParameter.
> + @param[in] StripQuotation if TRUE then strip the quotation marks
> surrounding
> + the parameters.
>
> @return EFI_INALID_PARAMETER A required parameter was NULL or
> pointed to a NULL or empty string.
> @return EFI_NOT_FOUND A closing " could not be found on the
> specified string
> @@ -87,7 +89,8 @@ EFIAPI
> GetNextParameter(
> IN OUT CHAR16 **Walker,
> IN OUT CHAR16 **TempParameter,
> - IN CONST UINTN Length
> + IN CONST UINTN Length,
> + IN BOOLEAN StripQuotation
> )
> {
> CONST CHAR16 *NextDelim;
> @@ -161,7 +164,11 @@ DEBUG_CODE_END();
> //
> // eliminate the unescaped quote
> //
> - CopyMem ((CHAR16*)NextDelim, NextDelim + 1, StrSize (NextDelim + 1));
> + if (StripQuotation) {
> + CopyMem ((CHAR16*)NextDelim, NextDelim + 1, StrSize (NextDelim +
> 1));
> + } else{
> + NextDelim++;
> + }
> }
> }
>
> @@ -178,9 +185,11 @@ DEBUG_CODE_END();
> All special character processing (alias, environment variable, redirection,
> etc... must be complete before calling this API.
>
> - @param[in] CommandLine String of command line to parse
> - @param[in, out] Argv pointer to array of strings; one for each
> parameter
> - @param[in, out] Argc pointer to number of strings in Argv array
> + @param[in] CommandLine String of command line to parse
> + @param[in] StripQuotation if TRUE then strip the quotation marks
> surrounding
> + the parameters.
> + @param[in, out] Argv pointer to array of strings; one for each
> parameter
> + @param[in, out] Argc pointer to number of strings in Argv array
>
> @return EFI_SUCCESS the operation was sucessful
> @return EFI_OUT_OF_RESOURCES a memory allocation failed.
> @@ -189,8 +198,9 @@ EFI_STATUS
> EFIAPI
> ParseCommandLineToArgs(
> IN CONST CHAR16 *CommandLine,
> - IN OUT CHAR16 ***Argv,
> - IN OUT UINTN *Argc
> + IN BOOLEAN StripQuotation,
> + IN OUT CHAR16 ***Argv,
> + IN OUT UINTN *Argc
> )
> {
> UINTN Count;
> @@ -228,7 +238,7 @@ ParseCommandLineToArgs(
> ; Walker != NULL && *Walker != CHAR_NULL
> ; Count++
> ) {
> - if (EFI_ERROR(GetNextParameter(&Walker, &TempParameter, Size))) {
> + if (EFI_ERROR(GetNextParameter(&Walker, &TempParameter, Size,
> TRUE))) {
> break;
> }
> }
> @@ -246,7 +256,7 @@ ParseCommandLineToArgs(
> Walker = (CHAR16*)NewCommandLine;
> while(Walker != NULL && *Walker != CHAR_NULL) {
> SetMem16(TempParameter, Size, CHAR_NULL);
> - if (EFI_ERROR(GetNextParameter(&Walker, &TempParameter, Size))) {
> + if (EFI_ERROR(GetNextParameter(&Walker, &TempParameter, Size,
> StripQuotation))) {
> Status = EFI_INVALID_PARAMETER;
> goto Done;
> }
> @@ -375,6 +385,7 @@ CreatePopulateInstallShellParametersProtocol (
> // Populate Argc and Argv
> //
> Status = ParseCommandLineToArgs(FullCommandLine,
> + TRUE,
> &(*NewShellParameters)->Argv,
> &(*NewShellParameters)->Argc);
>
> @@ -1369,6 +1380,7 @@ RestoreStdInStdOutStdErr (
>
> @param[in, out] ShellParameters Pointer to parameter structure to
> modify.
> @param[in] NewCommandLine The new command line to parse and
> use.
> + @param[in] Type The type of operation.
> @param[out] OldArgv Pointer to old list of parameters.
> @param[out] OldArgc Pointer to old number of items in
> Argv list.
>
> @@ -1380,11 +1392,15 @@ EFIAPI
> UpdateArgcArgv(
> IN OUT EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters,
> IN CONST CHAR16 *NewCommandLine,
> + IN SHELL_OPERATION_TYPES Type,
> OUT CHAR16 ***OldArgv OPTIONAL,
> OUT UINTN *OldArgc OPTIONAL
> )
> {
> + BOOLEAN StripParamQuotation;
> +
> ASSERT(ShellParameters != NULL);
> + StripParamQuotation = TRUE;
>
> if (OldArgc != NULL) {
> *OldArgc = ShellParameters->Argc;
> @@ -1393,7 +1409,15 @@ UpdateArgcArgv(
> *OldArgv = ShellParameters->Argv;
> }
>
> - return (ParseCommandLineToArgs(NewCommandLine, &(ShellParameters-
> >Argv), &(ShellParameters->Argc)));
> + if (Type == Script_File_Name) {
> + StripParamQuotation = FALSE;
> + }
> +
> + return ParseCommandLineToArgs( NewCommandLine,
> + StripParamQuotation,
> + &(ShellParameters->Argv),
> + &(ShellParameters->Argc)
> + );
> }
>
> /**
> diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.h
> b/ShellPkg/Application/Shell/ShellParametersProtocol.h
> index 2fd8f8c..926f362 100644
> --- a/ShellPkg/Application/Shell/ShellParametersProtocol.h
> +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.h
> @@ -18,6 +18,14 @@
>
> #include "Shell.h"
>
> +typedef enum {
> + Internal_Command,
> + Script_File_Name,
> + Efi_Application,
> + File_Sys_Change,
> + Unknown_Invalid
> +} SHELL_OPERATION_TYPES;
> +
> /**
> creates a new EFI_SHELL_PARAMETERS_PROTOCOL instance and populates
> it and then
> installs it on our handle and if there is an existing version of the
> protocol
> @@ -66,6 +74,7 @@ CleanUpShellParametersProtocol (
>
> @param[in, out] ShellParameters pointer to parameter structure to
> modify
> @param[in] NewCommandLine the new command line to parse and
> use
> + @param[in] Type the type of operation.
> @param[out] OldArgv pointer to old list of parameters
> @param[out] OldArgc pointer to old number of items in
> Argv list
>
> @@ -77,6 +86,7 @@ EFIAPI
> UpdateArgcArgv(
> IN OUT EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters,
> IN CONST CHAR16 *NewCommandLine,
> + IN SHELL_OPERATION_TYPES Type,
> OUT CHAR16 ***OldArgv,
> OUT UINTN *OldArgc
> );
> @@ -162,9 +172,11 @@ RestoreStdInStdOutStdErr (
> parameters for inclusion in EFI_SHELL_PARAMETERS_PROTOCOL. this
> supports space
> delimited and quote surrounded parameter definition.
>
> - @param[in] CommandLine String of command line to parse
> - @param[in, out] Argv pointer to array of strings; one for each
> parameter
> - @param[in, out] Argc pointer to number of strings in Argv array
> + @param[in] CommandLine String of command line to parse
> + @param[in] StripQuotation if TRUE then strip the quotation marks
> surrounding
> + the parameters.
> + @param[in, out] Argv pointer to array of strings; one for each
> parameter
> + @param[in, out] Argc pointer to number of strings in Argv array
>
> @return EFI_SUCCESS the operation was sucessful
> @return EFI_OUT_OF_RESOURCES a memory allocation failed.
> @@ -173,8 +185,9 @@ EFI_STATUS
> EFIAPI
> ParseCommandLineToArgs(
> IN CONST CHAR16 *CommandLine,
> - IN OUT CHAR16 ***Argv,
> - IN OUT UINTN *Argc
> + IN BOOLEAN StripQuotation,
> + IN OUT CHAR16 ***Argv,
> + IN OUT UINTN *Argc
> );
>
> /**
> @@ -187,10 +200,12 @@ ParseCommandLineToArgs(
> Temp Parameter must be large enough to hold the parameter before calling
> this
> function.
>
> - @param[in, out] Walker pointer to string of command line. Adjusted
> to
> - reminaing command line on return
> - @param[in, out] TempParameter pointer to string of command line item
> extracted.
> - @param[in] Length Length of (*TempParameter) in bytes
> + @param[in, out] Walker pointer to string of command line.
> Adjusted
> to
> + reminaing command line on return
> + @param[in, out] TempParameter pointer to string of command line item
> extracted.
> + @param[in] Length Length of (*TempParameter) in bytes
> + @param[in] StripQuotation if TRUE then strip the quotation marks
> surrounding
> + the parameters.
>
> @return EFI_INALID_PARAMETER A required parameter was NULL or
> pointed to a NULL or empty string.
> @return EFI_NOT_FOUND A closing " could not be found on the
> specified string
> @@ -200,7 +215,8 @@ EFIAPI
> GetNextParameter(
> IN OUT CHAR16 **Walker,
> IN OUT CHAR16 **TempParameter,
> - IN CONST UINTN Length
> + IN CONST UINTN Length,
> + IN BOOLEAN StripQuotation
> );
>
> #endif //_SHELL_PARAMETERS_PROTOCOL_PROVIDER_HEADER_
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index fc13595..9c370cc 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -1501,7 +1501,7 @@ InternalShellExecuteDevicePath(
> ShellParamsProtocol.StdIn =
> ShellInfoObject.NewShellParametersProtocol->StdIn;
> ShellParamsProtocol.StdOut =
> ShellInfoObject.NewShellParametersProtocol->StdOut;
> ShellParamsProtocol.StdErr =
> ShellInfoObject.NewShellParametersProtocol->StdErr;
> - Status = UpdateArgcArgv(&ShellParamsProtocol, NewCmdLine, NULL,
> NULL);
> + Status = UpdateArgcArgv(&ShellParamsProtocol, NewCmdLine,
> Efi_Application, NULL, NULL);
> ASSERT_EFI_ERROR(Status);
> //
> // Replace Argv[0] with the full path of the binary we're executing:
> --
> 1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
