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

config: throw when json is invalid #1123

Merged
merged 2 commits into from
Jun 15, 2017
Merged

Conversation

danielhochman
Copy link
Contributor

@danielhochman danielhochman commented Jun 15, 2017

In certain scenarios where invalid JSON is passed, we would hit NOT_REACHED. This changes the behavior to pass false back to the parser. This will cause the parser to stop and throw an error message with offset and line number.

@mattklein123
Copy link
Member

Since this can happen, do we have coverage of all of the ways this can happen?

@danielhochman
Copy link
Contributor Author

I'm not sure of all the ways it can happen or how to manipulate the parser to hit some of these states. Which state the parser decides to proceed to when something outside of the JSON specification is passed to it is a mystery to me.

@mattklein123
Copy link
Member

My opinion is to fix the crash that now has coverage and leave the others to just crash. We can fix them as cases are brought to our attention. But I don't feel that strongly.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

nice, thanks.

@danielhochman danielhochman merged commit 137ff18 into master Jun 15, 2017
@danielhochman danielhochman deleted the json-loader-invalid-throw branch June 15, 2017 22:34
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
Automatic merge from submit-queue.

[DO NOT MERGE] Auto PR to update dependencies of proxy

This PR will be merged automatically once checks are successful.
```release-note
none
```
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Updating to pull in #13074 which will use Apple's DNS resolver for iOS in order to allow Envoy Mobile to ship on iOS 14 without triggering the "local area network" modal.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Updating to pull in #13074 which will use Apple's DNS resolver for iOS in order to allow Envoy Mobile to ship on iOS 14 without triggering the "local area network" modal.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
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.

3 participants