Stop swallowing JSON parse errors #67
Conversation
cooperka
changed the title
|
Edited to add context, fyi (GitHub posted this PR from their editor without a description) |
| @@ -65,7 +65,7 @@ def verify_array_index!(path, index, size) | |||
| def safe_json_parse(data) | |||
| JSON.parse(data) | |||
| rescue JSON::ParserError | |||
| nil | |||
| raise JSON::ParserError, "Malformed JSON in multipart form data" | |||
There was a problem hiding this comment.
This function is no longer safe, so let's remove it and use JSON.parse directly where needed.
|
I don't remember why it was done like this, so yes, let's try to remove safety :) |
|
thanks for your patience -- feedback addressed and ready for re-review. |
|
Thank you. |
| @@ -6,3 +6,4 @@ | |||
| /pkg/ | |||
| /spec/reports/ | |||
| /tmp/ | |||
| /vendor/bundle/ | |||
There was a problem hiding this comment.
Common path for local gems to be stored
| example.run | ||
| described_class.strict_mode = mode | ||
| end | ||
|
|
There was a problem hiding this comment.
Honestly not sure what changed here, tabs and line endings seem unaltered 🤷
View the diff with ?w=1 in URL params to show actual changes.
|
Done! I basically just changed |
|
Looks good. Thank you for the contribution! |
If unparseable JSON is provided, the library should fail loudly rather than ignoring the input.
Looking through the blame, I couldn't find any specific reason why
safe_json_parseis meant to be safe -- in fact, because it swallows up errors and pretends that there's no problem, it made things very confusing to diagnose.Let me know if you'd like me to take this in any particular direction and I'm happy to update the PR.