From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 87B70817EC for ; Tue, 10 Jan 2017 13:09:48 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 10 Jan 2017 13:09:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,344,1477983600"; d="scan'208";a="28718792" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by orsmga002.jf.intel.com with ESMTP; 10 Jan 2017 13:09:48 -0800 Received: from orsmsx152.amr.corp.intel.com (10.22.226.39) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 10 Jan 2017 13:09:48 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.184]) by ORSMSX152.amr.corp.intel.com ([169.254.8.156]) with mapi id 14.03.0248.002; Tue, 10 Jan 2017 13:09:47 -0800 From: "Kinney, Michael D" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Carsey, Jaben" Thread-Topic: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args with white space Thread-Index: AQHSaSRNPJSV+ZrL3UuK1bQ9sew956EwM96AgAICSxA= Date: Tue, 10 Jan 2017 21:09:47 +0000 Message-ID: References: <1483820771-22460-1-git-send-email-michael.d.kinney@intel.com> <734D49CCEBEEF84792F5B80ED585239D5B878DDC@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5B878DDC@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzJhYmU3NzgtNGU2ZC00NDZlLTkxNWUtNTgxNzU3ZDE3OTE0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Im5MbjlaeEhTSFVwcjdlT2ZBZStzMUFjT1IzZDJnR0pJR3ZyckZ0WUlZVzg9In0= x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [Patch] ShellPkg/Shell: Add double quotes to args with white space X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Jan 2017 21:09:48 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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=20 specification section you reference.=20 This change applied to the case when the Argc/Argv list is=20 used to generate a new command line string from the Argc/Argv list provided by the Shell Parameters Protocol. The command=20 line string is then passed to a function that re-parses the=20 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=20 surrounded by double quotes. Status =3D 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 ; edk2-devel@lists.01.o= rg > Cc: Carsey, Jaben ; Kinney, Michael D > > Subject: RE: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args wit= h white > space >=20 > 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 wer= e > used to preserve whitespace have been removed." >=20 > "Double-quotation marks that surround arguments are stripped before they = are > passed to the entry point of a shell application. For more information, s= ee the > Argv > member of the EFI_SHELL_PARAMETERS_PROTOCOL." >=20 > Thanks/Ray >=20 > > -----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 ; Ni, Ruiyu ; > > Kinney, Michael D > > Subject: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args with = white > > space > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D332 > > > > When the ShellLib ShellExecute() API or the Shell Protocol Execute() AP= I 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 t= he > > 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 prese= nt > > 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 > > Cc: Ruiyu Ni > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Michael D Kinney > > --- > > 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 =3D > > AllocateCopyPool(StrSize(CurrentArg), CurrentArg); > > + ShellInfoObject.ShellInitSettings.FileName =3D NULL; > > + Size =3D 0; > > + // > > + // If first argument contains a space, then add double quotes be= fore the > > argument > > + // > > + if (StrStr (CurrentArg, L" ") !=3D NULL) { > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size= , L"\"", > > 0); > > + if (ShellInfoObject.ShellInitSettings.FileName =3D=3D NULL) { > > + return (EFI_OUT_OF_RESOURCES); > > + } > > + } > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size, > > + CurrentArg, 0); > > if (ShellInfoObject.ShellInitSettings.FileName =3D=3D NULL) { > > return (EFI_OUT_OF_RESOURCES); > > } > > // > > + // If first argument contains a space, then add double quotes af= ter the > > argument > > + // > > + if (StrStr (CurrentArg, L" ") !=3D NULL) { > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileName, &Size= , L"\"", > > 0); > > + if (ShellInfoObject.ShellInitSettings.FileName =3D=3D NULL) { > > + return (EFI_OUT_OF_RESOURCES); > > + } > > + } > > + // > > // We found `file-name`. > > // > > ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoStartup =3D 1;= @@ - > > 1055,13 +1075,28 @@ ProcessCommandLine( > > // Add `file-name-options` > > for (Size =3D 0 ; LoopVar < gEfiShellParametersProtocol->Argc ; = LoopVar++) > { > > ASSERT((ShellInfoObject.ShellInitSettings.FileOptions =3D=3D N= ULL && Size > > =3D=3D 0) || (ShellInfoObject.ShellInitSettings.FileOptions !=3D NULL))= ; > > - StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, > > - &Size, > > - L" ", > > - 0); > > - if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D NULL)= { > > - SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileNa= me); > > - return (EFI_OUT_OF_RESOURCES); > > + // > > + // Add a space between arguments > > + // > > + if (ShellInfoObject.ShellInitSettings.FileOptions !=3D NULL) { > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, = &Size, > L" > > ", 0); > > + if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D NUL= L) { > > + SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.File= Name); > > + return (EFI_OUT_OF_RESOURCES); > > + } > > + } > > + // > > + // If an argumnent contains a space, then add double quotes be= fore the > > argument > > + // > > + if (StrStr (gEfiShellParametersProtocol->Argv[LoopVar], L" ") = !=3D NULL) > { > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, > > + &Size, > > + L"\"", > > + 0); > > + if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D NUL= L) { > > + SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.File= Name); > > + return (EFI_OUT_OF_RESOURCES); > > + } > > } > > StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, > > &Size, > > @@ -1071,6 +1106,19 @@ ProcessCommandLine( > > SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileNa= me); > > return (EFI_OUT_OF_RESOURCES); > > } > > + // > > + // If an argumnent contains a space, then add double quotes af= ter the > > argument > > + // > > + if (StrStr (gEfiShellParametersProtocol->Argv[LoopVar], L" ") = !=3D NULL) > { > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, > > + &Size, > > + L"\"", > > + 0); > > + if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D NUL= L) { > > + SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.File= Name); > > + 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