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.

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

Conversation

wader
Copy link
Member

@wader wader commented Jul 5, 2023

Fixes #2572

tests/shtest Outdated Show resolved Hide resolved
tests/shtest Outdated Show resolved Hide resolved
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.

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.

@nicowilliams nicowilliams merged commit 95625c3 into jqlang:master Jul 5, 2023
15 checks passed
@nicowilliams
Copy link
Contributor

Thanks!

@wader wader deleted the jsonarg-is-valid branch July 5, 2023 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failure when using --jsonargs with invalid JSON and printing $ARGS
2 participants