use grep -E, add Makefile, add -S, -A, -T options, fix ensure_deps.sh for macOS, fix command line parsing, make JSONPath.sh and test scripts robust #16
Conversation
While we disagree with the notion that `egrep` is obsolete, tools such as gnu egrep whine when used with the warning: ``` egrep: warning: egrep is obsolescent; using ggrep -E ``` Sadly this pull request changes use of `egrep` to `grep -E` in order to stop the warnings that gnu egrep now issues.
Add a Makefile so that `make install` will work. Updated `README.md` to mention the `make install` alternative. Removed trailing whitespace from `README.md`.
We add a `-S` option to print spaces around :s'.
The default (w/o -S):
```json
"submit_slot":2,
```
With `-S`:
```json
"submit_slot" : 2,
```
We add an `-A` option start a JSON array on line as JSON member.
The default (w/o -A):
```json
"authors":
[
```
With `-A`:
```json
"authors":[
```
With `-S -A`:
```json
"authors" : [
```
We add `-T` to indent with tabs.
The default (w/o -T):
```json
"submit_slot":2,
"author_count":3,
```
With `-T`:
```json
"submit_slot":2,
"author_count":3,
```
We also removed a few trailing whitespace from the shell script.
lcn2
changed the title
lcn2
changed the title
For macOS (Darwin) we use use the correct paths for homebrew in `ensure_deps.sh` If there is an error under macOS, we also remind the user what to put in their $PATH.
lcn2
changed the title
Thank you @mclarkson.
It matches what We (the 9International Obfuscated C Code Content - IOCCC](https://ioccc-src.github.io/temp-test-ioccc/) plan to use your tool in our processing of submissions (submissions come with 2 JSON files that need to be manipulated and transformed / merged into other JSON files). We plan to use |
The command line parsing now uses the bash builtin `getopts`. Now, and invalid option is flagged as an error. Improper arguments to command line options are flagged as errors. The optional pattern is also obtained after parsing command line options. The usage message has been updated to include command line options that were implemented but not documented in the usage message. Updated the Invocation section of `README.md` to reflect the current command line, including command line options that were documented in the usage message, but not in `README.md`.
lcn2
changed the title
|
We updated this pull request to include a fix of the command line parsing, and updated the usage message and invocation text of UPDATE 0We also fix the command line usage of the tests to match the original |
The original usage message and `README.md` invocation has the optional pattern at the end of the command line . We fix 3 tests to place the pattern at the end of the command line. Moreover, we put `--` between command line options and a pattern to prevent the pattern from being interpreted as a command line option. And while unlikely to happen, this would allow pattern that begins with a dash to be correctly parsed. Ran `all-tests.sh` to test the above.
We ran into some issues with the test scripts that were fixed by some
shell script hardening and using some bash best practices. What follows
are fixes and improvements to the test scripts.
We make the tests scripts more robust:
* detect when the cd fails
* prevent unquoted shell variable expansion from corrupting execution
We improve the use of bash:
* use `env(1)` in the initial `#!` line to find `bash(1)` on `$PATH`
NOTE: macOS has deprecated the distributed `bash(1)` in favor of `zsh(1)`.
Not all systems have `bash(1)` as `/bin/sh`.
We allow for `bash(1)` to come from home brew or mac ports, or ...
We use bash constructs for slight speed gain and best practices:
* increment using ((++i)), in some cases avoiding a sub-shell launch
* bypass use of `cat(1)` by using `$(< "$argpfile")`
* use `find(1)` to select test files instead of the slower `ls(1)`
* avoid using shell glob problems by using `find(1)` to select test files
* use `$(...)` instead of `\`cmd\`` to help avoid data corrupting execution
Use `python3(1)` instead of just `python(1)`.
NOTE: Not all systems that have `python3(1)` have `python(1)`.
Some system admins have yet to select the default version of python.
The macOS platform, while they have `python3(1)` to not have `python(1)`.
The test scripts now pass the shellcheck script analysis tool.
See [www.shellcheck.net](https://www.shellcheck.net) for details.
Fixed a bug in `all-docker-tests.sh` where Centos tests were re-displayed
a 2nd time as if they were Debian tests.
Ran `./all-tests.sh` to verify the above under macOS and RHEL 9.
TODO: Fix a number of shell script problems with `JSONPath.sh` along
similar lines as above: especially where shell variables containing
JSON and other test data can take on values that can
corrupt the shell script execution. We wanted to make the
other test scripts more robust before taking on `JSONPath.sh`
so that we may use such tests.
lcn2
changed the title
lcn2
changed the title
|
Hello @mclarkson and other maintainers, Thank youWe the IOCCC judges have found the Making JSONPath.sh bullet proofPeople will see, when we public our tools, that we will be using your The IOCCC encourages people to submit code that is fairly twisted :-) and in some cases to bend the content rules to the breaking point. This will likely include people messing with the JSON data that a submission needs to describe itself. So we need to be prepared. 🤓 For the International Obfuscated C Code Contest - IOCCC we need the Test script hardenedWe have discovered cases where pathologic shell variable values could corrupt the execution of the script. We have begun to harden We discovered some issues with the test scripts where shell script hardening as well as applying other best bash practices helped, along with addressing some macOS and other probability issues. As a result we added commit c4c8ec2 to this PR. Hardening JSONPath.sh nextWe plan to fix a number of shell script concerns with Please stay tuned for the next commit (in a day or so). |
lcn2
changed the title
lcn2
changed the title
We make `JSONPath.sh` much more robust and use bash constructs for slight speed gain and best practices: * prevent unquoted shell variable expansion from corrupting execution * compute numerical values using ((var=equation)) instead of let * declare local variable before assigning a value * fixed how options to grep are accumulated * fixed how main is called * fixed how an unexpected EOF with token is thrown * fixed how array elements are unset * fixed how code determines of an array contains only empty values * changed cases where `cat(1)` of a file is piped into feeding the file on stdin * use `$(...)` instead of `\`cmd\`` to help avoid data corrupting execution * replaced use of `[ ... ]` with `[[ ... ]]` * removed shell variables that were set but never unused * fixed cases where a case statement lacked a default case * fixed how the literal string `'"*'` is used * fixed cases where a scalar is followed by a literal string that starts with `[` * fixed cases where `$@` is confused with `$*` All errors, warnings and issues reported by [shellcheck](https://www.shellcheck.net) have been addressed / fix. Now `JSONPath.sh` is error, warning and issue free under [shellcheck](https://www.shellcheck.net)! Ran `./all-tests.sh` to verify the above under macOS and RHEL 9 Linux.
|
With commit d7660c7 we have made JSONPath.sh more robust: We make
All errors, warnings and issues reported by shellcheck Ran |
|
Any change that this pull request could be processed? |
Hi there! Yes, in the next few days I should think. I'm very happy you made this PR, the work was long over due. Thank you! |
You are most welcome. |
egrep
While we disagree with the notion that
egrepis obsolete, tools such as gnu egrep whine when used with the warning:Sadly this pull request changes use of
egreptogrep -Ein order to stop the warnings that gnu egrep now issues.Add a Makefile so that
make installwill work.Updated
README.mdto mention themake installalternative.Removed trailing whitespace from
README.md.new command line options
We add a
-Soption to print spaces around :s'.The default (w/o -S):
With
-S:We add an
-Aoption start a JSON array on line as JSON member.The default (w/o -A):
"authors": [With
-A:"authors":[With
-S -A:"authors" : [We add
-Tto indent with tabs.The default (w/o -T):
With
-T:The IOCCC plans to use
-S -A -Tas this is identical to whatjparse(1)(and related friends) produce.Miscellanious
We also removed a few trailing whitespace from the shell script.
For macOS (Darwin) we use use the correct paths for homebrew
in
ensure_deps.shIf there is an error under macOS, wealso remind the user what to put in their $PATH.
command line parsing
fix and improve command line parsing and
The command line parsing now uses the bash builtin
getopts. Now, andinvalid option is flagged as an error. Improper arguments to command
line options are flagged as errors. The optional pattern is also obtained
after parsing command line options.
The usage message has been updated to include command line options that
were implemented but not documented in the usage message.
Updated the Invocation section of
README.mdto reflect the currentcommand line, including command line options that were documented in
the usage message, but not in
README.md.fix test command lines
The original usage message and
README.mdinvocation has the optionalpattern at the end of the command line . We fix 3 tests to place the
pattern at the end of the command line. Moreover, we put
--betweencommand line options and a pattern to prevent the pattern from being
interpreted as a command line option. And while unlikely to happen,
this would allow pattern that begins with a dash to be correctly parsed.
Ran
all-tests.shto test the above.improve test scripts
We ran into some issues with the test scripts that were fixed by some
shell script hardening and using some bash best practices. What follows
are fixes and improvements to the test scripts.
We make the tests scripts more robust:
We improve the use of bash:
env(1)in the initial#!line to findbash(1)on$PATHNOTE: macOS has deprecated the distributed
bash(1)in favor ofzsh(1).Not all systems have
bash(1)as/bin/sh.We allow for
bash(1)to come from home brew or mac ports, or ...We use bash constructs for slight speed gain and best practices:
cat(1)by using$(< "$argpfile")find(1)to select test files instead of the slowerls(1)find(1)to select test files$(...)instead of\cmd`` to help avoid data corrupting executionUse
python3(1)instead of justpython(1).NOTE: Not all systems that have
python3(1)havepython(1).Some system admins have yet to select the default version of python.
The macOS platform, while they have
python3(1)to not havepython(1).The test scripts now pass the shellcheck script analysis tool.
See www.shellcheck.net for details.
Fixed a bug in
all-docker-tests.shwhere Centos tests were re-displayeda 2nd time as if they were Debian tests.
Ran
./all-tests.shto verify the above under macOS and RHEL 9.improve JSONPath.sh
We make
JSONPath.shmuch more robust and use bash constructs for slightspeed gain and best practices:
cat(1)of a file is piped into feeding the file on stdin$(...)instead of\cmd`` to help avoid data corrupting execution[ ... ]with[[ ... ]]'"*'is used[$@is confused with$*All errors, warnings and issues reported by shellcheck
have been addressed / fix. Now
JSONPath.shis error, warning and issue freeunder shellcheck!
Ran
./all-tests.shto verify the above under macOS and RHEL 9 Linux.