The current implementation of ShellSubstituteVariables() does not allow substitution of variables names containing another variable name(s) between %...%
Example: %text%var_name%% where var_name variable contains 0. Here the variable value which should be returned on substitution would be contents of text0 variable. The current implementation would return nothing as %text0% would be eliminated by StripUnreplacedEnvironmentVariables(). Modify the code so that StripUnreplacedEnvironmentVariables checks if variable between %...% really exists. If it does not, delete %...%. If it exists, preserve it and tell the caller that another run of ShellConvertVariables() is needed. This way, any nested variable between %...% can be easily retrieved. REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2452 Signed-off-by: Vladimir Olovyannikov <vladimir.olovyanni...@broadcom.com> Contributed-under: TianoCore Contribution Agreement 1.1 --- ShellPkg/Application/Shell/Shell.c | 71 ++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 14 deletions(-) diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c index d16adae0ea30..3756a71794d1 100644 --- a/ShellPkg/Application/Shell/Shell.c +++ b/ShellPkg/Application/Shell/Shell.c @@ -1510,12 +1510,16 @@ ShellConvertAlias( /** This function will eliminate unreplaced (and therefore non-found) environment variables. - + If a variable exists between %...%, it will be preserved, and VarExists + would be TRUE. @param[in,out] CmdLine The command line to update. + @param[out] VarExists A pointer to the BOOLEAN which is set if + a Shell variable between %...% exists. **/ EFI_STATUS StripUnreplacedEnvironmentVariables( - IN OUT CHAR16 *CmdLine + IN OUT CHAR16 *CmdLine, + OUT BOOLEAN *VarExists ) { CHAR16 *FirstPercent; @@ -1558,12 +1562,38 @@ StripUnreplacedEnvironmentVariables( if (FirstQuote == NULL || SecondPercent < FirstQuote) { if (IsValidEnvironmentVariableName(FirstPercent, SecondPercent)) { // - // We need to remove from FirstPercent to SecondPercent - // - CopyMem(FirstPercent, SecondPercent + 1, StrSize(SecondPercent + 1)); - // - // don't need to update the locator. both % characters are gone. + // If this Shell variable exists, consider that another run is needed. + // For example, consider a variable %var%var2%% when var2 is 0. + // After the first run, it becomes %var0%, and after the second run + // it contains the value of var0 variable. + // Eliminate variable if it does not exist. // + CHAR16 *VarName; + + // Consider NULL terminator as well + VarName = (CHAR16 *)AllocateZeroPool((SecondPercent - FirstPercent) * + sizeof(CHAR16)); + if (VarName) { + CopyMem(VarName, FirstPercent + 1, + (SecondPercent - FirstPercent - 1) * sizeof(CHAR16)); + } + + if (!VarName || !ShellGetEnvironmentVariable(VarName)) { + // + // We need to remove from FirstPercent to SecondPercent. + // + CopyMem(FirstPercent, SecondPercent + 1, StrSize(SecondPercent + 1)); + // + // don't need to update the locator. both % characters are gone. + // + } else { + *VarExists = TRUE; + // + // In this case, locator needs to be updated as % characters present. + // + CurrentLocator = SecondPercent + 1; + } + SHELL_FREE_NON_NULL(VarName); } else { CurrentLocator = SecondPercent + 1; } @@ -1581,13 +1611,18 @@ StripUnreplacedEnvironmentVariables( If the return value is not NULL the memory must be caller freed. @param[in] OriginalCommandLine The original command line + @param[out] NeedExtraRun Indication that the caller needs to repeat + this call to convert variables as there are + existing variable(s) between %..% + present after the call. @retval NULL An error occurred. @return The new command line with no environment variables present. **/ CHAR16* ShellConvertVariables ( - IN CONST CHAR16 *OriginalCommandLine + IN CONST CHAR16 *OriginalCommandLine, + OUT BOOLEAN *NeedExtraRun ) { CONST CHAR16 *MasterEnvList; @@ -1601,11 +1636,13 @@ ShellConvertVariables ( ALIAS_LIST *AliasListNode; ASSERT(OriginalCommandLine != NULL); + ASSERT(NeedExtraRun != NULL); ItemSize = 0; NewSize = StrSize(OriginalCommandLine); CurrentScriptFile = ShellCommandGetCurrentScriptFile(); Temp = NULL; + *NeedExtraRun = FALSE; ///@todo update this to handle the %0 - %9 for scripting only (borrow from line 1256 area) ? ? ? @@ -1698,7 +1735,7 @@ ShellConvertVariables ( // // Remove non-existent environment variables // - StripUnreplacedEnvironmentVariables(NewCommandLine1); + StripUnreplacedEnvironmentVariables(NewCommandLine1, NeedExtraRun); // // Now cleanup any straggler intentionally ignored "%" characters @@ -1845,12 +1882,18 @@ ShellSubstituteVariables( ) { CHAR16 *NewCmdLine; - NewCmdLine = ShellConvertVariables(*CmdLine); - SHELL_FREE_NON_NULL(*CmdLine); - if (NewCmdLine == NULL) { - return (EFI_OUT_OF_RESOURCES); + BOOLEAN NeedExtraRun; + + NeedExtraRun = TRUE; + while (NeedExtraRun) { + NewCmdLine = ShellConvertVariables(*CmdLine, &NeedExtraRun); + SHELL_FREE_NON_NULL(*CmdLine); + if (NewCmdLine == NULL) { + return (EFI_OUT_OF_RESOURCES); + } + *CmdLine = NewCmdLine; } - *CmdLine = NewCmdLine; + return (EFI_SUCCESS); } -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#53095): https://edk2.groups.io/g/devel/message/53095 Mute This Topic: https://groups.io/mt/69589246/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
