protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Maximum 32 bit float value fails to round trip.

Open jmphilli opened this issue 4 years ago • 3 comments

What version of protobuf and what language are you using? Version: 3.14.0 Language: Python

What operating system (Linux, Windows, ...) and version? MacOS X 10.15.4

What runtime / compiler are you using (e.g., python version or gcc version) Python 3.8.0

What did you do? Steps to reproduce the behavior:

  1. Encode a message with a float who has the maximum 32 bit float value as defined in type_checkers._FLOAT_MAX.
  2. Decode that message into a python dictionary using json_format.MessageToDict
  3. ReEncode the python dictionary into a new protobuf message using json_format.ParseDict

What did you expect to see A new protobuf message with the float_value set to the maximum float value.

What did you see instead? The library raised an error because the float value was too large.

I created a pull request [not intending to merge] to exemplify the round trip failure over here. If that test were to pass we would know that the issue has been resolved.

The crux of the issue is this code. Python rounds up on this maximum float value. That causes the actual number to be greater than the max float which means it cannot be encoded in four bytes.

Possibly related to this old issue in C++ Anything else we should know about your project / environment N/A

jmphilli avatar Feb 23 '21 21:02 jmphilli

@acozzette any chance a fix will make it into future versions [pinging you because you had activity on the relevant pr]

jmphilli avatar Mar 23 '21 20:03 jmphilli

I'm curious now as to whether this still breaks under Python/upb. I will investigate.

ericsalo avatar Sep 01 '22 18:09 ericsalo

I have just patched the upb JSON parser to correctly handle float values very near the overflow limit, which is necessary but not sufficient for fixing this bug. The rest of the work will need to happen at the Python layer.

ericsalo avatar Sep 07 '22 21:09 ericsalo

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Feb 25 '24 10:02 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Mar 13 '24 10:03 github-actions[bot]