mirror of
git://git.code.sf.net/p/cdesktopenv/code
synced 2025-02-13 11:42:21 +00:00
cd - shouldn't ignore $OLDPWD when in a new scope (#249)
This bug was first reported at <https://github.com/att/ast/issues/8>. The 'cd' command currently takes the value of $OLDPWD from the wrong scope. In the following example 'cd -' will change the directory to /bin instead of /tmp: $ OLDPWD=/bin ksh93 -c 'OLDPWD=/tmp cd -' /bin src/cmd/ksh93/bltins/cd_pwd.c: - Use sh_scoped() to obtain the correct value of $OLDPWD. - Fix a use-after-free bug. Make the 'oldpwd' variable a static char that points to freeable memory. Each time cd is used, this variable is freed if it points to a freeable memory address and isn't also a pointer to shp->pwd. src/cmd/ksh93/sh/path.c: path_pwd(): - Simplify and add comments. - Scope $PWD properly. src/cmd/ksh93/tests/builtins.sh, src/cmd/ksh93/tests/leaks.sh: - Backport the ksh2020 regression tests for 'cd -' when $OLDPWD is set. - Add test for $OLDPWD and $PWD after subshare. - Add test for $PWD after 'cd'. - Add test for possible memory leak. - Add testing for 'unset' on OLDPWD and PWD. src/cmd/ksh93/COMPATIBILITY: - Add compatibility note about changes to $PWD and $OLDPWD. Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit is contained in:
Johnothan King
GitHub
parent
ed478ab7e3
commit
ca2443b58c
7 changed files with 148 additions and 41 deletions
8
NEWS
8
NEWS
|
|
@ -3,6 +3,14 @@ For full details, see the git log at: https://github.com/ksh93/ksh
|
|||
|
||||
Any uppercase BUG_* names are modernish shell bug IDs.
|
||||
|
||||
2021-03-31:
|
||||
|
||||
- Fixed a bug that caused 'cd -' to ignore the current value of $OLDPWD
|
||||
when it's set to a different directory in a new scope.
|
||||
|
||||
- Fixed a related bug that caused ksh to use the wrong value for $PWD
|
||||
when in a new scope.
|
||||
|
||||
2021-03-29:
|
||||
|
||||
- Fixed an intermittent crash that could occur in vi mode when using the 'b'
|
||||
|
|
|
|||
|
|
@ -111,6 +111,12 @@ For more details, see the NEWS file and for complete details, see the git log.
|
|||
Turning on the new --globcasedetect shell option restores
|
||||
case-insensitive globbing for case-insensitive file systems.
|
||||
|
||||
22. If $PWD or $OLDPWD are passed as invocation-local assignments to cd,
|
||||
their values are no longer altered in the outer scope when cd finishes.
|
||||
For example:
|
||||
ksh -c 'OLDPWD=/bin; OLDPWD=/tmp cd - > /dev/null; echo $OLDPWD'
|
||||
ksh -c 'cd /var; PWD=/tmp cd /usr; echo $PWD'
|
||||
now prints '/bin' followed by '/var'.
|
||||
____________________________________________________________________________
|
||||
|
||||
KSH-93 VS. KSH-88
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ int b_cd(int argc, char *argv[],Shbltin_t *context)
|
|||
register Shell_t *shp = context->shp;
|
||||
int saverrno=0;
|
||||
int rval,flag=0;
|
||||
char *oldpwd;
|
||||
static char *oldpwd;
|
||||
Namval_t *opwdnod, *pwdnod;
|
||||
if(sh_isoption(SH_RESTRICTED))
|
||||
errormsg(SH_DICT,ERROR_exit(1),e_restricted+4);
|
||||
|
|
@ -81,9 +81,16 @@ int b_cd(int argc, char *argv[],Shbltin_t *context)
|
|||
dir = argv[0];
|
||||
if(error_info.errors>0 || argc >2)
|
||||
errormsg(SH_DICT,ERROR_usage(2),"%s",optusage((char*)0));
|
||||
if(oldpwd && oldpwd!=shp->pwd && oldpwd!=e_dot)
|
||||
free(oldpwd);
|
||||
oldpwd = path_pwd(shp,0);
|
||||
opwdnod = (shp->subshell?sh_assignok(OLDPWDNOD,1):OLDPWDNOD);
|
||||
pwdnod = (shp->subshell?sh_assignok(PWDNOD,1):PWDNOD);
|
||||
opwdnod = sh_scoped(shp,OLDPWDNOD);
|
||||
pwdnod = sh_scoped(shp,PWDNOD);
|
||||
if(shp->subshell)
|
||||
{
|
||||
opwdnod = sh_assignok(opwdnod,1);
|
||||
pwdnod = sh_assignok(pwdnod,1);
|
||||
}
|
||||
if(argc==2)
|
||||
dir = sh_substitute(oldpwd,dir,argv[1]);
|
||||
else if(!dir)
|
||||
|
|
@ -216,8 +223,6 @@ success:
|
|||
nv_scan(sh_subtracktree(1),rehash,(void*)0,NV_TAGGED,NV_TAGGED);
|
||||
path_newdir(shp,shp->pathlist);
|
||||
path_newdir(shp,shp->cdpathlist);
|
||||
if(oldpwd && oldpwd!=e_dot)
|
||||
free(oldpwd);
|
||||
return(0);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@
|
|||
|
||||
#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
|
||||
#define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */
|
||||
#define SH_RELEASE_DATE "2021-03-29" /* must be in this format for $((.sh.version)) */
|
||||
#define SH_RELEASE_DATE "2021-03-31" /* must be in this format for $((.sh.version)) */
|
||||
#define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
|
||||
|
||||
/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
|
||||
|
|
|
|||
|
|
@ -244,49 +244,35 @@ static pid_t path_xargs(Shell_t *shp,const char *path, char *argv[],char *const
|
|||
char *path_pwd(Shell_t *shp,int flag)
|
||||
{
|
||||
register char *cp;
|
||||
register int count = 0;
|
||||
Namval_t *pwdnod;
|
||||
NOT_USED(flag);
|
||||
/* Don't bother if PWD already set */
|
||||
if(shp->pwd)
|
||||
return((char*)shp->pwd);
|
||||
while(1)
|
||||
/* First see if PWD variable is correct */
|
||||
pwdnod = sh_scoped(shp,PWDNOD);
|
||||
cp = nv_getval(pwdnod);
|
||||
if(!(cp && *cp=='/' && test_inode(cp,e_dot)))
|
||||
{
|
||||
/* try from lowest to highest */
|
||||
switch(count++)
|
||||
/* Check if $HOME is a path to the PWD; this ensures $PWD == $HOME
|
||||
at login, even if $HOME is a path that contains symlinks */
|
||||
cp = nv_getval(sh_scoped(shp,HOME));
|
||||
if(!(cp && *cp=='/' && test_inode(cp,e_dot)))
|
||||
{
|
||||
case 0:
|
||||
cp = nv_getval(PWDNOD);
|
||||
break;
|
||||
case 1:
|
||||
cp = nv_getval(HOME);
|
||||
break;
|
||||
case 2:
|
||||
cp = "/";
|
||||
break;
|
||||
case 3:
|
||||
{
|
||||
if(cp=getcwd(NIL(char*),0))
|
||||
{
|
||||
nv_offattr(PWDNOD,NV_NOFREE);
|
||||
_nv_unset(PWDNOD,0);
|
||||
PWDNOD->nvalue.cp = cp;
|
||||
goto skip;
|
||||
}
|
||||
break;
|
||||
}
|
||||
case 4:
|
||||
/* Get physical PWD (no symlinks) using getcwd(3), fall back to "." */
|
||||
cp = getcwd(NIL(char*),0);
|
||||
if(!cp)
|
||||
return((char*)e_dot);
|
||||
}
|
||||
if(cp && *cp=='/' && test_inode(cp,e_dot))
|
||||
break;
|
||||
/* Store in PWD variable */
|
||||
if(shp->subshell)
|
||||
pwdnod = sh_assignok(pwdnod,1);
|
||||
nv_offattr(pwdnod,NV_NOFREE);
|
||||
nv_putval(pwdnod,cp,NV_RDONLY);
|
||||
}
|
||||
if(count>1)
|
||||
{
|
||||
nv_offattr(PWDNOD,NV_NOFREE);
|
||||
nv_putval(PWDNOD,cp,NV_RDONLY);
|
||||
}
|
||||
skip:
|
||||
nv_onattr(PWDNOD,NV_NOFREE|NV_EXPORT);
|
||||
shp->pwd = (char*)(PWDNOD->nvalue.cp);
|
||||
nv_onattr(pwdnod,NV_NOFREE|NV_EXPORT);
|
||||
/* Set shell PWD */
|
||||
shp->pwd = (char*)(pwdnod->nvalue.cp);
|
||||
return(cp);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1059,5 +1059,85 @@ got=$?
|
|||
exp=1
|
||||
[[ $got == $exp ]] || err_exit "'kill %' has the wrong exit status (expected '$exp'; got '$got')"
|
||||
|
||||
# ======
|
||||
# 'cd -' should recognize the value of an overriden $OLDPWD variable
|
||||
# https://github.com/ksh93/ksh/pull/249
|
||||
# https://github.com/att/ast/issues/8
|
||||
|
||||
mkdir "$tmp/oldpwd" "$tmp/otherpwd"
|
||||
exp=$tmp/oldpwd
|
||||
OLDPWD=$exp
|
||||
cd - > /dev/null
|
||||
got=$PWD
|
||||
[[ $got == "$exp" ]] || err_exit "cd - doesn't recognize overridden OLDPWD variable" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
cd "$tmp"
|
||||
OLDPWD=$tmp/otherpwd
|
||||
got=$(OLDPWD=$tmp/oldpwd cd -)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "cd - doesn't recognize overridden OLDPWD variable if it is overridden in new scope" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
function fn
|
||||
{
|
||||
typeset OLDPWD=/tmp
|
||||
cd -
|
||||
}
|
||||
exp='/tmp'
|
||||
got=$(OLDPWD=/bin fn)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "cd - doesn't recognize overridden OLDPWD variable if it is overridden in function scope" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
function fn
|
||||
{
|
||||
typeset PWD=bug
|
||||
cd /tmp
|
||||
echo "$PWD"
|
||||
}
|
||||
exp='/tmp'
|
||||
got=$(fn)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "PWD isn't set after cd if already set in function scope" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
# $PWD should be set correctly after cd
|
||||
exp="$PWD
|
||||
$PWD"
|
||||
got=$(echo $PWD; PWD=/tmp cd /home; echo $PWD)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "PWD is incorrect after cd" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
# Test for $OLDPWD and/or $PWD leaking out of subshell
|
||||
exp='/tmp /dev'
|
||||
got=$(
|
||||
PWD=/dev
|
||||
OLDPWD=/tmp
|
||||
(
|
||||
cd /usr; cd /bin
|
||||
cd - > /dev/null
|
||||
)
|
||||
echo $OLDPWD $PWD
|
||||
)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "OLDPWD and/or PWD leak out of subshell" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
# $OLDPWD and $PWD should survive after being set in a subshare
|
||||
exp='/usr /bin'
|
||||
got=$(
|
||||
PWD=/dev
|
||||
OLDPWD=/tmp
|
||||
foo=${
|
||||
cd /usr; cd /bin
|
||||
}
|
||||
echo $OLDPWD $PWD
|
||||
)
|
||||
[[ $got == "$exp" ]] ||
|
||||
err_exit "OLDPWD and/or PWD fail to survive subshare" \
|
||||
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
|
||||
|
||||
# ======
|
||||
exit $((Errors<125?Errors:125))
|
||||
|
|
|
|||
|
|
@ -417,5 +417,27 @@ after=$(getmem)
|
|||
err_exit_if_leak 'unset PATH in subshell'
|
||||
disabled
|
||||
|
||||
# ======
|
||||
# Test for a memory leak after 'cd' (in relation to $PWD and $OLDPWD)
|
||||
original_pwd=$PWD
|
||||
before=$(getmem)
|
||||
for ((i=0; i < N; i++))
|
||||
do cd /tmp
|
||||
cd - > /dev/null
|
||||
PWD=/foo
|
||||
OLDPWD=/bar
|
||||
cd /bin
|
||||
cd /usr
|
||||
cd /home
|
||||
cd /home
|
||||
cd - > /dev/null
|
||||
unset OLDPWD PWD
|
||||
cd /bin
|
||||
cd /tmp
|
||||
done
|
||||
after=$(getmem)
|
||||
err_exit_if_leak 'PWD and/or OLDPWD changed by cd'
|
||||
cd $original_pwd
|
||||
|
||||
# ======
|
||||
exit $((Errors<125?Errors:125))
|
||||
|
|
|
|||
Loading…
Reference in a new issue