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
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 e999f6b1 caused sh_iorestore() to sync
all sfio streams unconditionally. So this was done before
hist_flush(), which caused unpredictable behaviour, including
temporary and/or permanent history corruption, as this also synched
shp->outpool before hist_flush() had a chance to do its thing.
The fix is to only call sfsync() in sh_iorestore() if we're
actually about to call ftruncate(2), and not otherwise.
Moral of the story: bug fixes should be as specific as possible to
minimise the risk of side effects.
src/cmd/ksh93/sh/io.c: sh_iorestore():
- Only call sfsync() if we're about to truncate a file.
src/cmd/ksh93/tests/pty.sh:
- Add test.
Thanks to Marc Wilson for reporting the bug and to Johnothan King
for finding the commit that introduced it.
Resolves: https://github.com/ksh93/ksh/issues/209
Relevant: https://github.com/att/ast/issues/61
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 95294419, we know that an nv_optimize() call may be
needed before using nv_isnull() if the shell is compiled with
SHOPT_OPTIMIZE. Move the nv_optimize() call from that commit
forward to before the new check that calls nv_isnull(), and only
bother with it if the type is M_BRACE.
src/cmd/ksh93/tests/variables.sh:
- Add tests for this bug. Test float and integer, and also check
that ${a=b} and ${a:=b} correctly treat the value of 'b' as an
arithmetic expression of which the result is assigned to 'a' if
'a' was typeset as numeric.
src/cmd/ksh93/tests/attributes.sh,
src/cmd/ksh93/tests/comvar.sh,
src/cmd/ksh93/tests/nameref.sh,
src/cmd/ksh93/tests/types.sh:
- Fix a number of tests to report failures correctly.
Resolves: https://github.com/ksh93/ksh/issues/157
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
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.
src/cmd/ksh93/tests/basic.sh:
- Fix syntax error (unbalanced single quote) in two -c script
invocations. It only failed to throw a syntax error due to a
problematic hack in ksh that may be removed soon.
See: https://github.com/ksh93/ksh/issues/199
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/io.sh:
- Redirect standard error on two ksh -i invocations to /dev/null
to work around the test hanging on AIX.
src/cmd/ksh93/tests/comvario.sh:
- Remove duplicate copyright header.
- Fix warning format.
src/cmd/ksh93/tests/functions.sh:
- Fix the 'TERM signal sent to last process of function kills the
script' test so that it works on AIX. We cannot rely on grepping
'ps' output as the external 'sleep' command does not show the
command name on AIX. Instead, find it by its parent PID.
src/cmd/ksh93/tests/locale.sh,
src/cmd/ksh93/tests/substring.sh:
- Rewrite the very broken multibyte locale tests (two outright
syntax errors due to unbalanced quotes, and none of the tests
actually worked).
- Since they set LC_ALL, move them to locale.sh.
src/cmd/ksh93/tests/variables.sh:
- Redirect stderr on some 'ulimit -t unlimited' invocations (which
fork subshells as the intended side effect) to /dev/null in case
that exceeds a system-defined limit.
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}.
This bug was backported along with a fix from 93v-. An inconsistent
state occurred if you caused a file name completion menu to appear
with two TABs (which also puts you in command mode) but then
re-enter insert mode (e.g. with 'a') instead of entering a number.
$ set -o vi
$ cd /
$ bin/p [press TAB twice]
1) pax
2) ps
3) pwd [now type 'a', 'wd', return]
$ bin/pwd
> [PS2 prompt wrongly appears; press return]
/
$
Here's another reproducer, suggesting the problem is a write past
the end of the screen buffer:
$ set -o vi
$ cd /
$ bin/p [press TAB twice]
1) pax
2) ps
3) pwd [press '0', then '$']
$ bin/p [cursor is one too far to the right, past the 'p'!]
[Further operations show random evidence of memory corruption]
Harald van Dijk found the cause (thanks!):
> In vi.c's textmod there is
>
> case '=': /** list file name expansions **/
> ...
> ++last_virt;
> ...
> if(ed_expand(vp->ed,(char*)virtual, &cur_virt, &last_virt, ch, vp->repeat_set?vp->repeat:-1)<0)
> {
> ...
> last_virt = i;
> ...
> }
> else if((c=='=' || (c=='\\'&&virtual[last_virt]=='/')) && !vp->repeat_set)
> {
> ...
> }
> else
> {
> ...
> --last_virt;
> ...
> }
> break;
>
> That middle block does not restore last_virt, and everything goes
> wrong after that. That function used to restore last_virt until
> commit 4cecde1 (#41). The commit message says it was taken from
> ksh93v- and indeed this bug is also present in that version too.
> If I restore the last_virt = i; that was there originally, like
> below, then this bug seems to be fixed. I do not know why it was
> taken out, taking it out does not seem to be necessary to fix the
> original bug.
src/cmd/ksh93/edit/vi.c: textmod():
- Restore the missing restore of last_virt.
src/cmd/ksh93/tests/pty.sh:
- Add test that checks basic completion menu functionality works
and runs modified versions of the two reproducers above.
Resolves: https://github.com/ksh93/ksh/issues/195
Reproducer:
$ ksh -c 'v=${ PATH=/dev/null; }; echo $PATH; whence ls'
/dev/null
/bin/ls
The PATH=/dev/null assignment should survive the shared-state
command substitution, and does, yet 'ls' is still found.
The variable became inconsistent with the internal pathlist.
This bugfix is from the 93v- beta.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Do not save and restore pathlist for a subshare.
- A few other subshell tweaks from 93v- that made sense:
. reset shp->subdup (bitmask for dups of 1) after saving it
. use e_dot instead of "." for consistency
. retry close(1) if it was interrupted
src/cmd/ksh93/tests/path.sh:
- Add test for this bug.
On some systems (AIX, HP-UX, OpenBSD) the pty tests may hang.
On all systems except Darwin/macOS, FreeBSD and Linux, the pty
tests show one or more regressions. But when I try out the failing
tests manually in a real session, it seems to work fine. So I
suspect pty is broken and not ksh.
src/cmd/ksh93/tests/pty.sh:
- For now, only run the pty tests on Darwin, FreeBSD and Linux.
src/lib/libast/Mamfile:
- tvsleep.c: Add missing error.h dependency (re: 2f7918de).
(unrelated, but just wasn't worth its own commit)
ksh crashed in various different and operating system-dependent
ways when attempting to create or apply justification strings
using typeset -L/-R/-Z, especially if large sizes are used.
The crashes had two immediate causes:
- In nv_newattr(), when applying justification attributes, a buffer
was allocated for the justified string that was exactly 8 bytes
longer than the original string. Any larger justification string
caused a buffer overflow (!!!).
- In nv_putval(), when applying existing attributes to a new value,
the corresponding memmove() either did not zero-terminate the
justified string (if the original string was longer than the
justified string) or could read memory past the original string
(if the original string was shorter than the justified string).
Both scenarios can cause a crash.
This commit fixes other minor issues as well, such as a mysterious
8 extra bytes allocated by several malloc/realloc calls. This may
have been some naive attempt to paper over the above bugs. It seems
no one can make any other kind of sense of it.
A readjustment bug with zero-filling was also fixed.
src/cmd/ksh93/sh/name.c:
- nv_putval():
. Get rid of the magical +8 bytes for malloc and realloc. Just
allocate one extra byte for the terminating zero.
. Fix the memmove operation to use strncpy instead, so that
buffer overflows are avoided in both scenarios described above.
Also make it conditional upon a size adjustment actually
happening (i.e. if 'dot' is nonzero).
. Mild refactoring: combine two 'if(sp)' blocks into one;
declare variables only used there locally for legibility.
- nv_newattr():
* Replace the fatally broken "let's allocate string length + 8
bytes no matter the size of the adjustment" routine with a new
one based on work by @hyenias (see comments in #142). It is
efficient with memory, taking into account numeric types,
growing strings, and shrinking strings.
* Fix zero-filling in readjustment after changing the initial
size of a -Z attribute. If the number was zero, all zeros were
still skipped, leaving an empty string.
Thanks to @hyenias for originally identifying this breakage and
laying the groundwork for fixing nv_newattr(), and to @lijog for
the crash analysis that revealed the key to the nv_putval() fix.
Resolves: https://github.com/ksh93/ksh/issues/142
Resolves: https://github.com/ksh93/ksh/issues/181
So now we know what that faulty check for shp->indebug in sh_trap()
was meant to do: it was meant to pass down the trap handler's exit
status, via sh_debug(), down to sh_exec() (xec.c) so that it could
then skip the execution of the next command if the trap's exit
status is 2, as documented in the manual page. As of d00b4b39, exit
status 2 was not passed down, so this stopped working.
This commit reinstates that functionality, but without the exit
status bug in command substitutions caused by the old way.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Save the trap's exit status before restoring the parent
envionment's exit status. Make this saved exit status the return
value of the function. (This does not break anything, AFAICT; the
majority of sh_trap() calls ignore the return value, and the few
that don't ignore it seem to expect it to return exactly this.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- The sh_trap() fix has one side effect: whereas the exit status of
a skipped command was always 2 (as per the trap handler), now it
is always 0, because it gets reset in sh_exec() but no command is
executed. That is probably not a desirable change in behaviour,
so let's fix that here instead: set sh.exitval to 2 when skipping
commands.
src/cmd/ksh93/sh.1:
- Document that ${.sh.command} shell-quotes its arguments for use
by 'eval' and such. This fact was not documented anywhere, AFAIK.
src/cmd/ksh93/shell.3:
- Document that $? (exit status) is made local to trap handlers.
- Document that sh_trap() returns the trap handler's exit status.
src/cmd/ksh93/tests/basic.sh:
- Add test for this bug.
- Add a missing test for the exit status 255 functionality (if a
DEBUG trap handler yields this exit status and we're executing a
function or dot script, a return is triggered).
Fixes: https://github.com/ksh93/ksh/issues/187
Many of the errors fixed in this commit are word repetitions
such as 'the the' and minor spelling errors. One formatting
error in the ksh man page has also been fixed.
In the 93v- beta, they add a newline instead of a space.
This has fewer side effects as final newlines get stripped.
It's still a hack and it would still be nice to have a real fix,
but it seems even the AT&T guys couldn't come up with one.
src/cmd/ksh93/sh/macro.c:
- To somehow avoid a memory leak involving alias substitution,
append a linefeed instead of a space to the comsub buffer.
src/cmd/ksh93/tests/subshell.sh:
- Add test for minor regression caused by the RedHat version.
This commit fixes a bug in the 'read' built-in: it did not properly
skip over multibyte characters. The bug never affects UTF-8 locales
because all UTF-8 bytes have the high-order bit set. But Shift-JIS
characters may include a byte corresponding to the ASCII backslash
character, which cauased buggy behaviour when using 'read' without
the '-r' option that disables backslash escape processing.
It also makes the regression tests compatible with Shift-JIS
locales. They failed with syntax errors.
src/cmd/ksh93/bltins/read.c:
- Use the multibyte macros when skipping over word characters.
Based on a patch from the old ast-developers mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01848.html
src/cmd/ksh93/include/defs.h:
- Be a bit smarter about causing the compiler to optimise out
multibyte code when SHOPT_MULTIBYTE is disabled. See the updated
comment for details.
src/cmd/ksh93/tests/locale.sh:
- Put all the supported locales in an array for future tests.
- Add test for the 'read' bug. Include it in a loop that tests
64 SHIFT-JIS character combinations. Only one fails on old ksh:
the one where the final byte corresponds to the ASCII backslash.
It doesn't hurt to test all the others anyway.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/quoting2.sh:
- Fix syntax errors that occurred in SHIFT-JIS locales as the
parser was processing literal UTF-8 characters. Not executing
that code is not enough; we need to make sure it never gets
parsed as well. This is done by wrapping the commands containing
literal UTF-8 strings in an 'eval' command as a single-quoted
operand.
.github/workflows/ci.yml:
- Run the tests in the ja_JP.SJIS locale instead of ja_JP.UTF-8.
UTF-8 is already covered by the nl_NL.UTF-8 test run; that should
be good enough.
This commit introduced the following bug, which is worse than the
one that commit fixed: it became impossible to alter the size of an
existing justified string attribute.
Thanks to @hyenias for catching this bug:
https://github.com/ksh93/ksh/issues/142#issuecomment-780931533
$ unset s; typeset -L 100 s=h; typeset +p s; typeset -L 5 s; typeset +p s
typeset -L 100 s
typeset -L 100 s
Expected output:
typeset -L 100 s
typeset -L 5 s
src/cmd/ksh93/sh/name.c:
- Revert.
src/cmd/ksh93/tests/attributes.sh:
- Revert: re-disable tests for minor attribute output regressions.
- Add a test for this bug and potential similar bugs.
A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:
$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
BAD
If the variable pre-exists in the subshell, the bug does not occur:
$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
(empty line, as expected)
The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.
To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.
src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
misuse of sh.shpwd described above.
- sh_assignok():
. Check subshell_noscope instead of shpwd to see if scope
creation is disabled. This makes it possible to distinguish
between restoring scope and handling subshares.
. If the current environment is a subshare that is in a virtual
subshell, create a scope in the parent subshell. This is done
by temporarily making the parent virtual subshell the current
subshell (by setting the global subshell_data pointer to it)
and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
set subshell_noscope instead of saving and unsetting sh.shpwd.
src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.
Fixes: https://github.com/ksh93/ksh/issues/143
This commit fixes the following:
1. Emacs mode ignores --nobackslashctrl (re: 24598fed) when in
reverse search.
2. When entering more than one backslash, emacs reverse search mode
deletes multiple backslashes after pressing backspace once.
Reproducer:
$ set --emacs --nobackslashctrl
$ <Ctrl+R> \\\\<Backspace>
3. Except when in reverse search, the backslash fails to escape a
subsequent interrupt character (^C). Reproducer:
$ set --emacs --backslashctrl
$ teststring \<Ctrl+C>
src/cmd/ksh93/edit/emacs.c:
- Disable escaping backslashes in emacs reverse search if
'nobackslashctrl' is enabled.
- Fix the buggy behavior of backslashes in emacs reverse
search by processing backslashes in a loop.
src/cmd/ksh93/tests/pty.sh:
- Add regression tests.
src/cmd/ksh93/sh.1:
- Fix a minor documentation error (^C is the usual interrupt
character, not ^?).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
'case x in esac' should be syntactically correct, but was an error:
$ ksh -c 'case x in esac'
ksh: syntax error at line 1: `case' unmatched
Inserting a newline was a workaround:
$ ksh -c $'case x in\nesac'
(no output)
The problem was that the 'esac' reserved word was not being
recognised if it immediately followed the 'in' reserved word.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Do not turn off recognition of reserved words after 'in' if we're
in a 'case' construct; only do this for 'for' and 'select'.
src/cmd/ksh93/tests/case.sh:
- Add seven regression test for correct recognition of 'esac'.
Only two failed on ksh93. The rest is to catch future bugs.
Fixes: https://github.com/ksh93/ksh/issues/177
This fixes the following:
1. 'set --posix' now works as an equivalent of 'set -o posix'.
2. The posix option turns off braceexpand and turns on letoctal.
Any attempt to override that in a single command such as 'set -o
posix +o letoctal' was quietly ignored. This now works as long
as the overriding option follows the posix option in the command.
3. The --default option to 'set' now stops the 'posix' option, if
set or unset in the same 'set' command, from changing other
options. This allows the command output by 'set +o' to correctly
restore the current options.
src/cmd/ksh93/data/builtins.c:
- To make 'set --posix' work, we must explicitly list it in
sh_set[] as a supported option so that AST optget(3) recognises
it and won't override it with its own default --posix option,
which converts the optget(3) string to at POSIX getopt(3) string.
This means it will appear as a separate entry in --man output,
whether we want it to or not. So we might as well use it as an
example to document how --optionname == -o optionname, replacing
the original documentation that was part of the '-o' description.
src/cmd/ksh93/sh/args.c: sh_argopts():
- Add handling for explitit --posix option in data/builtins.c.
- Move SH_POSIX syncing SH_BRACEEXPAND and SH_LETOCTAL from
sh_applyopts() into the option parsing loop here. This fixes
the bug that letoctal was ignored in 'set -o posix +o letoctal'.
- Remember if --default was used in a flag, and do not sync options
with SH_POSIX if the flag is set. This makes 'set +o' work.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/sh/args.c: sh_printopts():
- Do not potentially translate the 'on' and 'off' labels in 'set
-o' output. No other shell does, and some scripts parse these.
src/cmd/ksh93/sh/init.c: sh_init():
- Turn on SH_LETOCTAL early along with SH_POSIX if the shell was
invoked as sh; this makes 'sh -o' and 'sh +o' show expected
options (not that anyone does this, but correctness is good).
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- The state flags were in defs.h and most (but not all) of the
shell options were in shell.h. Gather all the shell state and
option flag definitions into one place in shell.h for clarity.
- Remove unused SH_NOPROFILE and SH_XARGS option flags.
src/cmd/ksh93/tests/options.sh:
- Add tests for these bugs.
src/lib/libast/misc/optget.c: styles[]:
- Edit default optget(3) option self-documentation for clarity.
Several changed files:
- Some SHOPT_PFSH fixes to avoid compiling dead code.
With this patch, the Korn shell can now guarantee that calls to
sleep on systems using the select or poll method always result in
the system clock advancing by that much time, assuming no
interruptions. This compensates for deficiencies in certain
systems, including SCO UnixWare.
Discussion: https://github.com/ksh93/ksh/pull/174
src/lib/libast/tm/tvsleep.c:
- Ensure that at least the time requested to sleep has elapsed
for the select and poll methods.
- Simplify the logic of calculating the time remaining to
sleep and handle the case of an argument of greater than
10e9 nanoseconds being passed to tvsleep.
src/cmd/ksh93/bltins/sleep.c:
- Eliminate the check for EINTR to handle other cases wherein
we have not slept enough.
src/cmd/ksh93/tests/variables.sh:
- Improve the diagnostic message when the sleep test fails.
- Revise the SECONDS function test to expect that we always
sleep for at least the time specified.
src/cmd/ksh93/tests/functions.h:
- Redirect ps stderr to /dev/null. UnixWare ps prints an error
message about not being able to find the controlling terminal
when shtests output is piped, but we are only using ps to find
the PID.
That OpenSUSE patch introduced a bug: file descriptors other than 1
that were globally redirected using 'exec' or 'redirect' no longer
survived a ${ shared-state; } command substitution.
Related: https://github.com/ksh93/ksh/issues/128
src/cmd/ksh93/sh/io.c:
- Add check for shp->subshare to the OpenSUSE patch.
src/cmd/ksh93/tests/io.sh:
- Add test.
.github/workflows/ci.yml:
- Go back to wrapping the regression tests in script(1).
src/cmd/ksh93/data/builtins.c:
- Never mind about the stty builtin.
src/cmd/ksh93/tests/pty.sh:
- Refuse to run if there isn't a functioning tty.
- Make sure stty(1) works on /dev/tty by redirecting stdin.
So, the pty regression tests on the Linux GitHub runner all failed.
Let's test an assumption: the reason is that we need the stty
builtin to properly set the pty state, because the OS-provided stty
command does not work if there is no real tty.
src/cmd/ksh93/data/builtins.c:
- Compile in the stty built-in. This adds about 20k to the binary
for a command that most users rarely need and even more rarely
need to be built in, so only compile it in on non-release builds.
src/cmd/ksh93/tests/pty.sh:
- Skip the tests if we cannot either use the stty builtin or change
the state of the real terminal to be compatible with the tests.
The Mac runner is still broken: intermittent pipe- and
signal-related regressions that do not occur on any real Mac.
1892358749
.github/workflows/ci.yml:
- Remove the macOS runner.
src/cmd/ksh93/tests/pty.sh:
- Do not skip pty tests if there is no tty. (On FreeBSD with no
tty, the tty builtin would need to be enabled in builtins.c.)
src/cmd/ksh93/tests/bracket.sh:
- Don't be noisy when skipping unavailable locales.
It is desirable to be able to run the tests on a system without
a functioning tty. Since this distribution comes with its own
pseudo-tty facility, pty, it should be possible to run the few
tests that require a tty on the pseudo-tty instead. I've verified
that they fail as expected on older ksh93.
Discussion: https://github.com/ksh93/ksh/pull/171
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/bracket.sh:
- Remove tests that require a tty.
src/cmd/ksh93/tests/pty.sh:
- Put them here, adapted to work as interactive pty scripts.
src/cmd/ksh93/tests/shtests:
- No longer refuse to run if there is no functioning tty.
.github/workflows/ci.yml:
- Since the tests no longer require a tty, no longer use script(1)
to get a pseudo-tty. Let's see if this works...
- Re-enable the Mac runner (re: 14632361). Maybe it has improved.
src/cmd/ksh93/tests/leaks.sh: Read vsz from UnixWare's ps
UnixWare's ps reports an accurate virtual size, so collecting that is
preferable to trying to parse the real resident size.
The GitHub runners apparently provide a non-working /dev/tty. To
avoid failures and confusion, shtests shold refuse to run the tests
and tell people to use script(1) to simulate a tty. On Linux, it
goes like this:
script -q -e -c 'bin/shtests --your-options-here'
On macOS and FreeBSD, the invocation is:
script -q /dev/null bin/shtests --your-options-here
The NetBSD and OpenBSD variants of script(1) need different
invocations again. They also don't pass down the command's exit
status, so would need a workaround for that.
It would be nice if we could use pty for this as this comes with
the distribution, so would work the same on every OS, but it seems
to be broken for this use case.
src/cmd/ksh93/tests/shtest:
- Use 'test -t 1' with stdout (fd 1) redirected to /dev/tty to
ensure the tty is actually on a terminal.
src/cmd/ksh93/tests/basic.sh:
- Remove superflous check for tty. All tests run through shtests.
Resolves: https://github.com/ksh93/ksh/pull/171
src/cmd/ksh93/features/externs: ARG_EXTRA_BYTES detection:
- Improve detection of extra bytes per argument: on every loop
iteration, recalculate the size of the environment while taking
the amount extra bytes we're currently trying into account. Also
count arguments (argv[]) as they are stored in the same buffer.
On 64-bit Linux with glibc, this now detects 9 extra bytes per
argument instead of 8. An odd number (literally and figuratively)
but apparently it needs it; I do think my method is correct now.
On 64-bit Solaris and macOS, this still detects 8 extra bytes.
(On 64-bit Linux with musl C library, it detects 0 bytes. Nice.)
src/cmd/ksh93/sh/path.c: path_xargs():
- Remove the kludge subtracting twice the size of the environment.
With the feature test fixed, this should no longer fail on Linux.
- Take into account the size of the final null element in the
argument and environment lists.
src/cmd/ksh93/tests/path.sh:
- Do not use awk for the test due to breakage in the system awks
on Solaris/Illumos (hangs) and AIX & UnixWare (drops arguments).
Instead, use (wait for it...) ksh. It's a bit slower, but works.
src/cmd/ksh93/tests/bracket.sh:
- Read the list of installed locales to ensure the locale to be tested
actually exists on the system under test.
- Produce a warning diagnostic for skipped locales.
- Additionally test the en_US.ISO8859-1 and en_US.UTF-8 locales.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
src/cmd/ksh93/features/math.sh:
- Specify ast_float.h within iffehdrs instead of math.h, so that iffe
will pick up on macro substitutions within libast. This should make
any future efforts to remedy floating point behavior easier as well.
- Always include ast_float.h within the generated math header file,
not just on IA64 platforms.
src/cmd/ksh93/tests/arith.sh:
- Test pow(1.0,-Inf) and pow(1.0,NaN) for IEEE compliance as well.
- Test the exponentiation operator (**) in addition, as streval.c,
which processes the same, calls pow() separately.
src/lib/libast/features/float:
- Test the IEEE compliance of the underlying math library's pow()
function and substitute macros producing compliant behavior if
necessary.
If I haven't missed anything, this should make the non-interactive
aspects of job control in scripts work as expected, except for the
"<command unknown>" issue in the output of 'bg', 'fg' and 'jobs'
(which is not such a high priority as those commands are really
designed for interactive use).
Plus, I believe I now finally understand what these three are for:
* The job.jobcontrol variable is set to nonzero by job_init() in
jobs.c if, and only if, the shell is interactive *and* managed to
get control of the terminal. Therefore, any changing of terminal
settings (tcsetpgrp(3), tty_set()) should only be done if
job.jobcontrol is nonzero. This commit changes several checks for
sh_isoption(SH_INTERACTIVE) to checks for job.jobcontrol for
better consistency with this.
* The state flag, sh_isstate(SH_MONITOR), determines whether the
bits of job control that are relevant for both scripts and
interactive shells are active, which is mostly making sure that a
background job gets its own process group (setpgid(3)).
* The shell option, sh_isoption(SH_MONITOR), is just that. When the
user turns it on or off, the state flag is synched with it. It
should usually not be directly checked for, as the state may be
temporarily turned off without turning off the option.
Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/sh/args.c:
- Move synching the SH_MONITOR state flag with the SH_MONITOR
shell option from b_set() (the 'set' builtin) to sh_applyopts()
which is indirectly called from b_set() and is also used when
parsing the shell invocation command line. This ensures -m is
properly enabled in both scenarios.
src/cmd/ksh93/sh/jobs.c:
- job_init(): Do not refuse to initialise job control on
non-interactive shells. Instead, skip everything that should only
be done on interactive shells (i.e., everything to do with the
terminal). This function is now even more of a mess than it was
before, so refactoring may be desirabe at some point.
- job_close(), job_set(), job_reset(), job_wait(): Do not reset the
terminal process group (tcsetpgrp()) if job.jobcontrol isn't on.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): TFORK: For SIGINT handling, check the SH_MONITOR
state flag, not the shell option.
- sh_exec(): TFORK: Do not turn off the SH_MONITOR state flag in
forked children. The non-interactive part of job control should
stay active. Instead, turn off the SH_INTERACTIVE state flag so
we don't get interactive shell behaviour (i.e. job control noise
on the terminal) in forked subshells.
- _sh_fork(), sh_ntfork(): Do not reset the terminal process group
(tcsetpgrp()) if job.jobcontrol isn't on. Do not turn off the
SH_MONITOR state flag in forked children.
src/cmd/ksh93/sh/subshell.c: sh_subfork():
- Do not turn off the monitor option and state in forked subshells.
The non-interactive part of job control should stay active.
src/cmd/ksh93/bltins/misc.c: b_bg():
- Check isstate(SH_MONITOR) instead of sh_isoption(SH_MONITOR) &&
job.jobcontrol before throwing a 'no job control' error.
This fixes a minor bug: fg, bg and disown could quietly fail.
src/cmd/ksh93/tests/jobs.sh:
- Add tests for 'fg' with job control IDs (%%, %1) in scripts.
- Add test checking that a background job launched from a subsell
with job control enabled correctly becomes the leader of its own
process group.
Makes progress on: https://github.com/ksh93/ksh/issues/119
Another longstanding whopper of a bug in basic ksh93 functionality:
run a ${ shared-state; } command substitution twice and job control
promptly loses track of all your running jobs. New jobs are tracked
again until you run another two shared-state command substitutions.
This is in at least 93t+, 93u-, 93u+, 93v- and ksh2020.
$ sleep 300 &
[1] 56883
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi1; }
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi2; }
$ jobs # Nothing!
$ fg
ksh: fg: no such job
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- The current environment number shp->curenv (a.k.a. sh.curenv) was
not being restored if the virtual subshell we're leaving is of
the shared-state command substitution variety as it was wrongly
considered to be part of the environment that didn't need
restoring. This caused it to be out of sync with shp->jobenv
(a.k.a. sh.jobenv) which did get restored from savedcurenv.
Restore both from savedcurenv at the same time for any subshell.
(How these numbers are used exactly remains to be discovered.)
src/cmd/ksh93/tests/jobs.sh:
- Added, with a test for this bug to start it off. There is no
other test script where job control fits, and a lot more related
fixes are anticipated: https://github.com/ksh93/ksh/issues/119
It was easier than expected to fix this one. The many regression
test failures caused by disabling it were all due to one bug:
'typeset -p' output broke when building without this option.
src/cmd/ksh93/sh/nvtree.c: nv_attribute():
- In this function to print the attributes of a name-value pair,
move four lines of code out of #if SHOPT_FIXEDARRAY...#endif that
were inadvertently moved into the #if block in ksh93 2012-05-18.
See the changes to nvtree.c in this multishell repo commit:
aabab56a
src/cmd/ksh93/data/builtins.c:
- Update/rewrite 'typeset -a' documentation.
- Make it adapt to SHOPT_FIXEDARRAY.
- Fix a few typos.
src/cmd/ksh93/tests/arrays2.sh:
- Only one regression test needs a SHOPT_FIXEDARRAY check.
.github/workflows/ci.yml:
- Disable SHOPT_FIXEDARRAY when regression-testing without SHOPTs.
- Enable xtrace, add ':' commands for traced comments. This should
make the CI runner output logs a little more readable.
Many compile-time options were broken so that they could not be
turned off without causing compile errors and/or regression test
failures. This commit now allows the following to be disabled:
SHOPT_2DMATCH # two dimensional ${.sh.match} for ${var//pat/str}
SHOPT_BGX # one SIGCHLD trap per completed job
SHOPT_BRACEPAT # C-shell {...,...} expansions (, required)
SHOPT_ESH # emacs/gmacs edit mode
SHOPT_HISTEXPAND # csh-style history file expansions
SHOPT_MULTIBYTE # multibyte character handling
SHOPT_NAMESPACE # allow namespaces
SHOPT_STATS # add .sh.stats variable
SHOPT_VSH # vi edit mode
The following still break ksh when disabled:
SHOPT_FIXEDARRAY # fixed dimension indexed array
SHOPT_RAWONLY # make viraw the only vi mode
SHOPT_TYPEDEF # enable typeset type definitions
Compiling without SHOPT_RAWONLY just gives four regression test
failures in pty.sh, but turning off SHOPT_FIXEDARRAY and
SHOPT_TYPEDEF causes compilation to fail. I've managed to tweak the
code to make it compile without those two options, but then dozens
of regression test failures occur, often in things nothing directly
to do with those options. It looks like the separation between the
code for these options and the rest was never properly maintained.
Making it possible to disable SHOPT_FIXEDARRAY and SHOPT_TYPEDEF
may involve major refactoring and testing and may not be worth it.
This commit has far too many tweaks to list. Notables fixes are:
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/data/options.c:
- Do not compile in the shell options and documentation for
disabled features (braceexpand, emacs/gmacs, vi/viraw), so the
shell is not left with no-op options and inaccurate self-doc.
src/cmd/ksh93/data/lexstates.c:
- Comment the state tables to associte them with their IDs.
- In the ST_MACRO table (sh_lexstate9[]), do not make the S_BRACE
state for position 123 (ASCII for '{') conditional upon
SHOPT_BRACEPAT (brace expansion), otherwise disabling this causes
glob patterns of the form {3}(x) (matching 3 x'es) to stop
working as well -- and that is ksh globbing, not brace expansion.
src/cmd/ksh93/edit/edit.c: ed_read():
- Fixed a bug: SIGWINCH was not handled by the gmacs edit mode.
src/cmd/ksh93/sh/name.c: nv_putval():
- The -L/-R left/right adjustment options to typeset do not count
zero-width characters. This is the behaviour with SHOPT_MULTIBYTE
enabled, regardless of locale. Of course, what a zero-width
character is depends on the locale, but control characters are
always considered zero-width. So, to avoid a regression, add some
fallback code for non-SHOPT_MULTIBYTE builds that skips ASCII
control characters (as per iscntrl(3)) so they are still
considered to have zero width.
src/cmd/ksh93/tests/shtests:
- Export the SHOPT_* macros from SHOPT.sh to the tests as
environment variables, so the tests can check for them and decide
whether or how to run tests based on the compile-time options
that the tested binary was presumably compiled with.
- Do not run the C.UTF-8 tests if SHOPT_MULTIBYTE is not enabled.
src/cmd/ksh93/tests/*.sh:
- Add a bunch of checks for SHOPT_* env vars. Since most should
have a value 0 (off) or 1 (on), the form ((SHOPT_FOO)) is a
convenient way to use them as arithmetic booleans.
.github/workflows/ci.yml:
- Make GitHub do more testing: run two locale tests (Dutch and
Japanese UTF-8 locales), then disable all the SHOPTs that we can
currently disable, recompile ksh, and run the tests again.
The >;word and <>;word redirection operators cannot be used with
the 'exec' builtin, but the 'redirect' builtin (which used to be
an alias of 'command exec') permitted them. However, they do not
have the documented effect of the added ';'. So this commit blocks
those operators for 'redirect' as they are blocked for 'exec'.
It also tweaks redirect's error message if a non-redirection
argument is encountered.
src/cmd/ksh93/sh/parse.c: simple():
- Set the lexp->inexec flag for SYSREDIR (redirect) as well as
SYSEXEC (exec). This flag is checked for in sh_lex() (lex.c) to
throw a syntax error if one of these two operators is used.
src/cmd/ksh93/sh.1, src/cmd/ksh93/data/builtins.c:
- Documentation tweaks.
src/cmd/ksh93/sh/xec.c, src/cmd/ksh93/bltins/misc.c:
- When 'redirect' gives an 'incorrect syntax' (e_badsyntax) error
message, include the first word that was found not to be a valid
redirection. This is simply the first argument, as redirections
are removed from the arguments list.
src/cmd/ksh93/tests/io.sh:
- Update test to reflect new error message format.
Permanent redirections of that form broke in subshells when used
with the 'redirect' command, because I had overlooked one instance
where the new 'redirect' builtin needs to match the behaviour of
the 'exec' builtin.
src/cmd/ksh93/tests/io.sh: sh_exec():
- Do not restore file descriptors in (virtual) subshells for
'redirect' just as this isn't done for 'exec'.
src/cmd/ksh93/tests/io.sh:
- Add regression test for this bug.
- Complete the test for f9427909 which I committed prematurely.
Fixes: https://github.com/ksh93/ksh/issues/167
This is some nonsense: redirections that store a file descriptor
greater than 9 in a variable, like {var}<&2 and the like, stopped
working if brace expansion was turned off. '{var}' is not a brace
expansion as it doesn't contain ',' or '..'; something like 'echo
{var}' is always output unexpanded. And redirections and brace
expansion are two completely unrelated things. It wasn't documented
that these redirections require the -B/braceexpand option, either.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Remove incorrect check for braceexpand option before processing
redirections of this form.
src/cmd/ksh93/COMPATIBILITY:
- Insert a brief item mentioning this.
src/cmd/ksh93/sh.1:
- Correction: these redirections do not yield a file descriptor >
10, but > 9, a.k.a. >= 10.
- Add a brief example showing how these redirections can be used.
src/cmd/ksh93/tests/io.sh:
- Add a quick regression test.
It was trivial to crash ksh by making an autoloaded function
definition file autoload itself, causing a stack overflow due to
infinite recursion. This commit adds loop detection that stops a
function that is being autoloaded from autoloading itself either
directly or indirectly, without removing the ability of autoloaded
function definition files to autoload other functions.
src/cmd/ksh93/sh/path.c: funload():
- Detect loops by checking if the path of a function to be
autoloaded was already added to a new internal static tree,
and if not, adding it while the function is being loaded.
src/cmd/ksh93/tests/path.sh:
- Add regression test.
- Tweak a couple of others to be freeze- and crash-proof.
NEWS:
- Add this fix + a forgotten entry for the previous fix (6f3b23e6).
Fixes: https://github.com/ksh93/ksh/issues/136
