Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate JSON for --jsonarg #2658

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Validate JSON for --jsonarg #2658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,12 @@ int main(int argc, char* argv[]) {
if (further_args_are_strings) {
ARGS = jv_array_append(ARGS, jv_string(argv[i]));
} else if (further_args_are_json) {
ARGS = jv_array_append(ARGS, jv_parse(argv[i]));
jv v = jv_parse(argv[i]);
if (!jv_is_valid(v)) {
fprintf(stderr, "%s: invalid JSON text passed to --jsonargs\n", progname);
die();
}
ARGS = jv_array_append(ARGS, v);
} else {
jq_util_input_add_input(input_state, argv[i]);
nfiles++;
Expand All @@ -330,7 +335,12 @@ int main(int argc, char* argv[]) {
if (further_args_are_strings) {
ARGS = jv_array_append(ARGS, jv_string(argv[i]));
} else if (further_args_are_json) {
ARGS = jv_array_append(ARGS, jv_parse(argv[i]));
jv v = jv_parse(argv[i]);
if (!jv_is_valid(v)) {
fprintf(stderr, "%s: invalid JSON text passed to --jsonargs\n", progname);
die();
}
ARGS = jv_array_append(ARGS, v);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of duplication here. I guess one could extract to some kind of parse_or_die function? could be use for --argjson code also. Maybe for later?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's fine.

} else {
jq_util_input_add_input(input_state, argv[i]);
nfiles++;
Expand Down
13 changes: 13 additions & 0 deletions tests/shtest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,19 @@ fi
echo '{"a":1,"b",' | $JQ --stream > /dev/null 2> $d/err
grep 'Objects must consist of key:value pairs' $d/err > /dev/null

## Regression test for issue #2572 assert when using --jsonargs and invalid JSON
$JQ -n --jsonargs null invalid && EC=$? || EC=$?
if [ "$EC" -ne 2 ]; then
echo "--jsonargs exited with wrong exit code, expected 2 got $EC" 1>&2
exit 1
fi
# this tests the args_done code path "--"
$JQ -n --jsonargs null -- invalid && EC=$? || EC=$?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this should be exit 2 (usage error?) or invalid input error?

I also noticed that we skip using $VALGRIND is some places, should we fix that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not very consistent regarding exit status codes. It'd be nice if a) we documented jq's exit codes besides in the -e case, b) maybe made sure that the various usage and system error exit codes differ from the ones for -e?. But that's not for this issue.

And yes, we should fix the non-use of valgrind, but we should do so separately.

if [ "$EC" -ne 2 ]; then
echo "--jsonargs exited with wrong exit code, expected 2 got $EC" 1>&2
exit 1
fi

## Fuzz parser

## XXX With a $(urandom) builtin we could move this test into tests/all.test
Expand Down