mirror of
git://git.code.sf.net/p/cdesktopenv/code
synced 2025-02-13 11:42:21 +00:00
234 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Johnothan King
|
086d504393
|
Lots of man page fixes and some other minor fixes (#284)
Noteworthy changes: - The man pages have been updated to fix a ton of instances of runaway underlining (this was done with `sed -i 's/\\f5/\\f3/g'` commands). This commit dramatically increased in size because of this change. - The documentation for spawnveg(3) has been extended with information about its usage of posix_spawn(3) and vfork(2). - The documentation for tmfmt(3) has been updated with the changes previously made to the man pages for the printf and date builtins (though the latter builtin is disabled by default). - The shell's tracked alias tree (hash table) is now documented in the shell(3) man page. - Removed the commented out regression test for an ERRNO variable as the COMPATIBILITY file states it was removed in ksh93. |
||
|
Johnothan King
|
2c22ace1e6
|
Fix LINENO after unsetting it a virtual subshell (#283)
There is a TODO note in variables.sh that notes the value of LINENO
is wrong after a virtual subshell. The following script should
print '6', but the bug causes it to print '1' instead:
$ cat /tmp/lineno
#!/bin/ksh
(
unset LINENO
:
)
echo $LINENO
This bug started to occur after the bugfix applied in
|
||
Martijn Dekker
|
32d1abb1ba |
shcomp: fix redirection with process substitution
The commands within a process substitution used as an argument to a
redirection (e.g. < <(...) or > >(...)) are simply not included in
parse trees dumped by shcomp. This can be verified with a command
like hexdump -C. As a result, these process substitutions do not
work when running a bytecode-compiled shell script.
The fix is surprisingly simple. A process substitution is encoded
as a complete parse tree. When used with a redirection, that parse
tree is used as the file name for the redirection. All we need to
do is treat the "file name" as a parse tree instead of a string if
flags indicate a process substitution.
A process substitution is detected by the struct ionod field
'iofile'. Checking the IOPROCSUB bit flag is not enough. We also
need to exclude the IOLSEEK flag as that form of redirection may
use the IOARITH flag which has the same bit value as IOPROCSUB (see
include/shnodes.h).
src/cmd/ksh93/sh/tdump.c: p_redirect():
- Call p_tree() instead of p_string() for a process substitution.
src/cmd/ksh93/sh/trestore.c: r_redirect():
- Call r_tree() instead of r_string() for a process substitution.
src/cmd/ksh93/include/version.h:
- Bump the shcomp binary header version as this change is not
backwards compatible; previous trestore.c versions don't know how
to read the newly compiled process substitutions and would crash.
src/cmd/ksh93/tests/io.sh:
- Add test.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh:
- Revert shcomp workarounds. (re:
|
||
|
Martijn Dekker
|
b7dde4e747 |
Fix ksh exit on syntax error in profile (re: cb67a01b, ceb77b13)
Johnothan King writes:
> There are two regressions related to how ksh handles syntax
> errors in the .kshrc file. If ~/.kshrc or the file pointed to by
> $ENV have a syntax error, ksh exits during startup. Additionally,
> the error message printed is incorrect:
>
> $ cat /tmp/synerror
> ((
> echo foo
>
> # ksh93u+m
> $ ENV=/tmp/synerror arch/*/bin/ksh -ic 'echo ${.sh.version}'
> /tmp/synerror: syntax error: `/t/tmp/synerror' unmatched
>
> # ksh93u+
> $ ENV=/tmp/synerror ksh93u -ic 'echo ${.sh.version}'
> /tmp/synerror: syntax error: `(' unmatched
> Version AJM 93u+ 2012-08-01
>
> The regression that causes the incorrect error message was
> introduced by commit
|
||
|
Martijn Dekker
|
7954855f21 |
Don't import/export readonly attribute via magic A__z env var
While automagically importing/exporting ksh variable attributes via the environment is probably a misfeature in general (now disabled for POSIX standard mode), doing so with the readonly attribute is particularly problematic. Scripts can take into account the possibility of importing unwanted attributes by unsetting or typesetting variables before using them. But there is no way for a script to get rid of an unwanted imported readonly variable. This is a possible attack vector with no possible mitigation. This commit blocks both the import and the export of the readonly attribute through the environment. I consider it a security fix. src/cmd/ksh93/sh/init.c: env_import_attributes(): - Clear NV_RDONLY from imported attributes before applying them. src/cmd/ksh93/sh/name.c: sh_envgen(): - Remove NV_RDONLY from bitmask defining attributes to export. |
||
|
Johnothan King
|
f28bce61a7
|
Fix multiple problems with the getconf builtin (#280)
This commit fixes three problems with getconf pathbound builtin: 1. The -l/--lowercase option did not change all variable names to lower case. 2. The -q/--quote option now quotes all string values. Previously, it only quoted string values that had a space or other non-shellsafe character. 3. The -c/--call, -n/--name and -s/--standard options matched all variable names provided by 'getconf -a', even if none were actual matches. Additionally, references to the confstr and sysconf functions have been updated to reference section 3 of the man pages instead of section 2. src/lib/libast/port/astconf.c: - Previously, only values that had spaces in them were quoted. Change that behavior to quote all string values by using the FMT_ALWAYS flag. Bug report: https://github.com/att/ast/issues/1173 - Not all variable names were printed in lowercase by 'getconf -l'. Fix it by adding a few missing instances of fmtlower. Bug report: https://github.com/att/ast/issues/1171 - Add the missing code to the '#if _pth_getconf_a' block to handle -c/-n/-s while parsing the OS's native 'getconf -a' output. This approach reuses code for name matching from other parts of astconflist(). Resolves: https://github.com/ksh93/ksh/issues/279 src/lib/libcmd/getconf.c: - Update the documentation to note the -q flag only quotes strings. src/cmd/ksh93/tests/bulitins.sh: - Add regression tests for the getconf bugs fixed in this commit. Co-authored-by: Martijn Dekker <martijn@inlv.org> |
||
|
Martijn Dekker
|
b0a6c1bde5 |
Further fix '<>;' and fix crash on 32-bit systems (re: 6701bb30)
Accessing t->tre.treio for every sh_exec() run is invalid because 't' is of type Shnode_t, which is a union that can contain many different kinds of structs. As all members of a union occupy the same address space, only one can be used at a time. Which member is valid to access depends on the node type sh_exec() was called with. The invalid access triggered a crash on 32-bit systems when executing an arithmetic command like ((x=1)). The t->tre.treio union member should be accessed for a simple command (case TCOM in sh_exec()). The fix is also needed for redirections attached to blocks (case TSETIO) in which case the union member to use is t->fork.forkio. src/cmd/ksh93/sh/xec.c: - Add check_exec_optimization() function that checks for all the conditions where the exec optimisation should not be done. For redirections we need to loop through the whole list to check for an IOREWRITE (<>;) one. - sh_exec(): case TCOM (simple command): Only bother to call check_exec_optimization() if there are either command arguments or redirections (IOW: don't bother for bare variable assignments), so move it to within the if(io||argn) block. - sh_exec(): case TSETIO: This needs a similar fix. To avoid the optimization breaking again if the last command is a subshell with a <>; redirection attached, we need to not only set execflg to 0 but also clear the SH_NOFORK state bit from the 'flags' variable which is passed on to the recursive sh_exec() call. src/cmd/ksh93/tests/io.sh: - Update and expand tests. Add tests for redirections attached to simple commands (TCOM) and various kinds of code block (TSETIO). Co-authored-by: Johnothan King <johnothanking@protonmail.com> Resolves: https://github.com/ksh93/ksh/issues/278 |
||
|
Martijn Dekker
|
ba43436f10 |
emacs: Fix digits input after completion (re: 16e4824c, e8b3274a)
Immediately after tab-completing the name of a directory, it is not possible to type digits after the slash; ksh eats them as it parses them as a menu selection for a nonexistent menu. Reproducer: $ mkdir -p emacstest/123abc $ cd emacste[tab]123abc Actual results: $ cd emacstest/abc Expected results: $ cd emacstest/123abc Workarounds are to press a non-numeric key followed by backspace, or hit [tab] again to get a list of options. Originally reported by Arnon Weinberg, 2012-12-23 07:15:19 UTC, at: https://bugzilla.redhat.com/889745 The fix had been partially backported from ksh 93v- by AT&T ( |
||
|
Johnothan King
|
6701bb30de
|
Fix <>; redirection for final command exec optimization (#277)
The <>; operator doesn't work correctly if it's used as the last
command of a -c script. Reproducer:
$ echo test > a; ksh -c 'echo x 1<>; a'; cat a
x
st
This bug is caused by ksh running the last command of -c scripts
with execve(2) instead of posix_spawn(3) or fork(2). The <>;
operator is noted by the man page as being incompatible with the
exec builtin (see also the ksh93u+ man page), so it's not
surprising this bug occurs when ksh runs a command using execve:
> <>;word cannot be used with the exec and redirect built-ins.
The ksh2020 fix simply removed the code required for ksh to use
this optimization at all. It's not a performance friendly fix and
only papers over the bug, so this commit provides a better fix.
This bug was first reported at:
https://github.com/att/ast/issues/9
In addition, this commit re-enables the execve(2) optimization for
the last command for scripts loaded from a file. It was enabled in
in older ksh versions, and was only disabled in interactive shells:
https://github.com/ksh93/ast-open-history/blob/2011-06-30/src/cmd/ksh93/sh/main.c#L593-L599
It was changed on 2011-12-24 to only be used for -c scripts:
https://github.com/ksh93/ast-open-history/blob/2011-12-24/src/cmd/ksh93/sh/main.c#L593-L599
We think there is no good reason why scripts loaded from a file
should be optimised less than scripts loaded from a -c argument.
They're both scripts; there's no essential difference between them.
So this commit reverts that change. If there is a bug left in the
optimization after this fix, this revert increases the chance of
exposing it so that it can be fixed.
src/cmd/ksh93/sh/xec.c:
- The IOREWRITE flag is set when handling the <>; operator, so to
fix this bug, avoid exec'ing the last command if it uses <>;. See
also commit
|
||
|
Martijn Dekker
|
519bb08265
|
Allow invoking path-bound built-in commands by direct path or preceding PATH assignment (#275)
Path-bound builtins on ksh (such as /opt/ast/bin/cat) break some
basic assumptions about paths in the shell that should hold true,
e.g., that a path output by whence -p or command -v should actually
point to an executable command. This commit should fix the
following:
1. Path-bound built-ins (such as /opt/ast/bin/cat) can now be
executed by invoking the canonical path (independently of the
value of $PATH), so the following will now work as expected:
$ /opt/ast/bin/cat --version
version cat (AT&T Research) 2012-05-31
$ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
version cat (AT&T Research) 2012-05-31
In the event an external command by that path exists, the
path-bound builtin will now override it when invoked using the
canonical path. To invoke a possible external command at that
path, you can still use a non-canonical path, e.g.:
/opt//ast/bin/cat or /opt/ast/./bin/cat
2. Path-bound built-ins will now also be found on a PATH set
locally using an assignment preceding the command, so something
like the following will now work as expected:
$ PATH=/opt/ast/bin cat --version
version cat (AT&T Research) 2012-05-31
The builtin is not found by sh_exec() because the search for
builtins happens long before invocation-local preceding
assignments are processsed. This only happens in sh_ntfork(),
before forking, or in sh_fork(), after forking. Both sh_ntfork()
and sh_fork() call path_spawn() to do the actual path search, so
a check there will cover both cases.
This does mean the builtin will be run in the forked child if
sh_fork() is used (which is the case on interactive shells with
job.jobcontrol set, or always after compiling with SHOPT_SPAWN
disabled). Searching for it before forking would mean
fundamentally redesigning that function to be basically like
sh_ntfork(), so this is hard to avoid.
src/cmd/ksh93/sh/path.c: path_spawn():
- Before doing anything else, check if the passed path appears in
the builtins tree as a pathbound builtin. If so, run it. Since a
builtin will only be found if a preceding PATH assignment
temporarily changed the PATH, and that assignment is currently in
effect, we can just sh_run() the builtin so a nested sh_exec()
invocation will find and run it.
- If 'spawn' is not set (i.e. we must return), set errno to 0 and
return -2. See the change to sh_ntfork() below.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): When searching for built-ins and the restricted option
isn't active, also search bltin_tree for names beginning with a
slash.
- sh_ntfork(): Only throw an error if the PID value returned is
exactly -1. This allows path_spawn() to return -2 after running a
built-in to tell sh_ntfork() to do the right things to restore
state.
src/cmd/ksh93/sh/parse.c: simple():
- When searching for built-ins at parse time, only exclude names
containing a slash if the restricted option is active. This
allows finding pointers to built-ins invoked by literal path like
/opt/ast/bin/cat, as long as that does not result from an
expansion. This is not actually necessary as sh_exec() will also
cover this case, but it is an optimisation.
src/lib/libcmd/getconf.c:
- Replace convoluted deferral to external command by a simple
invocation of the path to the native getconf command determined
at compile time (by src/lib/libast/comp/conf.sh). Based on:
https://github.com/ksh93/ksh/issues/138#issuecomment-816384871
If there is ever a system that has /opt/ast/bin/getconf as its
default native external 'getconf', then there would still be an
infinite recursion crash, but this seems extremely unlikely.
Resolves: https://github.com/ksh93/ksh/issues/138
|
||
|
Johnothan King
|
2c38fb93fd
|
Fix the exit status returned when a command isn't executable (#273)
Previous discussion: https://github.com/att/ast/issues/485 If ksh attempts to execute a non-executable command found in the PATH, in some instances the error message and return status are incorrect. In the example below, ksh returns with exit status 126 when using the -c execve(2) optimization or when using fork(2) in an interactive shell. However, using posix_spawn(3) causes the exit status to change: $ echo 'print cannot execute' > /tmp/x # Runs command with spawnveg (i.e., posix_spawn or vfork) $ ksh -c 'PATH=/tmp; x; echo $?' ksh: x: not found 127 # Runs command with execve $ ksh -c 'PATH=/tmp; x'; echo $? ksh: x: cannot execute [Permission denied] 126 # Runs command with fork $ ksh -ic 'PATH=/tmp; x; echo $?' ksh: x: cannot execute [Permission denied] 126 Since 'x' is in the PATH but can't be executed, the correct exit status is 126, not 127. It's worth noting this bug doesn't cause the regression tests to fail with ksh93u+m, but it does cause one test to fail when run under dtksh: path.sh[706]: Long nonexistent command name: got status 126, '' This commit backports various fixes for this bug from ksh2020, with additional fixes applied (since there were still some additional issues the ksh2020 patch didn't fix). The lacking regression test for exit status 126 in path.sh has been rewritten to test for more scenarios where ksh failed to return the correct error message and/or exit status. I can also confirm with this patch applied the path.sh regression tests now pass when run under dtksh. src/cmd/ksh93/sh/path.c: - Add a comment to path_absolute() describing 'oldpp' is the current pointer in the while loop and 'pp' is the next pointer. Backported from: https://github.com/att/ast/commit/a6cad450 - The patch from ksh2020 didn't fix this bug in the SHOPT_SPAWN code (because ksh2020 prefers fork(2)), so issues with the exit status could still occur when using spawnveg. To fix this, always set 'noexec' to the value of errno if can_execute fails. Before this fix, errno was discarded if 'pp' was a null pointer and can_execute failed. - If a command couldn't be executed and the error wasn't ENOENT, save errno in a 'not_executable' variable. If an executable command couldn't be found in the PATH, exit with status 126 and set errno to the saved value. This was based on a ksh2020 bugfix, but it has been reworked a little bit to fix a bug that caused a mismatch between the error message shown and errno. Example with a non-executable file in PATH: $ nonexec ksh2020: nonexec: cannot execute [No such file or directory] The ksh2020 patch: <https://github.com/att/ast/pull/493> - Backport a ksh2020 bugfix for directories in the PATH when running one of the added regression tests on OpenBSD: https://github.com/att/ast/pull/767 src/cmd/ksh93/data/msg.c, src/cmd/ksh93/include/shell.h, src/cmd/ksh93/sh/{path,xec}.c: - If a command name is too long (ENAMETOOLONG), then it wasn't found in the PATH. For that case return exit status 127, like for ENOENT. src/cmd/ksh93/tests/path.sh: - Replace the old test with a new set of more extensive tests. These tests check the error message and exit status when ksh attempts to run a command using any of the following: - execve(2), used with the last command run with -c (*A tests). - posix_spawn(3)/vfork(2), used in noninteractive scripts (*B tests). - fork(2), used in interactive shells with job control (*C tests). - command -x (*D tests). - exec(1) (*E tests). - Add a regression test from ksh2020 for attempting to execute a directory: https://github.com/att/ast/pull/758 src/lib/libast/include/ast.h, src/lib/libast/include/wait.h: - Avoid bitshifts in macros for static error codes. The return values of command not found and exec related errors are static values and should not require any macro magic for calculation. Backported from: https://github.com/att/ast/commit/c073b102 - Simplify EXIT_* and W* macros to use 8 bits. |
||
hyenias
|
d6ddd89053
|
Correct memory fault when removing default nameref KSH_VERSION (#271)
This commit fixes a segmentation fault when an attempt was made to unset the default KSH_VERSION variable prior any other nameref activity such as creating another nameref or even reassigning the nameref KSH_VERSION to something else. (new shell without prior nameref activity) $ nameref KSH_VERSION=.sh.version $ unset -n KSH_VERSION Memory fault src/cmd/ksh93/sh/name.c: _nv_unset(): - Add a 'Refdict' check before attempting to remove a value from it as apparently one does not exist until some sort of nameref activity occurs after shell startup as the default nameref of 'KSH_VERSION=.sh.version' does not create one. |
||
|
Johnothan King
|
75796a9c75
|
Fix += operator regressions (re: fae8862c) (#270)
The bugfix for BUG_CMDSPASGN backported in commit
|
||
|
Martijn Dekker
|
d50d3d7c4c |
Reset arithmetic recursion level on all errors (re: 264ba48b)
The recursion level for arithmetic expressions is kept track of in
a static 'level' variable in streval.c. It is reset when arithmetic
expressions throw an error.
But an error for an arithmetic expression may also occur elsewhere
-- at least in one case: when an arithmetic expression attempts to
change a read-only variable. In that case, the recursion level is
never reset because that code does not have access to the static
'level' variable.
If many such conditions occur (as in the new readonly.sh regression
tests), an arithmetic command like 'i++' may eventually fail with a
'recursion too deep' error.
To mitigate the problem, MAXLEVEL in streval.c was changed from 9
to 1024 in
|
||
|
Johnothan King
|
5461f11968
|
Fix handling of '--posix' and '--default' (#265)
src/cmd/ksh93/sh/args.c: sh_argopts():
- Remove special-casing for --posix (see also data/builtins.c) and
move the case -5: to the case ':' instead, so this option is
handled like all other long options. This change fixes two bugs:
1. 'set --posix' had no effect on the letoctal or braceexpand
options. Reproducer:
$ set --posix
$ [[ -o braceexpand ]]; echo $?
0
$ [[ -o letoctal ]]; echo $?
1
2. 'ksh --posix' could not run scripts correctly because it
wrongly enabled '-c'. Reproducer:
$ ksh --posix < <(echo 'exit 0')
ksh: -c requires argument
Usage: ksh [--posix] [arg ...]
Help: ksh [ --help | --man ] 2>&1
- Don't allow 'set --default' to unset the restricted option.
src/cmd/ksh93/tests/options.sh:
- Add regression tests for the bugs described above, using -o posix
and --posix.
src/cmd/ksh93/tests/restricted.sh:
- Add a regression test for 'set --default' in rksh.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
|
||
|
Johnothan King
|
504cbda269
|
Fix 'printf %T' ignoring the current locale in LC_TIME (#263)
src/lib/libast/tm/tmlocale.c: - Load the locale set by LC_TIME or LC_ALL if it hasn't been loaded before or if it was loaded previously but isn't the current locale. src/cmd/ksh93/tests/locale.sh: - Add a regression test using the nl_NL.UTF-8 and ja_JP.UTF-8 locales. Fixes: https://github.com/ksh93/ksh/issues/261 |
||
|
Johnothan King
|
a065558291
|
Fix more compiler warnings, typos and other minor issues (#260)
Many of these changes are minor typo fixes. The other changes (which are mostly compiler warning fixes) are: NEWS: - The --globcasedetect shell option works on older Linux kernels when used with FAT32/VFAT file systems, so remove the note about it only working with 5.2+ kernels. src/cmd/ksh93/COMPATIBILITY: - Update the documentation on function scoping with an addition from ksh93v- (this does apply to ksh93u+). src/cmd/ksh93/edit/emacs.c: - Check for '_AST_ksh_release', not 'AST_ksh_release'. src/cmd/INIT/mamake.c, src/cmd/INIT/ratz.c, src/cmd/INIT/release.c, src/cmd/builtin/pty.c: - Add more uses of UNREACHABLE() and noreturn, this time for the build system and pty. src/cmd/builtin/pty.c, src/cmd/builtin/array.c, src/cmd/ksh93/sh/name.c, src/cmd/ksh93/sh/nvtype.c, src/cmd/ksh93/sh/suid_exec.c: - Fix six -Wunused-variable warnings (the name.c nv_arrayptr() fixes are also in ksh93v-). - Remove the unused 'tableval' function to fix a -Wunused-function warning. src/cmd/ksh93/sh/lex.c: - Remove unused 'SHOPT_DOS' code, which isn't enabled anywhere. https://github.com/att/ast/issues/272#issuecomment-354363112 src/cmd/ksh93/bltins/misc.c, src/cmd/ksh93/bltins/trap.c, src/cmd/ksh93/bltins/typeset.c: - Add dictionary generator function declarations for former aliases that are now builtins (re: |
||
|
Martijn Dekker
|
2e5b625915 |
Allow path-bound builtins on restricted shells
If a system administrator prefixes /opt/ast/bin to the path and then invokes the shell in restricted mode, they clearly intend for the user to run those AST utilities. Similarly, if a system administrator sets a PATH for a restricted shell that includes libraries listed in the .paths file, they must have intended for the user to use those loadable built-ins, as they will be associated with the pathnames of their respective libraries. Since the user cannot change PATH or use the builtin command, they still cannot load just any built-in they choose. src/cmd/ksh93/sh/path.c: - Remove SH_RESTRICTED check when handling path-bound builtins or dynamic libaries containining builtins in $PATH. src/cmd/ksh93/tests/builtins.sh: - Add test verifying a restricted user can use /opt/ast/bin/cat via a PATH search. Progresses: https://github.com/ksh93/ksh/issues/138 |
||
|
Johnothan King
|
0cd8646361
|
Backport bugfix for BUG_CSUBSTDO from ksh93v- 2012-08-24 (#259)
This commit fixes BUG_CSUBSTDO, which could break stdout inside of non-forking command substitutions. The breakage only occurred when stdout was closed outside of the command substitution and a file descriptor other than stdout was redirected in the command substitution (such as stderr). Thanks to the ast-open-history repo, I was able to identify and backport the bugfix from ksh93v- 2012-08-24. This backport may fix other bugs as well. On 93v- 2012-08-24 it fixed the regression below, though it was not triggered on 93u+(m). src/cmd/ksh93/tests/heredoc.sh 487 print foo > $tmp/foofile 488 x=$( $SHELL 2> /dev/null 'read <<< $(<'"$tmp"'/foofile) 2> /dev/null;print -r "$REPLY"') 489 [[ $x == foo ]] || err_exit '<<< $(<file) not working' src/cmd/ksh93/sh/io.c: sh_open(): - If the just-opened file descriptor exists in sftable and is flagged with SF_STRING (as in non-forking command substitutions, among other situations), then move the file descriptor to a number >= 10. src/cmd/ksh93/tests/io.sh: - Add a regression test for BUG_CSUBSTDO, adapted from the one in modernish. |
||
|
Johnothan King
|
b2a7ec032f
|
Add LC_TIME to the supported locale variables (#257)
The current version of 93u+m does not have proper support for the
LC_TIME variable. Setting LC_TIME has no effect on printf %T, and
if the locale is invalid no error message is shown:
$ LC_TIME=ja_JP.UTF-8
$ printf '%T\n' now
Wed Apr 7 15:18:13 PDT 2021
$ LC_TIME=invalid.locale
$ # No error message
src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/init.c:
- Add support for the $LC_TIME variable. ksh93v- attempted to add
support for LC_TIME, but the patch from that version was extended
because the variable still didn't function correctly.
src/cmd/ksh93/tests/variables.sh:
- Add LC_TIME to the regression tests for LC_* variables.
|
||
|
Martijn Dekker
|
6b9703ffdd |
Backport bugfixes for arrays of 'enum' types from ksh 93v- beta
These fixes are applied rather blindly as no one has yet managed to
understand the almost entirely uncommented arrays and variables
handling code (arrays.c, name.c, nvdisc.c, nvtree.c, nvtype.c).
Hopefully we'll figure all that out at some point. In the meantime
these backported fixes appear to work fine, and these bugs impact
the usability of 'enum', so I'm just going to have to violate my
own policy and backport these fixes without understanding them.
Thanks to @JohnoKing for putting in a lot of work tracing these.
Further discussion at: https://github.com/ksh93/ksh/issues/87
src/cmd/ksh93/sh/array.c:
- nv_arraysettype():
* Further simplify the function. After my initial simplification
of it (re:
|
||
|
Martijn Dekker
|
db2b1affdf |
Fix unsetting array element after expanding array subscript range
Simple reproducer:
set -A arr a b c d; : ${arr[1..2]}; unset arr[1]; echo ${arr[@]}
Output:
a
Expected output:
a c d
The ${arr[1..2]} expansion broke the subsequent 'unset' command
so that it unsets element 1 and on, instead of only 1.
This regression was introduced in nv_endsubscript() on 2009-07-31:
https://github.com/ksh93/ast-open-history/commit/c47896b4/src/cmd/ksh93/sh/array.c
That change checks for the ARRAY_SCAN attribute which enables
processing ranges of array elements instead of single array
elements, and restores it after. That restore is evidently not
correct as it causes the subsequent unset command to malfunction.
If we revert that change, the bug disappears and the regression
tests show no failures. However, I don't know what this was meant
to accomplish and what other bug we might introduce by reverting
this. However, no corresponding regression test was added along
with the 2009-07-31 change, nor is there any corresponding message
in the changelog. So this looks to be one of those mystery changes
that we'll never know the reason for.
Since we currently have proof that this change causes breakage and
no evidence that it fixes anything, I'll go ahead and revert it
(and add a regression test, of course). If that causes another
regression, hopefully someone will find it at some point.
src/cmd/ksh93/sh/array.c: nv_endsubscript():
- Revert the 2009-07-31 change that saves/restores the ARRAY_SCAN
attribute.
- Keep the 'ap' pointer as it is now used by newer code. Move the
declaration up to the beginning of the block, as is customary.
src/cmd/ksh93/sh/init.c:
- Cosmetic change: remove an unused array_scan() macro that I found
when grepping the code for ARRAY_SCAN. The macro was introduced
in version 2001-06-01 but the code that used it was replaced in
version 2001-07-04, without removing the macro itself.
Resolves: https://github.com/ksh93/ksh/issues/254
|
||
|
Johnothan King
|
56b530c433
|
Fix bell character handling when redrawing command line (#250)
To set a window title in bash and zsh, the $PS1 prompt can be set
with the title placed between $'\E]0;' and $'\a':
set -o emacs # Or vi mode
typeset -A fmt=(
[start_title]=$'\E]0;'
[end_title]=$'\a'
)
PS1="${fmt[start_title]}$(hostname): $(uname)${fmt[end_title]}\$ "
This also works in ksh unless the shell receives SIGWINCH. With a
$PS1 that sets a window title, the prompt breaks until two
interrupts are received. This is caused by ed_setup() skipping
$'\a' (the bell character) when setting up the e_prompt buffer
which is an edited version of the final line of the PS1 prompt for
use when redrawing the command line.
One fix would be to avoid cutting out the bell character. But if
the prompt contains a bell, we only want the terminal to beep when
a new prompt is printed, and not upon refreshing the command line,
e.g. when receiving SIGWINCH or pressing Ctrl+L.
To avoid the problem, this commit adds code that cuts out sequences
of the form ESC ] <number> ; <text> BELL from the prompt redraw
buffer altogether. They are not needed there because these
sequences will already have taken effect when the full prompt was
printed by io_prompt().
This commit also adds a tweak that should improve the recognition
of other escape sequences to count their length.
src/cmd/ksh93/edit/edit.c: ed_setup():
- When preparing the e_prompt buffer, cut out dtterm/xterm
Operating System Commands that set window/icon title, etc.
See: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
- When counting the length of escape sequences in that part of PS1,
try to recognize some more types of sequences. These changes are
part of a ksh2020 patch: https://github.com/att/ast/issues/399
src/cmd/ksh93/sh.1:
- Document that any '!' in escape sequences in the PS1 prompt needs
to be changed to '!!'. To avoid breaking compatibility, this
requirement is documented instead of backporting the changes to
io_prompt() from https://github.com/att/ast/issues/399 which try
to remove that requirement for specific escape sequences.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
|
||
|
hyenias
|
264ba48bdd
|
Hardening of readonly variables (#239)
Ksh currently restricts readonly scalar variables from having their
values directly changed via a value assignment. However, since ksh
allows variable attributes to be altered, the variable's value can
be indirectly altered. For instance, if TMOUT=900 (for a 15 minute
idle timeout) was set to readonly, all that is needed to alter the
value of TMOUT from 900 to 0 is to issue 'typeset -R1 TMOUT',
perhaps followed by a 'typeset -i TMOUT' to turn off the shell's
timeout value.
In addition, there are problems with arrays. The following is
incorrectly allowed:
typeset -a arr=((a b c) 1)
readonly arr
arr[0][1]=d
arr=(alphas=(a b c);name=x)
readonly arr.alphas
arr.alphas[1]=([b]=5)
arr=(alphas=(a b c);name=x)
readonly arr.alphas
arr.alphas[1]=(b)
typeset -C arr=(typeset -r -a alphas=(a b c);name=x)
arr.alphas[1]=()
src/cmd/ksh93/bltins/typeset.c: setall():
- Relocate readonly attribute check higher up the code and widen
its application to issue an error message if the pre-existing
name-pair has the readonly bit flag set.
- To avoid compatibility problems, don't check for readonly if
NV_RDONLY is the only attribute set (ignoring NV_NOFREE). This
allows 'readonly foo; readonly foo' to keep working.
src/cmd/ksh93/sh/array.c: nv_endsubscript():
- Apply a readonly flag check when an array subscript or append
assignment occurs, but allow type variables (typeset -T) as they
utilize '-r' for 'required' sub-variables.
src/cmd/ksh93/tests/readonly.sh:
- New file. Create readonly tests that validate the warning message
and validate that the readonly variable did not change.
src/cmd/ksh93/sh/streval.c:
- Bump MAXLEVEL from 9 to 1024 as a workaround for arithmetic
expansion, avoiding a spurious error about too much recursion
when the readonly.sh tests are run. This change is backported
from ksh 93v-.
TODO: debug a spurious increase in arithmetic recursion level
variable when readonly.sh tests with 'typeset -i' are run.
That is a different bug for a different commit.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
|
||
|
Johnothan King
|
56913f8c2a
|
Fix bugs related to 'uname -d' in the 'uname' builtin (#251)
This commit fixes a bug in the ksh uname builtin's -d option that could
change the output of -o (I was only able to reproduce this on Linux):
$ builtin uname
$ uname -o
GNU/Linux
$ uname -d
(none)
$ uname -o
(none)
I identified this patch from ksh2020 as a fix for this bug:
<https://github.com/att/ast/pull/1187>
The linked patch was meant to fix a crash in 'uname -d', although I've
had no luck reproducing it: <https://github.com/att/ast/issues/1184>
src/lib/libcmd/uname.c:
- Pass correct buffer to getdomainname() while executing uname -d.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for the reported 'uname -d' crash.
- Add a regression test for the output of 'uname -o' after 'uname -d'.
- To handle potential crashes when running the regression tests in older
versions of ksh, fork the command substitutions that run 'uname -d'.
|
||
|
Johnothan King
|
ca2443b58c
|
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> |
||
|
Johnothan King
|
113a9392ff
|
Fix vi mode crashes when going back one word (#246)
This bug was originally reported at <https://github.com/att/ast/issues/1467>. A crash can occur when using the 'b' or 'B' vi mode commands to go back one word. I was able to reproduce these crashes with 100% consistency on an OpenBSD virtual machine when ksh is compiled with -D_std_malloc. Reproducer: $ set -o vi $ asdf <ESC> <b or B> The fix is based on Matthew DeVore's analysis: > I suspect this is caused by this line: >> while (vi_isalph(tcur_virt) && tcur_virt >= first_virt) --tcur_virt; > which is in the b codepath. It checks vi_isalph(tcur_virt) before checking > if tcur_virt is in range. These two clauses should be reversed. Note that > line 316 is a similar check for pressing B, and there the tcur_virt value > is checked first. src/cmd/ksh93/edit/vi.c: - Check tcur_virt before using isalph() or isblank() to fix both crashes. At the start of the backword() while loop this check was performed twice, so the redundant check has been removed. src/cmd/ksh93/tests/pty.sh: - Add a regression test for the b, B, w and W editor commands. |
||
|
Johnothan King
|
fc2d5a6019
|
test foo =~ foo should fail with exit status 2 (#245)
When test is passed the '=~' operator, it will silently fail with
exit status 1:
$ test foo =~ foo; echo $?
1
This bug is caused by test_binop reaching the 'NOTREACHED' area of
code. The bugfix was adapted from ksh2020:
https://github.com/att/ast/issues/1152
src/cmd/ksh93/bltins/test.c: test_binop():
- Error out with a message suggesting usage of '[[ ... ]]' if '=~'
is passed to the test builtin.
- Special-case TEST_END (']]') as that is not really an operator.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
|
||
|
Martijn Dekker
|
71934570bf |
Add --globcasedetect shell option for globbing and completion
One of the best-kept secrets of libast/ksh93 is that the code includes support for case-insensitive file name generation (a.k.a. pathname expansion, a.k.a. globbing) as well as case-insensitive file name completion on interactive shells, depending on whether the file system is case-insensitive or not. This is transparently determined for each directory, so a path pattern that spans multiple file systems can be part case-sensitive and part case- insensitive. In more precise terms, each slash-separated path name component pattern P is treated as ~(i:P) if its parent directory exists on a case-insensitive file system. I recently discovered this while dealing with <https://github.com/ksh93/ksh/issues/223>. However, that support is dead code on almost all current systems. It depends on pathconf(2) having a _PC_PATH_ATTRIBUTES selector. The 'c' attribute is supposedly returned if the given directory is on a case insensitive file system. There are other attributes as well (at least 'l', see src/lib/libcmd/rm.c). However, I have been unable to find any system, current or otherwise, that has _PC_PATH_ATTRIBUTES. Google and mailing list searches yield no relevant results at all. If anyone knows of such a system, please add a comment to this commit on GitHub, or email me. An exception is Cygwin/Windows, on which the "c" attribute was simply hardcoded, so globbing/completion is always case- insensitive. As of Windows 10, that is wrong, as it added the possibility to mount case-sensitive file systems. On the other hand, this was never activated on the Mac, even though macOS has always used a case-insensitive file like Windows. But, being UNIX, it can also mount case-sensitive file systems. Finally, Linux added the possibility to create individual case- insensitive ext4 directories fairly recently, in version 5.2. https://www.collabora.com/news-and-blog/blog/2020/08/27/using-the-linux-kernel-case-insensitive-feature-in-ext4/ So, since this functionality latently exists in the code base, and three popular OSs now have relevant file system support, we might as well make it usable on those systems. It's a nice idea, as it intuitively makes sense for globbing and completion behaviour to auto-adapt to file system case insensitivity on a per-directory basis. No other shell does this, so it's a nice selling point, too. However, the way it is coded, this is activated unconditionally on supported systems. That is not a good idea. It will surprise users. Since globbing is used with commands like 'rm', we do not want surprises. So this commit makes it conditional upon a new shell option called 'globcasedetect'. This option is only compiled into ksh on systems where we can actually detect FS case insensitivity. To implement this, libast needs some public API additions first. *** libast changes *** src/lib/libast/features/lib: - Add probes for the linux/fs.h and sys/ioctl.h headers. Linux needs these to use ioctl(2) in pathicase(3) (see below). src/lib/libast/path/pathicase.c, src/lib/libast/include/ast.h, src/lib/libast/man/path.3, src/lib/libast/Mamfile: - Add new pathicase(3) public API function. This uses whatever OS-specific method it can detect at compile time to determine if a particular path is on a case-insensitive file system. If no method is available, it only sets errno to ENOSYS and returns -1. Currently known to work on: macOS, Cygwin, Linux 5.2+, QNX 7.0+. - On systems (if any) that have the mysterious _PC_PATH_ATTRIBUTES selector for pathconf(2), call astconf(3) and check for the 'c' attribute to determine case insensitivity. This should preserve compatibility with any such system. src/lib/libast/port/astconf.c: - dynamic[]: As case-insensitive globbing is now optional on all systems, do not set the 'c' attribute by default on _WINIX (Cygwin/Windows) systems. - format(): On systems that do not have _PC_PATH_ATTRIBUTES, call pathicase(3) to determine the value for the "c" (case insensitive) attribute only. This is for compatibility as it is more efficient to call pathicase(3) directly. src/lib/libast/misc/glob.c, src/lib/libast/include/glob.h: - Add new GLOB_DCASE public API flag to glob(3). This is like GLOB_ICASE (case-insensitive matching) except it only makes the match case-insensitive if the file system for the current pathname component is determined to be case-insensitive. - gl_attr(): For efficiency, call pathicase(3) directly instead of via astconf(3). - glob_dir(): Only call gl_attr() to determine file system case insensitivity if the GLOB_DCASE flag was passed. This makes case insensitive globbing optional on all systems. - glob(): The options bitmask needs to be widened to fit the new GLOB_DCASE option. Define this centrally in a new GLOB_FLAGMASK macro so it is easy to change it along with GLOB_MAGIC (which uses the remaining bits for a sanity check bit pattern). src/lib/libast/path/pathexists.c: - For efficiency, call pathicase(3) directly instead of via astconf(3). *** ksh changes *** src/cmd/ksh93/features/options, src/cmd/ksh93/SHOPT.sh: - Add new SHOPT_GLOBCASEDET compile-time option. Set it to probe (empty) by default so that the shell option is compiled in on supported systems only, which is determined by new iffe feature test that checks if pathicase(3) returns an ENOSYS error. src/cmd/ksh93/data/options.c, src/cmd/ksh93/include/shell.h: - Add -o globcasedetect shell option if compiling with SHOPT_GLOBCASEDET. src/cmd/ksh93/sh/expand.c: path_expand(): - Pass the new GLOB_DCASE flag to glob(3) if the globcasedetect/SH_GLOBCASEDET shell option is set. src/cmd/ksh93/edit/completion.c: - While file listing/completion is based on globbing and automatically becomes case-insensitive when globbing does, it needs some additional handling to make a string comparison case-insensitive in corresponding cases. Otherwise, partial completions may be deleted from the command line upon pressing tab. This code was already in ksh 93u+ and just needs to be made conditional upon SHOPT_GLOBCASEDET and globcasedetect. - For efficiency, call pathicase(3) directly instead of via astconf(3). src/cmd/ksh93/sh.1: - Document the new globcasedetect shell option. |
||
|
Johnothan King
|
814b5c6890
|
Fix various minor problems and update the documentation (#237)
These are minor fixes I've accumulated over time. The following
changes are somewhat notable:
- Added a missing entry for 'typeset -s' to the man page.
- Add strftime(3) to the 'see also' section. This and the date(1)
addition are meant to add onto the documentation for 'printf %T'.
- Removed the man page the entry for ksh reading $PWD/.profile on
login. That feature was removed in commit
|
||
|
Martijn Dekker
|
33d0f004de |
File completion: fix incomplete multibyte support
Upon encountering two filenames with multibyte characters starting with the same byte, a partial multibyte character was completed. Reproducer (to run in UTF-8 locale): $ touch XXXá XXXë $ : XX <== pres tab $ : XXX^? <== partial multibyte character appears Note: á is $'\xc3\xa1' and ë is $'\xc3\xab' (same initial byte). src/cmd/ksh93/edit/completion.c: - Add multibyte support to the charcmp() and overlaid() functions. Thanks to Harald van Dijk for useful code and suggestions. - Add a few missing mbinit() calls. The state of multibyte processing must be reset before starting a new loop in case a previous processing run was interrupted mid-character. src/cmd/ksh93/tests/pty.sh: - Add test based on Harald's reproducer. Resolves: https://github.com/ksh93/ksh/issues/223 |
||
|
Martijn Dekker
|
936a1939a8
|
Allow proper tilde expansion overrides (#225)
Until now, when performing any tilde expansion like ~/foo or ~user/foo, ksh added a placeholder built-in command called '.sh.tilde', ostensibly with the intention to allow users to override it with a shell function or custom builtin. The multishell ksh93 repo <https://github.com/multishell/ksh93/> shows this was added sometime between 2002-06-28 and 2004-02-29. However, it has never worked and crashed the shell. This commit replaces that with something that works. Specific tilde expansions can now be overridden using .set or .get discipline functions associated with the .sh.tilde variable (see manual, Discipline Functions). For example, you can use either of: .sh.tilde.set() { case ${.sh.value} in '~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;; '~doc') .sh.value=~/Documents ;; '~ksh') .sh.value=/usr/local/src/ksh93/ksh ;; esac } .sh.tilde.get() { case ${.sh.tilde} in '~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;; '~doc') .sh.value=~/Documents ;; '~ksh') .sh.value=/usr/local/src/ksh93/ksh ;; esac } src/cmd/ksh93/include/variables.h, src/cmd/ksh93/data/variables.c: - Add SH_TILDENOD for a new ${.sh.tilde} predefined variable. It is initially unset. src/cmd/ksh93/sh/macro.c: - sh_btilde(): Removed. - tilde_expand2(): Rewritten. I started out with the tiny version of this function from the 2002-06-28 version of ksh. It uses the stack instead of sfio, which is more efficient. A bugfix for $HOME == '/' was retrofitted so that ~/foo does not become //foo instead of /foo. The rest is entirely new code. To implement the override functionality, it now checks if ${.sh.tilde} has any discipline function associated with it. If it does, it assigns the tilde expression to ${.sh.tilde} using nv_putval(), triggering the .set discipline, and then reads it back using nv_getval(), triggering the .get discipline. The resulting value is used if it is nonempty and does not still start with a tilde. src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/tests/builtins.sh: - Since ksh no longer adds a dummy '.sh.tilde' builtin, remove the ad-hoc hack that suppressed it from the output of 'builtin'. src/cmd/ksh93/tests/tilde.sh: - Add tests verifying everything I can think of, as well as tests for bugs found and fixed during this rewrite. src/cmd/ksh93/tests/pty.sh: - Add test verifying that the .sh.tilde.set() discipline does not modify the exit status value ($?) when performing tilde expansion as part of tab completion. src/cmd/ksh93/sh.1: - Instead of "tilde substitution", call the basic mechanism "tilde expansion", which is the term used everywhere else (including the 1995 Bolsky/Korn ksh book). - Document the new override feature. Resolves: https://github.com/ksh93/ksh/issues/217 |
||
|
Johnothan King
|
14352ba0a7
|
Save $? when discipline triggered without command (#226)
A discipline function could incorrectly influence the value of $?
(exit status of last command) outside its context if it was
triggered without another command being run, e.g. when a prompt
variable is read, or COLUMNS or LINES is set.
Reproducers include:
PS1 prompt:
$ PS1.get() { true; }
$ false
$ echo $?
0
PS2 prompt:
$ PS2.get() { return 13; }
$ \
>
$ echo $?
13
The set discipline is affected too, e.g. COLUMNS and LINES:
$ COLUMNS.set() { return 13; }
$ true
$ (press return)
$ echo $?
13
There are probably other contexts where the shell reads or changes
variables without running commands, allowing their get or set
disciplines to influence $?. So this commit makes ksh save $? for
all .get, .set, .append, and .unset discipline calls.
src/cmd/ksh93/sh/nvdisc.c:
- assign(): Save/restore $? when running a .set/.append/.unset
discipline function.
- lookup(): Save/restore $? when running a .get discipline.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for $? after displaying a prompt
and when setting a LINES.set discipline function.
src/cmd/ksh93/tests/return.sh:
- The above test fails in script form on ksh93u+ and ksh2020, as
it exposes another form of #117 that occurs after running a
subshell. Add the above regression test here as well
(re:
|
||
|
hyenias
|
4f9ce41aaa
|
typeset: Allow last numeric type given to be used (#221)
For most numeric types the last provided one wins out. This commit closes the gap for -F and -i numerics to not be covered up by other preceding float types. Note: -u for requesting an unsigned float or integer was considered and decided to be left alone as it stands, so as to not allow the variable to become an uppercased string if the requested options ended with a -u. As it stands for a case when multiple numeric types are requested, a -u option may be applied after the last numeric type is processed. Examples: -EF becomes -F -Fi becomes -i -Fu becomes -F -uF becomes -F -Fui becomes -i (because isfloat==1, unsigned is not applied) -Fiu becomes -iu (isfloat is reset and allows unsigned to be set) src/cmd/ksh93/bltins/typeset.c: b_typeset(): - Reset attribute bit flags for -E and -X when -F is requested by adding in NV_EXPNOTE to be removed. - For -i option if a float precedes it, reset isfloat and -E/-F attribute bit flags. - Take into account the impact of the shortint flag on floats. src/cmd/ksh93/tests/attributes.sh: - Add some validation tests to confirm that, when a -F follows either -E or -X, -F is used. - Add some validation tests to confirm that, when -F/E/X precede a -i, the variable becomes an integer and not a float. - Add in various tests when -s followed a float. |
||
|
Martijn Dekker
|
1df6a82a8a |
Make ~ expand to home directory after unsetting HOME
There was an issue with tilde expansion if the HOME var is unset. $ unset HOME $ echo ~ martijn Only the username is returned. Users are more likely to expect the current user's home directory as configured in the OS. POSIXly, the expansion of ~ is based on the value of HOME. If HOME is unset, the results are unspecified. After unsetting HOME, in bash, ~ returns the user's home directory as specified by the OS, whereas in all other shells, ~ expands to the empty string. Only ksh93 returns the username. The behaviour of bash is more useful. Discussion: https://github.com/ksh93/ksh/pull/225#issuecomment-799074107 src/cmd/ksh93/sh/macro.c, src/cmd/ksh93/tests/tilde.sh: - sh_tilde(): Backport fix by Mike Gilbert from ksh2020. See: https://github.com/att/ast/issues/1391 https://github.com/att/ast/pull/1396 https://github.com/att/ast/commit/070d365d - Add test. src/cmd/ksh93/COMPATIBILITY: - Note this change. |
||
|
Johnothan King
|
6d63b57dd3
|
Re-enable SHOPT_DEVFD, fixing process substitution fd leaks (#218)
This commit fixes a long-standing bug (present since at least ksh93r) that caused a file descriptor leak when passing a process substitution to a function, or (if compiled with SHOPT_SPAWN) to a nonexistent command. The leaks only occurred when ksh was compiled with SHOPT_DEVFD; the FIFO method was unaffected. src/cmd/ksh93/sh/xec.c: sh_exec(): - When a process substitution is passed to a built-in, the remaining file descriptor is closed with sh_iorestore. Do the same thing when passing a process substitution to a function. This is done by delaying the sh_iorestore() call to 'setexit:' where both built-ins and functions terminate and set the exit status ($?). This means that call now will not be executed if a longjmp is done, e.g. due to an error in a special built-in. However, there is already another sh_iorestore() call in main.c, exfile(), line 418, that handles that scenario. - sh_ntfork() can fail, so rather than assume it will succeed, handle a failure by closing extra file descriptors with sh_iorestore(). This fixes the leak on command not found with SHOPT_SPAWN. src/cmd/ksh93/include/defs.h: - Since the file descriptor leaks are now fixed, remove the workaround that forced ksh to use the FIFO method. src/cmd/ksh93/SHOPT.sh: - Add SHOPT_DEVFD as a configurable option (default: probe). src/cmd/ksh93/tests/io.sh: - Add a regression test for the 'not found' file descriptor leak. - Add a test to ensure it keeps working with 'command'. Fixes: https://github.com/ksh93/ksh/issues/67 |
||
|
Johnothan King
|
c3eac977ea
|
Fix unused process substitutions hanging (#214)
On systems where ksh needs to use the older and less secure FIFO method for process substitutions (which is currently all of them as the more modern and solid /dev/fd method is still broken, see #67), process substitutions could leave background processes hanging in these two scenarios: 1. If the parent process exits without opening a pipe to the child process forked by the process substitution. The fifo_check() function in xec.c, which is periodically called to check if the parent process still exists while waiting for it to open the FIFO, verified the parent process's existence by checking if the PPID had reverted to 1, the traditional PID of init. However, POSIX specifies that the PPID can revert to any implementation- defined system process in that case. So this breaks on certain systems, causing unused process substitutions to hang around forever as they never detect that the parent disappeared. The fix is to save the current PID before forking and having the child check if the PPID has changed from that saved PID. 2. If command invoked from the main shell is passed a process substitution, but terminates without opening the pipe to the process substitution. In that case, the parent process never disappears in the first place, because the parent process is the main shell. So the same infinite wait occurs in unused process substitutions, even after correcting problem 1. The fix is to remember all FIFOs created for any number of process substitutions passed to a single command, and unlink any remaining FIFOs as they represent unused command substitutions. Unlinking them FIFOs causes sh_open() in the child to fail with ENOENT on the next periodic check, which can easily be handled. Fixing these problems causes the FIFO method to act identically to the /dev/fd method, which is good for compatibility. Even when #67 is fixed this will still be important, as ksh also runs on systems that do not have /dev/fd (such as AIX, HP-UX, and QNX), so will fall back to using FIFOs. --- Fix problem 1 --- src/cmd/ksh93/sh/xec.c: - Add new static fifo_save_ppid variable. - sh_exec(): If a FIFO is defined, save the current PID in fifo_save_ppid for the forked child to use. - fifo_check(): Compare PPID against the saved value instead of 1. --- Fix problem 2 --- To keep things simple I'm abusing the name-value pair routines used for variables for this purpose. The overhead is negligible. A more elegant solution is possible but would involve adding more code. src/cmd/ksh93/include/defs.h: _SH_PRIVATE: - Define new sh.fifo_tree pointer to a new FIFO cleanup tree. src/cmd/ksh93/sh/args.c: sh_argprocsubs(): - After launching a process substitution in the background, add the FIFO to the cleanup list before freeing it. src/cmd/ksh93/sh/xec.c: - Add fifo_cleanup() that unlinks all FIFOs in the cleanup list and clears/closes the list. They should only still exist if the command never used them, however, just run 'unlink' and don't check for existence first as that would only add overhead. - sh_exec(): * Call fifo_cleanup() on finishing all simple commands (when setting $?) or when a special builtin fails. * When forking, clear/close the cleanup list; we do not want children doing duplicate cleanup, particularly as this can interfere when using multiple process substitutions in one command. * Process substitution handling: > Change FIFO check frequency from 500ms to 50ms. Note that each check sends a signal that interrupts open(2), causing sh_open() to reinvoke it. This causes sh_open() to fail with ENOENT on the next check when the FIFO no longer exists, so we do not need to add an additional check for existence to fifo_check(). Unused process substitutions now linger for a maximum of 50ms. > Do not issue an error message if errno == ENOENT. - sh_funct(): Process substitutions can be passed to functions as well, and we do not want commands within the function to clean up the FIFOs for the process substitutions passed to it from the outside. The problem is solved by simply saving fifo_tree in a local variable, setting it to null before running the function, and cleaning it up before restoring the parent one at the end. Since sh_funct() is called recursively for multiple-level function calls, this correctly gives each function a locally scoped fifo_tree. --- Tests --- src/cmd/ksh93/tests/io.sh: - Add tests covering the failing scenarios. Co-authored-by: Martijn Dekker <martijn@inlv.org> |
||
|
Martijn Dekker
|
d4adc8fcf9 |
Fix test -v for numeric types & set/unset state for short int
This commit fixes two interrelated problems.
1. The -v unary test/[/[[ operator is documented to test if a
variable is set. However, it always returns true for variable
names with a numeric attribute, even if the variable has not
been given a value. Reproducer:
$ ksh -o nounset -c 'typeset -i n; [[ -v n ]] && echo $n'
ksh: n: parameter not set
That is clearly wrong; 'echo $n' should never be reached and the
error should not occur, and does not occur on mksh or bash.
2. Fixing the previous problem revealed serious breakage in short
integer type variables that was being masked. After applying
that fix and then executing 'typeset -si var=0':
- The conditional assignment expansions ${var=123} and
${var:=123} assigned 123 to var, even though it was set to 0.
- The expansions ${var+s} and ${var:+n} incorrectly acted as if
the variable was unset and empty, respectively.
- '[[ -v var ]]' and 'test -v var' incorrectly returned false.
The problems were caused by a different storage method for short
ints. Their values were stored directly in the 'union Value'
member of the Namval_t struct, instead of allocated on the stack
and referred to by a pointer, as regular integers and all other
types do. This inherently broke nv_isnull() as this leaves no
way to distinguish between a zero value and no value at all.
(I'm also pretty sure it's undefined behaviour in C to check for
a null pointer at the address where a short int is stored.)
The fix is to store short ints like other variables and refer
to them by pointers. The NV_INT16P combined bit mask already
existed for this, but nv_putval() did not yet support it.
src/cmd/ksh93/bltins/test.c: test_unop():
- Fix problem 1. For -v, only check nv_isnull() and do not check
for the NV_INTEGER attribute (which, by the way, is also used
for float variables by combining it with other bits).
See also
|
||
|
Martijn Dekker
|
4a8072e826 |
Fix ${!foo@} and ${!foo*} to include 'foo' itself in search
These expansions are supposed to yield all variable names beginning with the indicated prefix. This should include the variable name that is identical to the prefix (as 'prefix' begins with 'prefix'). This bugfix is backported from the abandoned ksh 93v- beta, so AT&T intended this change. It also makes ksh work like bash in this. src/cmd/ksh93/sh/macro.c: varsub(): M_NAMESCAN: - Check if the prefix itself exists. If so, start with that. src/cmd/ksh93/tests/variables.sh: - Add tests for these expansions. src/cmd/ksh93/sh.1: - Fix the incomplete documentation of these expansions. src/cmd/ksh93/COMPATIBILITY: - Note the change as it's potentially incompatible in corner cases. Resolves: https://github.com/ksh93/ksh/issues/183 |
||
|
hyenias
|
5aba0c7251
|
Fix set/unset state for short integer (typeset -si) (#211)
This commit fixes at least three bugs:
1. When issuing 'typeset -p' for unset variables typeset as short
integer, a value of 0 was incorrectly diplayed.
2. ${x=y} and ${x:=y} were still broken for short integer types
(re:
|
||
|
Martijn Dekker
|
40860dac20 |
job_init(): fix init on setpgid() permission denied (re: 41ebb55a)
Symptoms of this bug below. These only seem to occur on Linux and
only if you replace your initial login shell by ksh using 'exec'.
1. An erroneous 'Interrupt' message is printed after stopping the
read builtin in a script. Reproducer:
$ exec arch/*/bin/ksh
$ cat ./reproducer.sh
#!/bin/sh
read foo
$ ./reproducer.sh
^C$ <Enter>
[1] + Interrupt ../reproducer.sh
2. Ctrl+C fails to stop /bin/package make. Reproducer:
$ exec arch/*/bin/ksh
$ mv arch arch.old
$ bin/package make
# Press Ctrl+C multiple times
Analysis: In
|
||
|
Martijn Dekker
|
aad74597f7 |
Fixes for -G/--globstar (re: 5312a59d)
The fix for '.' and '..' in regular globbing broke '.' and '..' in globstar. No globstar pattern that contains '.' or '..' as any pathname component still matched. This commit fixes that. This commit also makes symlink/** mostly work, which it never has done in any ksh93 version. It is correct and expected that symlinks found by patterns are not resolved, but symlinks were not resolved even when specified as explicit non-pattern pathname components. For example, /tmp/** breaks if /tmp is a symlink (e.g. on macOS), which looks like a bug. src/lib/libast/include/glob.h, src/lib/libast/misc/glob.c: glob_dir(): - Make symlink/** work. we can check if the string pointed to by pat is exactly equal to *. If so, we are doing regular globbing for that particular pathname element, and it's okay to resolve symlinks. If not (if it's **), we're doing globstar and we should not be matching symlinks. - Let's also introduce proper identification of symlinks (GLOB_SYM) and not lump them in with other special files (GLOB_DEV). - Fix the bug with literal '.' and '..' components in globstar patterns. In preceding code, the matchdir pointer gets set to the complete glob pattern if we're doing globstar for the current pathname element, null if not. The pat pointer gets set to the elements of the pattern that are still left to be processed; already-done elements are trimmed from it by increasing the pointer. So, to do the right thing, we need to make sure that '.' or '..' is skipped if, and only if, it is the final element in the pattern (i.e., if pat does not contain a slash) and is not specified literally as '.' or '..', i.e., only if '.' or '..' was actually resolved from a glob pattern. After this change, '**/.*', '**/../.*', etc. do the right thing, showing all your hidden files and directories without undesirable '.' and '..' results; '.' and '..' are skipped as final elements, unless you literally specify '**/.', '**/..', '**/foo/bar/..', etc. src/cmd/ksh93/COMPATIBILITY: - Note the symlink/** globstar change. src/cmd/ksh93/sh.1: - Try to document the current globstar behaviour more exhausively. src/cmd/ksh93/tests/glob.sh: - Add tests. Try to cover all the corner cases. src/cmd/ksh93/tests/shtests: - Since tests in glob.sh do not use err_exit, they were not counted. Special-case glob.sh for counting the tests: count the lines starting with a test_* function call. Resolves: https://github.com/ksh93/ksh/issues/146 |
||
|
Martijn Dekker
|
89c69b076d |
Fix command history corruption on syntax error (re: e999f6b1)
Analysis: When a syntax error occurs, the shell performs a
longjmp(3) back to exfile() in main.c on line 417:
415| if(jmpval)
416| {
417| Sfio_t *top;
418| sh_iorestore((void*)shp,0,jmpval);
419| hist_flush(shp->gd->hist_ptr);
420| sfsync(shp->outpool);
The first thing it does is restore the file descriptor state
(sh_iorestore), then it flushes the history file (hist_flush), then
it synchronises sfio's logical stream state with the physical
stream state using (sfsync).
However, the fix applied in
|
||
|
Johnothan King
|
c1986c4e1a
|
Fix Ctrl+D after ksh receives SIGWINCH (#208)
src/cmd/ksh93/edit/edit.c: ed_read(): - The loop that handles SIGWINCH assumes sfpkrd will return and set errno to EINTR if ksh is sent SIGWINCH. This only occurs when select(2) is used to wait for input, so tell sfpkrd to use select if possible. This is only done if the last argument given to sfpkrd is '2', which should avoid regressions. src/lib/libast/sfio/sfpkrd.c: sfpkrd(): - Always use select if the last argument is 2. This allows sfpkrd() to intercept SIGWINCH when necessary. Fixes: https://github.com/ksh93/ksh/issues/202 |
||
|
Martijn Dekker
|
9f2389ed93 |
Fix ${x=y} and ${x:=y} for numeric types of x
These POSIX expansions first assign y to x if x is unset or empty,
respectively, and then they yield the value of x. This was not
working on any ksh93 version if x was typeset as numeric (integer
or float) but still unset, as in not assigned a value.
$ unset a; typeset -i a; printf '%q\n' "${a:=42}" "$a"
0
''
Expected output:
42
42
src/cmd/ksh93/sh/macro.c:
- Fix the test for set/unset variable. It was broken because it
only checked for the existence of the node, which exists after
'typeset', but did not check if a value had been assigned. This
additional check needs to be done with the nv_isnull() macro, but
only for expansions of the regular M_BRACE type. Special
expansions cannot have an unset state.
- As of commit
|
||
|
Martijn Dekker
|
f8f2c4b608 |
Remove obsolete quote balancing hack
The old Bourne shell failed to check for closing quotes and command substitution backticks when encountering end-of-file in a parser context (such as a script). ksh93 implemented a hack for partial compatibility with this bug, tolerating unbalanced quotes and backticks in backtick command subsitutions, 'eval', and command line invocation '-c' scripts only. This hack became broken for backtick command substitutions in fe20311f/350b52ea as a memory leak was fixed by adding a newline to the stack at the end of the command substitution. That extra newline becomes part of any string whose quotes are not properly terminated, causing problems such as the one detailed here: https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01889.html $ touch abc $ echo `ls "abc` ls: abc : not found No other fix for the memory leak is known that doesn't cause other problems. (The alternative fix detailed in the referenced mailing list post causes a different corner-case regression.) Besides, the hack has always caused other corner case bugs as well: $ ksh -c '((i++' Actual: ksh: i++(: not found (If an external command 'i++(' existed, it would be run) Expect: ksh: syntax error at line 1: `(' unmatched $ ksh -c 'i=0; echo $((++i' Actual: (empty line; the arithmetic expansion is ignored) Expect: ksh: syntax error at line 1: `(' unmatched $ ksh -c 'echo $(echo "hi)' Actual: ksh: syntax error at line 1: `(' unmatched Expect: ksh: syntax error at line 1: `"' unmatched So, it's time to get rid of this hack. The old Bourne shell is dead and buried. No other shell tries to support this breakage. Tolerating syntax errors is just asking for strange side effects, inconsistent states, and corner case bugs. We should not want to do that. Old scripts that rely on this will just need to be fixed. src/cmd/ksh93/sh/lex.c: - struct lexdata: Remove 'char balance' member for remembering an unbalanced quote or backtick. - sh_lex(): Remove the back to remember and compensate for unbalanced quotes/backticks that was executed only if we were executing a script from a string, as opposed to a file. src/cmd/ksh93/COMPATIBILITY: - Note the change. Resolves: https://github.com/ksh93/ksh/issues/199 |
||
|
Martijn Dekker
|
b48e5b3365 |
Fix arbitrary command execution vuln in array subscripts in arith
This commit fixes an arbitrary command execution vulnerability in array subscripts used within the arithmetic subsystem. One of the possible reproducers is: var='1$(echo INJECTION >&2)' ksh -c \ 'typeset -A a; ((a[$var]++)); typeset -p a' Output before this commit: INJECTION typeset -A a=([1]=1) The 'echo' command has been surreptitiously executed from an external environment variable. Output after this commit: typeset -A a=(['1$(echo INJECTION >&2)']=1) The value is correctly used as an array subscript and nothing in it is parsed or executed. This is as it should be, as ksh93 supports arbitrary subscripts for associative arrays. If we think about it logically, the C-style arithmetic subsystem simply has no business messing around with shell expansions or quoting at all, because those don't belong to it. Shell expansions and quotes are properly resolved by the main shell language before the arithmetic subsystem is even invoked. It is particularly important to maintain that separation because the shell expansion mechanism also executes command substitutions. Yet, the arithmetic subsystem subjected array subscripts that contain `$` (and only array subscripts -- how oddly specific) to an additional level of expansion and quote resolution. For some unfathomable reason, there are two lines of code doing specifically this. The vulnerability is fixed by simply removing those. Incredibly, variants of this vulnerability are shared by bash, mksh and zsh. Instead of fixing it, it got listed in Bash Pitfalls! http://mywiki.wooledge.org/BashPitfalls#y.3D.24.28.28_array.5B.24x.5D_.29.29 src/cmd/ksh93/sh/arith.c: - scope(): Remove these two lines that implement the vulnerability. if(strchr(sub,'$')) sub = sh_mactrim(shp,sub,0); - scope(), arith(): Remove the NV_SUBQUOTE flag from two nv_endsubscript() calls. That flag causes the array subscript to retain the current level of shell quoting. The shell quotes everything as in "double quotes" before invoking the arithmetic subsystem, and the bad sh_mactrim() call removed one level of quoting. Since we're no longer doing that, this flag should no longer be passed, or subscripts may get extra backslash escapes. src/cmd/ksh93/include/name.h, src/cmd/ksh93/sh/array.c: - nv_endsubscript(): The NV_SUBQUOTE flag was only passed from arith.c. Since it is now unused, remove it. src/cmd/ksh93/tests/arith.sh: - Tweak some tests: fix typos, report wrong values. - Add 21 tests. Most are based on reproducers contributed by @stephane-chazelas and @hyenias. They verify that this vulnerability is gone and that no quoting bugs were introduced. Resolves: https://github.com/ksh93/ksh/issues/152 |
||
|
hyenias
|
a61430f1b5
|
Readonly attribute size fix (#201)
Corrected the size of attribute(s) being overwritten with 0 when 'readonly' or 'typeset -r' was applied to an existing variable. Since one cannot set any attributes with the 'readonly' command, its function call to setall() needs to be adjusted to acquire the current size from the old size or existing size of the variable. A plain 'typeset -r' is the same as 'readonly' in that it needs to load the old size as its current size for use in the subsequent to call to nv_newattr(). src/cmd/ksh93/bltins/typeset.c: setall(): - Both 'readonly' and 'typeset -r' end up calling setall(). setall() has full visibility into all user supplied values and existing values that are needed to differentiate whereas name.c newattr() acquires combined state flags. - Added a conditional check if the readonly flag was requested by user then meets the criteria of having present size of 0, cannot be a numeric nor binary string, and is void of presence of any of the justified string attributes. - -L/R/Z justified string attributes if not given a value default to a size of 0 which means to autosize. A binary string can have a fixed field size, e.g. -bZ. The present of any of the -L/R/Z attribules means that current size is valid and should be used even if it is zero. src/cmd/ksh93/tests/attributes.sh: - Added various tests to capture and reiterate that 'readonly' should be equivalent to 'typeset -r' and applying them should not alter the previous existing size unless additional attributes are set along with typeset command. |
||
|
Martijn Dekker
|
c928046aa9 |
Fix ${.sh.fun} leaking out of DEBUG trap
The value of the ${.sh.fun} variable, which is supposed to contain
the name of the function currently being executed, leaks out of the
DEBUG trap if it executes a function. Reproducer:
$ fn() { echo "executing the function"; }
$ trap fn DEBUG
$ trap - DEBUG
executing the function
$ echo ${.sh.fun}
fn
${.sh.fun} should be empty outside the function.
Annalysis:
The sh_debug() function in xec.c, which executes the DEBUG trap
action, contains these lines, which are part of restoring the state
after running the trap action with sh_trap():
nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE);
nv_putval(SH_FUNNAMENOD,shp->st.funname,NV_NOFREE);
shp->st = savst;
First the SH_PATHNAMENOD (${.sh.file}) and SH_FUNNAMENOD
(${.sh.fun}) variables get restored from the values in the shell's
scoped information struct (shp->st), but that is done *before*
restoring the parent scope with 'shp->st = savst;'. It should be
done after. Fixing the order is sufficient to fix the bug.
However, I am not convinced that these nv_putval() calls are good
for anything at all. Setting, unsetting, restoring, etc. the
${.sh.fun} and ${.sh.file} variables is already being handled
perfectly well elsewhere in the code for executing functions and
sourcing dot scripts. The DEBUG trap is neither here nor there.
There's no reason for it to get involved with these variables.
I was unable to break anything after simply removing those two
lines. So I strongly suspect this is another case, out of many now,
where a bug in ksh93 is properly fixed by removing some code.
I couldn't get ${.sh.file} to leak similarly -- I think this is
because SH_PATHNAMENOD (and not SH_FUNNOD) is set explicitly in
exfile() in main.c, masking this incorrect restore. It is the only
place where SH_PATHNAMENOD and SH_FUNNOD are not both set.
src/cmd/ksh93/sh/xec.c:
- Remove these two spurious nv_putval() calls.
src/cmd/ksh93/tests/variables.sh:
- Add regression test for leaking ${.sh.fun}.
|
||
|
Martijn Dekker
|
d9865ceae1 |
emacs: Fix three tab completion bugs
1. The editor accepted literal tabs without escaping in certain cases, causing buggy and inconsistent completion behaviour. https://github.com/ksh93/ksh/issues/71#issuecomment-656970959 https://github.com/ksh93/ksh/issues/71#issuecomment-657216472 2. After completing a filename by choosing from a file completion menu, the terminal cursor was placed one position too far to the right, corrupting command line display. This happened with multiline active. https://github.com/ksh93/ksh/issues/71#issue-655093805 3. A completion menu was displayed if the file name to be completed was at the point where the rest of it started with a number, even if that part uniquely identified it so the menu had 1 item. https://www.mail-archive.com/ast-users@lists.research.att.com/msg00436.html src/cmd/ksh93/edit/emacs.c: - Cosmetic consistency: change two instances of cntl('[') to ESC. - ed_emacsread(): Fix number 1 by refusing to continue into default processing if a tab character was not used for tab completion. Instead, beep and continue to the next read loop iteration. This behaviour is consistent with most other shells, so I doubt there will be objections. To enter a literal tab it's simple enough to escape it with ^V (the 'stty lnext' character) or \. - draw(): Fix number 2 by correcting an off-by-one error in the ed_setcursor() call that updates the terminal's cursor display in multiline mode. The 'old' and 'new' parameters need to have identical values in this particular call to avoid the cursor position being off by one to the right. This change makes it match the corresponding ed_setcursor() call in vi.c. See below* for details. Thanks to Lev Kujawski for the help in analysing. src/cmd/ksh93/edit/completion.c: ed_expand(): - Fix number 3 by changing from '=' mode (menu-based completion) to '\' mode (ordinary filename completion) if the menu would only show one option, which was pointless and annoying. This never happened in vi mode, so possibly the ed_expand() call in emacs.c could have been improved instead. But I'm comfortable with fixing it here and not in emacs.c, because this fixes it at a more fundamental level, plus it's straightforward and obvious here. Resolves: https://github.com/ksh93/ksh/issues/71 ____ * Further details on bug number 2: At https://github.com/ksh93/ksh/issues/71#issuecomment-786391565 Martijn Dekker wrote: > I'm back to my original hypothesis that there is somehow an > off-by-one error related to the ed_setcursor() call that gets > executed when in multiline mode. I cannot confirm whether that > off-by-one error is actually in the call itself, or occurs > sometime earlier on one of the many possible occasions where > ep->cursor is changed. But everything else appears to work > correctly, so it's not unlikely that the problem is in the call > itself. > > For reference, this is the original version of that call in > emacs.c: > > ksh/src/cmd/ksh93/edit/emacs.c > Lines 1556 to 1557 in |