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 validation of payloads containing floats #45

Merged
merged 2 commits into from
Nov 21, 2019
Merged

Conversation

OrangeTux
Copy link
Contributor

The validation of payloads using jsonschemas could fail when the payload
contained a float. This problem is described in this issue:
python-jsonschema/jsonschema#247

This commit implements a work around for this issue by changing the
float parser for certain payloads from float() to decimal.Decimal().

Fixes: #43

@OrangeTux OrangeTux requested review from proelke and tropxy November 13, 2019 20:41
@ghost
Copy link

ghost commented Nov 13, 2019

DeepCode Report (#5fc60d)

DeepCode analyzed this pull request.
There is 1 new warning.

ocpp/messages.py Outdated
@@ -32,7 +33,7 @@ def unpack(msg):
Unpacks a message into either a Call, CallError or CallResult.
"""
try:
msg = json.loads(msg)
msg = json.loads(msg, parse_float=decimal.Decimal)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it ok to always parse floats here but only schemas in a few instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I need to verify that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I started working on this a bit yesterday, and I got stuck on that part of porting the code over. If it's an issue I have an idea.

Copy link
Contributor Author

@OrangeTux OrangeTux Nov 19, 2019

Choose a reason for hiding this comment

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

I've removed the argument parse_float=decimal.Decimal. It is not needed in this place.

If a message requires special validation regarding the floats the message is reparsed again using parse_float=decimal.Decimal. See line 143 of this file.

The validation of payloads using jsonschemas could fail when the payload
contained a float. This problem is described in this issue:
python-jsonschema/jsonschema#247

This commit implements a work around for this issue by changing the
float parser for certain payloads from `float()` to `decimal.Decimal()`.

Fixes: #43
Copy link
Collaborator

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

LGTM

@OrangeTux OrangeTux merged commit 6d2d29d into master Nov 21, 2019
@OrangeTux OrangeTux deleted the 43-parse-float branch November 21, 2019 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix validation of payloads containing floats
3 participants