* [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare @ 2018-08-07 9:14 Ruiyu Ni 2018-08-07 17:57 ` Carsey, Jaben 0 siblings, 1 reply; 4+ messages in thread From: Ruiyu Ni @ 2018-08-07 9:14 UTC (permalink / raw) To: edk2-devel; +Cc: Jaben Carsey, Jim Dailey REF: https://bugzilla.tianocore.org/show_bug.cgi?id=777 Per Shell spec, the environment variable has a case-sensitive name. But today's implementation of EfiShellSetEnv() compares the environment variable name case insensitively, which causes variable like "CWD" cannot be set due to "cwd" is pre-defined variable. The patch fixes this issue. The EfiShellGetEnv() doesn't have such issue because it will call into ShellFindEnvVarInList() which uses StrCmp(). Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Cc: Jim Dailey <jim_dailey@dell.com> --- ShellPkg/Application/Shell/ShellProtocol.c | 39 +++++++----------------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c index f2ca2029e3..767cbc99a0 100644 --- a/ShellPkg/Application/Shell/ShellProtocol.c +++ b/ShellPkg/Application/Shell/ShellProtocol.c @@ -2924,36 +2924,15 @@ EfiShellSetEnv( // // Make sure we dont 'set' a predefined read only variable // - if (gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"cwd") == 0 - ||gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"Lasterror") == 0 - ||gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"profiles") == 0 - ||gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"uefishellsupport") == 0 - ||gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"uefishellversion") == 0 - ||gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - L"uefiversion") == 0 - ||(!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && - gUnicodeCollation->StriColl( - gUnicodeCollation, - (CHAR16*)Name, - (CHAR16*)mNoNestingEnvVarName) == 0) - ){ + if ((StrCmp (Name, L"cwd") == 0) || + (StrCmp (Name, L"Lasterror") == 0) || + (StrCmp (Name, L"profiles") == 0) || + (StrCmp (Name, L"uefishellsupport") == 0) || + (StrCmp (Name, L"uefishellversion") == 0) || + (StrCmp (Name, L"uefiversion") == 0) || + (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && + StrCmp (Name, mNoNestingEnvVarName) == 0) + ) { return (EFI_INVALID_PARAMETER); } return (InternalEfiShellSetEnv(Name, Value, Volatile)); -- 2.16.1.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare 2018-08-07 9:14 [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare Ruiyu Ni @ 2018-08-07 17:57 ` Carsey, Jaben 2018-08-07 18:27 ` Jim.Dailey 0 siblings, 1 reply; 4+ messages in thread From: Carsey, Jaben @ 2018-08-07 17:57 UTC (permalink / raw) To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Jim Dailey Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Ni, Ruiyu > Sent: Tuesday, August 07, 2018 2:14 AM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Jim Dailey > <jim_dailey@dell.com> > Subject: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive > compare > Importance: High > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=777 > > Per Shell spec, the environment variable has a case-sensitive name. > But today's implementation of EfiShellSetEnv() compares the > environment variable name case insensitively, which causes variable > like "CWD" cannot be set due to "cwd" is pre-defined variable. > > The patch fixes this issue. > > The EfiShellGetEnv() doesn't have such issue because it will > call into ShellFindEnvVarInList() which uses StrCmp(). > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Cc: Jaben Carsey <jaben.carsey@intel.com> > Cc: Jim Dailey <jim_dailey@dell.com> > --- > ShellPkg/Application/Shell/ShellProtocol.c | 39 +++++++----------------------- > 1 file changed, 9 insertions(+), 30 deletions(-) > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c > b/ShellPkg/Application/Shell/ShellProtocol.c > index f2ca2029e3..767cbc99a0 100644 > --- a/ShellPkg/Application/Shell/ShellProtocol.c > +++ b/ShellPkg/Application/Shell/ShellProtocol.c > @@ -2924,36 +2924,15 @@ EfiShellSetEnv( > // > // Make sure we dont 'set' a predefined read only variable > // > - if (gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"cwd") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"Lasterror") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"profiles") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefishellsupport") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefishellversion") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefiversion") == 0 > - ||(!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > - gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - (CHAR16*)mNoNestingEnvVarName) == 0) > - ){ > + if ((StrCmp (Name, L"cwd") == 0) || > + (StrCmp (Name, L"Lasterror") == 0) || > + (StrCmp (Name, L"profiles") == 0) || > + (StrCmp (Name, L"uefishellsupport") == 0) || > + (StrCmp (Name, L"uefishellversion") == 0) || > + (StrCmp (Name, L"uefiversion") == 0) || > + (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > + StrCmp (Name, mNoNestingEnvVarName) == 0) > + ) { > return (EFI_INVALID_PARAMETER); > } > return (InternalEfiShellSetEnv(Name, Value, Volatile)); > -- > 2.16.1.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare 2018-08-07 17:57 ` Carsey, Jaben @ 2018-08-07 18:27 ` Jim.Dailey 2018-08-08 5:03 ` Ni, Ruiyu 0 siblings, 1 reply; 4+ messages in thread From: Jim.Dailey @ 2018-08-07 18:27 UTC (permalink / raw) To: ruiyu.ni; +Cc: edk2-devel, jaben.carsey Ray, Table 8 in version 2.2 of the Shell Spec says "lasterror" (lower-case L). Why compare to "Lasterror"? Regards, Jim -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com] Sent: Tuesday, August 7, 2018 12:57 PM To: Ni, Ruiyu; edk2-devel@lists.01.org Cc: Dailey, Jim Subject: RE: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > -----Original Message----- > From: Ni, Ruiyu > Sent: Tuesday, August 07, 2018 2:14 AM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Jim Dailey > <jim_dailey@dell.com> > Subject: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive > compare > Importance: High > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=777 > > Per Shell spec, the environment variable has a case-sensitive name. > But today's implementation of EfiShellSetEnv() compares the > environment variable name case insensitively, which causes variable > like "CWD" cannot be set due to "cwd" is pre-defined variable. > > The patch fixes this issue. > > The EfiShellGetEnv() doesn't have such issue because it will > call into ShellFindEnvVarInList() which uses StrCmp(). > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Cc: Jaben Carsey <jaben.carsey@intel.com> > Cc: Jim Dailey <jim_dailey@dell.com> > --- > ShellPkg/Application/Shell/ShellProtocol.c | 39 +++++++----------------------- > 1 file changed, 9 insertions(+), 30 deletions(-) > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c > b/ShellPkg/Application/Shell/ShellProtocol.c > index f2ca2029e3..767cbc99a0 100644 > --- a/ShellPkg/Application/Shell/ShellProtocol.c > +++ b/ShellPkg/Application/Shell/ShellProtocol.c > @@ -2924,36 +2924,15 @@ EfiShellSetEnv( > // > // Make sure we dont 'set' a predefined read only variable > // > - if (gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"cwd") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"Lasterror") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"profiles") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefishellsupport") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefishellversion") == 0 > - ||gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - L"uefiversion") == 0 > - ||(!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > - gUnicodeCollation->StriColl( > - gUnicodeCollation, > - (CHAR16*)Name, > - (CHAR16*)mNoNestingEnvVarName) == 0) > - ){ > + if ((StrCmp (Name, L"cwd") == 0) || > + (StrCmp (Name, L"Lasterror") == 0) || > + (StrCmp (Name, L"profiles") == 0) || > + (StrCmp (Name, L"uefishellsupport") == 0) || > + (StrCmp (Name, L"uefishellversion") == 0) || > + (StrCmp (Name, L"uefiversion") == 0) || > + (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > + StrCmp (Name, mNoNestingEnvVarName) == 0) > + ) { > return (EFI_INVALID_PARAMETER); > } > return (InternalEfiShellSetEnv(Name, Value, Volatile)); > -- > 2.16.1.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare 2018-08-07 18:27 ` Jim.Dailey @ 2018-08-08 5:03 ` Ni, Ruiyu 0 siblings, 0 replies; 4+ messages in thread From: Ni, Ruiyu @ 2018-08-08 5:03 UTC (permalink / raw) To: Jim.Dailey@dell.com; +Cc: Carsey, Jaben, edk2-devel@lists.01.org It's a typo. V2 patch will correct this. Thanks/Ray > -----Original Message----- > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of > Jim.Dailey@dell.com > Sent: Wednesday, August 8, 2018 2:27 AM > To: Ni, Ruiyu <ruiyu.ni@intel.com> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case > sensitive compare > > Ray, > > Table 8 in version 2.2 of the Shell Spec says "lasterror" (lower-case L). Why > compare to "Lasterror"? > > Regards, > Jim > > -----Original Message----- > From: Carsey, Jaben [mailto:jaben.carsey@intel.com] > Sent: Tuesday, August 7, 2018 12:57 PM > To: Ni, Ruiyu; edk2-devel@lists.01.org > Cc: Dailey, Jim > Subject: RE: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive > compare > > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> > > > -----Original Message----- > > From: Ni, Ruiyu > > Sent: Tuesday, August 07, 2018 2:14 AM > > To: edk2-devel@lists.01.org > > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Jim Dailey > > <jim_dailey@dell.com> > > Subject: [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case > > sensitive compare > > Importance: High > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=777 > > > > Per Shell spec, the environment variable has a case-sensitive name. > > But today's implementation of EfiShellSetEnv() compares the > > environment variable name case insensitively, which causes variable > > like "CWD" cannot be set due to "cwd" is pre-defined variable. > > > > The patch fixes this issue. > > > > The EfiShellGetEnv() doesn't have such issue because it will call into > > ShellFindEnvVarInList() which uses StrCmp(). > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > > Cc: Jaben Carsey <jaben.carsey@intel.com> > > Cc: Jim Dailey <jim_dailey@dell.com> > > --- > > ShellPkg/Application/Shell/ShellProtocol.c | 39 > > +++++++----------------------- > > 1 file changed, 9 insertions(+), 30 deletions(-) > > > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c > > b/ShellPkg/Application/Shell/ShellProtocol.c > > index f2ca2029e3..767cbc99a0 100644 > > --- a/ShellPkg/Application/Shell/ShellProtocol.c > > +++ b/ShellPkg/Application/Shell/ShellProtocol.c > > @@ -2924,36 +2924,15 @@ EfiShellSetEnv( > > // > > // Make sure we dont 'set' a predefined read only variable > > // > > - if (gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"cwd") == 0 > > - ||gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"Lasterror") == 0 > > - ||gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"profiles") == 0 > > - ||gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"uefishellsupport") == 0 > > - ||gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"uefishellversion") == 0 > > - ||gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - L"uefiversion") == 0 > > - ||(!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > > - gUnicodeCollation->StriColl( > > - gUnicodeCollation, > > - (CHAR16*)Name, > > - (CHAR16*)mNoNestingEnvVarName) == 0) > > - ){ > > + if ((StrCmp (Name, L"cwd") == 0) || > > + (StrCmp (Name, L"Lasterror") == 0) || > > + (StrCmp (Name, L"profiles") == 0) || > > + (StrCmp (Name, L"uefishellsupport") == 0) || > > + (StrCmp (Name, L"uefishellversion") == 0) || > > + (StrCmp (Name, L"uefiversion") == 0) || > > + (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && > > + StrCmp (Name, mNoNestingEnvVarName) == 0) > > + ) { > > return (EFI_INVALID_PARAMETER); > > } > > return (InternalEfiShellSetEnv(Name, Value, Volatile)); > > -- > > 2.16.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-08 5:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-07 9:14 [PATCH] ShellPkg/set: Fix EfiShellSetEnv to use case sensitive compare Ruiyu Ni 2018-08-07 17:57 ` Carsey, Jaben 2018-08-07 18:27 ` Jim.Dailey 2018-08-08 5:03 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox