From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 E08E081B3E for ; Tue, 10 Jan 2017 20:58:49 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP; 10 Jan 2017 20:58:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,345,1477983600"; d="scan'208";a="211979740" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 10 Jan 2017 20:58:49 -0800 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 10 Jan 2017 20:58:49 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 10 Jan 2017 20:58:49 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by shsmsx102.ccr.corp.intel.com ([169.254.2.88]) with mapi id 14.03.0248.002; Wed, 11 Jan 2017 12:58:45 +0800 From: "Ni, Ruiyu" To: "Kinney, Michael D" , "edk2-devel@lists.01.org" CC: "Carsey, Jaben" Thread-Topic: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args with white space Thread-Index: AQHSaSROaYFFe22lf0m4hqQjnxw6W6EvrTBwgAIGKYCAAQj0IA== Date: Wed, 11 Jan 2017 04:58:45 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B87B2A2@SHSMSX104.ccr.corp.intel.com> References: <1483820771-22460-1-git-send-email-michael.d.kinney@intel.com> <734D49CCEBEEF84792F5B80ED585239D5B878DDC@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] 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: Wed, 11 Jan 2017 04:58:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Mike, I have no other concerns now. Reviewed-by: Ruiyu Ni Thanks/Ray > -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, January 11, 2017 5:10 AM > To: Ni, Ruiyu ; edk2-devel@lists.01.org; Kinney, Mich= ael > D > Cc: Carsey, Jaben > Subject: RE: [edk2] [Patch] ShellPkg/Shell: Add double quotes to args wit= h > white space >=20 > Ray, >=20 > 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. >=20 > This change applied to the case when the Argc/Argv list is used to genera= te 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 functio= n > that re-parses the command line arguments into the Argc/Argv list in the > Shell Parameters Protocol. >=20 > I am seeing a real failure case when the ShellExecute()API is used to exe= cute > 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 w= hose > filename contains a space character. In order to pass that filename to t= he > edit command, the name of the file must be surrounded by double quotes. >=20 > Status =3D ShellExecute ( > &ImageHandle, > L"edit \"a b.txt\"", > FALSE, > NULL, > &ExecuteStatus > ); >=20 > Before this change, the following command line is generated, and the edit > command fails with an error that there are too many parameters. >=20 > edit a b.txt >=20 > After this change, the correct command line is generated and the edit > command correctly opens the file "a b.txt". >=20 > edit "a b.txt" >=20 > What is the specific concern you have with this patch? >=20 > Best regards, >=20 > Mike >=20 >=20 > > -----Original Message----- > > From: Ni, Ruiyu > > Sent: Sunday, January 8, 2017 10:17 PM > > To: Kinney, Michael D ; > > edk2-devel@lists.01.org > > Cc: Carsey, Jaben ; Kinney, Michael D > > > > 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 ; 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() > > > 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 > > > 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 > > > + before 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 > > > + after 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 > > > NULL && Size =3D=3D 0) || (ShellInfoObject.ShellInitSettings.FileOpti= ons !=3D > NULL)); > > > - StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions, > > > - &Size, > > > - L" ", > > > - 0); > > > - if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D NUL= L) { > > > - > SHELL_FREE_NON_NULL(ShellInfoObject.ShellInitSettings.FileName); > > > - 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 N= ULL) { > > > + > 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" > > > + ") !=3D NULL) > > { > > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions= , > > > + &Size, > > > + L"\"", > > > + 0); > > > + if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D N= ULL) { > > > + > 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" > > > + ") !=3D NULL) > > { > > > + StrnCatGrow(&ShellInfoObject.ShellInitSettings.FileOptions= , > > > + &Size, > > > + L"\"", > > > + 0); > > > + if (ShellInfoObject.ShellInitSettings.FileOptions =3D=3D N= ULL) { > > > + > 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