Throw value error when serializing JSON object with NaN value #5810
Conversation
There was a problem hiding this comment.
I don't believe a request() method will throw a ValueError ever and so this constitutes a backwards incompatible change. We should create a new exception, however, that inherits from RequestsException to cover this case and do something like
try:
body = complexjson.dumps(json, allow_nan=False)
except ValueError as ve:
raise InvalidJSONError(json, original_exc=ve)| try: | ||
| body = complexjson.dumps(json, allow_nan=False) | ||
| except ValueError as ve: | ||
| raise InvalidJSONError(ve, request=self) |
There was a problem hiding this comment.
@sigmavirus24 is this okay? Couldn't do it the exact way you specified, but request contains the JSON info.
There was a problem hiding this comment.
I have an instinct of "No" but that's because people do not read the docs and sometimes pass open files and such in addition to the json= keyword and keeping a reference to those might not be the best but that's a "The user has shot themselves in the foot" situation only for high traffic (lots of requests doing the wrong thing explicitly that has undefined behaviour), poorly designed software (because it's potentially not releasing/decref'ing the exception which doesn't decref the request which doesn't decref the file obj).
tl;dr - This should be fine
* disallow nan values in json serialize * test nan value in json post * added exception for invalid json in request * use invalid json exception * invalid json test
* disallow nan values in json serialize * test nan value in json post * added exception for invalid json in request * use invalid json exception * invalid json test
* disallow nan values in json serialize * test nan value in json post * added exception for invalid json in request * use invalid json exception * invalid json test
I am attempting to fix #5767 with this PR. Feedback is really appreciated.