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

Fix JSON validation for mismatched brackets #115

Merged
merged 5 commits into from
Jun 14, 2022

Conversation

muneebahmed10
Copy link
Contributor

Fixes an issue where an invalid JSON with mismatched outermost brackets may be marked as valid, e.g. "{ \"Hello\": \"World\" ]". This is due to the validator not checking for matching brackets at the outermost depth.

While fixing this, I also found a bug in the unit tests, where a case that was supposed to error was checking for success instead. If that test case had been correct, then this bug would've been caught.

It was a little frustrating running the unit tests on my computer, since they don't compile with gcc 10 or later. I had to pass in the cmake flag -DCMAKE_C_FLAGS="-fcommon" in order to run them, which I couldn't find documented here. This hasn't been caught by GitHub Actions since Actions is using ubuntu-latest, which is still 20.04 and comes with an older version of gcc.

#define MISMATCHED_BRACKETS2_LENGTH ( sizeof( MISMATCHED_BRACKETS ) - 1 )
#define MISMATCHED_BRACKETS2_LENGTH ( sizeof( MISMATCHED_BRACKETS2 ) - 1 )

#define MISMATCHED_BRACKETS3 "{\"foo\":[\"bar\",\"xyz\"]]"
Copy link
Contributor

@paulbartell paulbartell Jun 13, 2022

Choose a reason for hiding this comment

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

This is only used once.. Does it really need to be a #defined constant?

@aggarg aggarg merged commit 9c68791 into FreeRTOS:main Jun 14, 2022
@muneebahmed10 muneebahmed10 deleted the fix-validate branch June 14, 2022 20:00
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.

4 participants