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::ParserError when image data is an invalid json string #37

Merged
merged 6 commits into from Apr 4, 2020
Merged

Fix JSON::ParserError when image data is an invalid json string #37

merged 6 commits into from Apr 4, 2020

Conversation

gael-ian
Copy link
Contributor

@gael-ian gael-ian commented Apr 3, 2020

Hello,

This pull request should fix the JSON::ParserError raised when one of the validator is fed with an invalid JSON string (issue #20).

Talking about validations, you always have to decide wether something will be considered valid or not. I choose to handle invalid JSON strings as invalid values and to add an error to the record. That's why are exceptions are rescued at this level instead of in the parse_values methods of each validator: I needed the record and attribute to add the error and did not want to pass them to parse_values.

The error message use the default :invalid message, as there's no need to be more precise. Invalid JSON string are not common things and are mostly due to nasty bots trying to break in an application. I don't think we need to be more explicit on this.

I hope you'll find the time to merge this soon.

Thanks

@coveralls
Copy link

coveralls commented Apr 3, 2020

Coverage Status

Coverage increased (+26.09%) to 99.6% when pulling c4bde2e on notus-sh:json-parser-error into 74d9a59 on musaffa:master.

@musaffa musaffa merged commit 7572767 into musaffa:master Apr 4, 2020
@musaffa
Copy link
Owner

musaffa commented Apr 4, 2020

@gael-ian Your PR has been merged. Thanks! ❤️

@gael-ian
Copy link
Contributor Author

gael-ian commented Apr 4, 2020

The pleasure is mine ;)

@gael-ian gael-ian deleted the json-parser-error branch February 23, 2023 01:31
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.

None yet

3 participants