in the past, but managed to re-surface...
The expression "${0+\}}" should expand to "}" not "\}"
Almost all other shells handle it that way (incl FreeBSD & dash).
Issue pointed out by Martijn Dekker.
Add ATF sub-tests for the 4 old var expand operators (${var+word}
${var-word} ${var-word} and ${var?word} - including the forms
with the ':' included) and amongst those tests include test cases
for this issue, so if the bug tries to appear again, we can squash
it quicker. (The newer pattern matching operators are already
well tested as part of testing patterns.)
Fix handling of "$@" (that is, double quoted dollar at), when it
appears in a string which will be subject to field splitting.
Eg:
${0+"$@" }
More common usages, like the simple "$@" or ${0+"$@"} end up
being entirely quoted, so no field splitting happens, and the
problem was avoided.
See the PR for more details.
This ends up making a bunch of old hack code (and some that was
relatively new) vanish - for now it is just #if 0'd or commented out.
Cleanups of that stuff will happen later.
That some of the worst $@ hacks are now gone does not mean that processing
of "$@" does not retain a very special place in every hackers heart.
RIP extreme ugliness - long live the merely ordinary ugly.
Added a new bin/sh ATF test case to verify that all this remains fixed.
bug (and its fix (depending upon whether the test is run against
an unfixed, or fixed, shell).
An obvious indication of the failure is the following (one of the
new sub-tests)
p=A
cat <<EOF
${p+\%$p\%}
${p+%$p%}
EOF
which should output
\%A\%
%A%
as a here doc is treated as a double quoted string, except
that the " character is just a character. In such a string,
the \ is only an escape character when the following character
is special, otherwise it represents a literal \ (which is the
case here).
An unfixed shell will omit the backslashes in the output.
It gets even more wrong if the % chars are replaced by "
(double quote) chars, which should make no difference, other
than the corresponding change, in the output. But doesn't
(it doesn't even produce output broken in a similar way).
This one is a harder case to be specific about however,
as while the fixed (and expected in the test) output is what
is technically correct, only a few shells actually produce
it, most generate something different (but not all the same.)
when the locale is ru_RU.UTF-8 (which has ',' as its decimal radix char).
Inspired by a problem with rc.subr experiened by ru_RU.UTF-8 reported on
netbsd-users. These are in the "fraction" test case.
While here, add some more (sub-)tests of invalid input, to make sure they
fail (in the "nonnumeric" test case), and for fun, a couple of subtests
testing hex input fractional delays (in the "hex" test case).
including testing correct handling of error cases.
One of these new tests located a bug which will be fixed
within minutes of this commit ...
While doing this, move the test that was used only in the
echo builtin test case (testing if the NetBSD shell was being
tested) into a utility function, and also call it in the eval
test, so NetBSD specific behaviour of what is unspecified in
the standard can be checked to ensure it is not accidentally
altered.
In particular, add a readonly test to detect the bug that was just fixed...
(but there is more than that one added here).
Also, allow zsh to run more of these tests than it did, what is a builtin
command most places (including in POSIX) can be a reserved word in zsh!
utilities. That is, not the low level ones that look like syntax,
but aren't: break/continue/return; not those which are really
just external programs that are built in for efficiency (printf,
test, and kill - though kill really needs to be built in) - those
should all have separate test programs (there is a test here for the
built-in echo, as that is an entirely different thing to /bin/echo);
and also not those for which there are other tests because of the
nature of the built-in (like exit, wait, shift, ...). Lastly not
"times" either as that just seems to be too hard to test rationally.
There is a test (well, framework) for ulimit and there's also t_ulimit.sh
one of those should go, but I am not yet sure which is the best way
to reconcile things.
Note: many (in fact) most of the test cases added here are either
entirely empty (no tests at all, beyond testing that the built-in is
in fact a shell built-in) or only very rudimentary tests - assistance
in fleshing those out would be welcome (the boilerplate is all here,
all that is needed is some actual tests to run...)
(and a few more subtests in an existing test case).
The two new test cases currently fail, because of issues with
expanding "${1+$@}" which will (hopefully) be fixed soon.
(This looks to have been broken sometime during 2013 ... then I
made it even worse with some of the parser changes a while ago,
though the end result is that it appears less broken than it
really is.)
includion a test to make sure that the file doesn't get truncated.
Add new subtests to the "incorrect redirections" test case, to
validate correct behaviour of the shell when redirections fail in
various scenarios, including when the redirect is the whole command.
More along these lines are really needed, but this is better than nothing.
All the added tests pass on the /bin/sh currently in netbsd HEAD.
was not being done properly (too many different code paths inside sh)
so add even more subtests to the case_matching test case to verify
that all (that I can think of for now anyway) get fixed when this
gets cleaned up. The case_matching test case still fails, but now
6 of its subtests should fail (until sh is corrected ... soon).
Martijn Dekker (private e-mail.) Variable expansions that are
double quoted result in literal characters (nver pattern matching
meta chars.) This includes '\' (that one was the bug.)
[On the other hand, a variable in a case pattern expansion that is
unquoted, produces a pattern, and in that the \ character can be
used to excape other pattern meta-chars (and itself.]
This addition will cause the case_matching test case to fail (two
of the newly added sub-tests fail) until fixes to /bin/sh are made.
(That is comiung soon, the code exists already.)
In the filename expansion test, don't insist on testing cases
of [^a] anything not an a) (etc) - while almost all shells allow
^ there (as in regular expressions) this is not standard sh/glob syntax
(! is used for that, not ^). Use of ^ as first char after '[' in a []
match has unspecified behaviour. So, check if the shell being
tested supports ^ used this way before running the tests of ^ in [].
This makes no difference at all when testing the NetBSD sh which
does allow ^ for that use case.
Fix an obvious (in hindsight) stupidity in the case pattern tests
(a cut/paste/forget-to-fix type error).
Many cleanups, including some additional sub-tests, for the var
substring matching (just a couple of actual fixes to the tests).
(This was the last added, and least polished part - still is.)
This makes no difference to the filename expansion tests (11 of 167
still fail), the fix reduces the failing case match tests from
14 of 261 to 13 of 261. For var substring tests, the failure
has changed from 6 of 87 to 8 of 99. (Some) Fixes to sh will appear
soon.
For comparison, using this version of the test, dash passes all case
and var substring tests, and (aside from the [^...] tests which it
does not implement) fails 4 of the filename tests. bash fails one
case test (a bash oddity in interpretation of the spec, which is unique
to bash amongst shells I have tested) and one filename expansion (all
var substring tests pass). Other shells are much worse (though the
freebsd shell results were coloured by the version of their shell I
tested still having the [[:xxx:]] always matches '[' bug, which is
fixed in later versions of their shell)
patterns, and variable expansion substring matching)
Currently (2018-07-10) all 3 sub-tests fail (sh bugs...)
Expect to see 14 (of 261) case matching sub-tests fail, 11 (of 167) filename
expansion (glob) sub-tests fail, and 6 (of 87) var substring sub-tests fail.
Also expect those numbers to reduce as sh bugs are fixed.
Assert that -9223372036854775808 % -1 and -9223372036854775808 / -1 return
message about overflow / underflow detected.
These tests pass correctly.
Sponsored by <The NetBSD Foundation>
Currently this test case will fail, a fix is coming soon (not worth
marking this as an expected failure.)
This test case and the initial bug report comes from
Martijn Dekker's modernish (shell/test set).
Currently (or when testing any shell that does not support -X) the
test will be skipped (also for [m]ksh (but not ksh93 etc) where there
is an absurdly badly named -X option, skip the new test for them as well.)
When -X appears in /bin/sh, this will verify that it is probably working
(the test is MUCH more gruelling than any rational use of -X would be.)
Currently testing ~user is too much effort to contemplate (other than
assuming that it works in order to verify that it works...) so only bare ~
is being tested for now. Maybe someday...
Right now (@ time of commit), this new test is expected to fail, as ~
expansions are horribly badly broken (have been for months, some forms
for much longer) in all but the simplest of uses. Fix for that coming
very soon.
This test will be skipped on shells (such as /bin/sh in -current as of
the date of this commit) which do not support $'...'
While here fix a typo in a comment (there are probably more...)
Add two new sh syntax test cases to check for bug fixes for the parser
problems (syntax errors unidentified) reported in the two PRs.
In the latter case, also include some tests that test similar
looking, valid, syntax - to make sure the fix for the PR does not
break code that should not fail. This is not needed for the earlier
PR as other tests, and just normal parsing, is sufficient.
These tests have been verified to fail with a current /bin/sh and to
pass with the sh updates that are to be committed soon -- and because that
fix is expected within hours, the tests are not marked as expected to
fail, just let them actually fail for now.
after than in ksh and bash, and the similar thing in zsh) for which a
request has been made for support in the NetBSD sh.
Until the support is committed, the test will be skipped.
STACKSTRNUL() bug to eventually be found. Not any of the test cases
directly - the shell running the tests (the same /bin/sh) managed to
build one of the f_variable_syntax sub-tests incorrectly, and that was
enough to eventually allow the bug to be located and squashed.
Like all good promotions, this one comes with increased work, and no extra pay.
We currently parse var expansions like "${x-"a b c"}" incorrectly
according to POSIX (and ancient tradition) though in a way that is
generally unexpcted (there are 2 quoted strings there, according
to the standard, "${x-" and "}" and 'a b c' is unquoted.) This has
never been fixed in our sh, as like in many other shells, it is just
a little unbelievable, most people expect that inside the {} is a
whole new ballpark, and everything starts again, and it does in cases
like "${x%"a b c"}" (the newer matching operators.) There have been
comments in this test explaining how the test is actually testing for
incorrect behaviour for a while now.
But I have since learned of POSIX bug #221 (2010), the resolution of which is
to make the problem case unspecified. This will not actually become part
of posix until the next major version (POSIX 8) whenever that appears (still
years away I expect) - but at least we now can feel happier about the
way our code works and not worry quite so much about testing that incorrect
code keeps working incorrectly....
produces output with a very large number of consecutive embegged \n
characters.
This test is currently expected to fail (as of commit date) but is not
marked as atf_expect_fail as the shell should be fixed to avoid the
problem quite soon. Until then almost anything might happen to the
sh that runs this test (from just plain wrong results, to core dumps,
even possibly the right results, though that's unlikely).
Whie doing this, get rid of the duplicate implementation of the
nested_cmdsubs_in_heredoc test (which was achieving nothing). I know
how I managed to do that, but on advice of counsel, I ... (it was
just a harmless waste of a tiny amount of CPU time "compiling" the test,
just like writing "x=0;" on consecutive lines....)
These should detect if the errors that caused MAKDEV to fail and
pkgsrc/pkgtools/cwrappers to fail to build (problem detected in
libnbcompat/configure) ever return.
Also fixed one of the other sub-tests so that it actually does what
it should - no idea how this one has been passing, it did not for me
when I was checking the new ones (but perhaps in the interim I have
fixed something else in sh, the problem was in the area I have been
playing, and I originally debugged the new tests using a newer version
of /bin/sh than has yet been committed.)
by IFS just the same as any other expansion (split when they should be,
and not when they shouldn't).
Good thing I did ... this discovered a regression in the new expand
code (from an hour or three ago) where quoted arith expansions are
being split, and shouldn't be. (on the other hand, when they should
be split, they are being split correctly, so that's good...)
No need to worry too much about this, in real life (as opposed to
torture tests) no-one ever puts digits in IFS, and a $(( )) expansion
only ever contains digits, so in practice, splitting never happens,
whether because it is quoted, or just becaue there is nothing to split.
The FreeBSD shell does it correctly (they do word splitting using a
totally different mechanism than we do) - I will find where I missed
testing for quoted expansions later tonight. Until that happens,
expect a test failure from t_fsplit:split_arith
While here add more comments in the other test casess (one had a comment
already) that our shell parses incorrectly (this is a parsing problem,
not an expansion problem, and the bug has existed forever .. and is
shared by bash).
That is, in an expression like "${var:-word}" the whole thing (including
word) is quoted by the double quotes, and in "${var:-"word"}" the
expansion is quoted, but 'word' is not, the 2nd " (the one before 'w')
ends the opening quote, and the third (before }) turns quoting on
again (it is required that there be an even number of unquoted quotes inside
a ${} expression, so things like "${var:-"word} are simply invalid.)
Another way of saying this, is that if the { is quoted (for which the
only way is using " to get to this kind of expansion at all) the }
must also be quoted (" quoted). There is no quote nesting here.
This also means that in "${var:-'word'}" the single quotes are just
characters, they are quoted by the "" that surround them, just as in
"a 'string' containing quotes", but "${var:-"'word'"}} is valid and
contains a single quoted word.
Note that this is different than the patetrn-match/trim expansions
( ${var%pattern} etc) where any surrounding quotes do not affect the
pattern, and all forms of quoting can be used in it - but it always
parses as a pattern, and is never subject to filename expansion, so
a '*' in it that is to mean "match any string" does not need special
quoting to avoid it expanding to file names, regardless of whether
the ${var%*} is quoted or not, but a * that is to be matched as a
character literally does need to be quoted.
I will probably file a PR about this bug sometime, but as it is
very old, and doesn't every seem to bother anyone (and is shared by
bash) there isn't any big hurry to open yet another verry messy can
of excrement to fix it...
This does mean that the FreeBSD shell "fails" some of our tests.
(The test is wrong, their shell is correct.)
as to why ${011} is ${11} and not ${9} (that is, why we interpret it
that way, the "why could it not be the other way?" is just "because
that is not how it was ever implemented".
piece of mind (to verify I was not breaking anything here.)
Also added some commentary better explaining why one of the tests of splitting
quoted variable expansions is almost certainly incorrect (both the tests,
and what sh does) ... though bash does the same as us, so all is not lost!
whose meaning is defined entirely by context.
For those who read commit messages, and want a (small) challenge,
work out where (and what) to insert as punctuation/operator chars
in the following to produce 3 ines of output, and what those will be:
for in in in do in do case in in in echo do do echo in esac done
(There are no comments, quotes of any kind, or any kind of sub-shell,
including cmd substitutions) With correct non alpha-numeric chars added,
it works.
but with \ newline line continuations inserted at strange places.
For the shell as it is today, since strip passes, wrap_strip should
automaticallty pass as well (and does), as the \ newline combination
is simply removed, producing identical input to that of strip.
However, for accurate line counting ($LINENO: coming soon) newlines (no
matter the context) cannot simply "go away" - we have to know where they
occur(ed) (perhaps long after the text was read) so we know what line
number we are actually processing. This new test case is (perhaps just
part) of future-proofing that the modified code does not break anything.
ryo