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

Socket.IO silently crashes when sending NaN or Inf #1438

Open
CodeDook opened this issue Mar 5, 2025 · 5 comments
Open

Socket.IO silently crashes when sending NaN or Inf #1438

CodeDook opened this issue Mar 5, 2025 · 5 comments
Assignees
Labels

Comments

@CodeDook
Copy link

CodeDook commented Mar 5, 2025

Describe the bug
When attempting to emit any data (tuple/list/dict/just one value/etc) containing math.nan or math.inf from Python, instead of being translated to their Javascript equivalents NaN and Infinity, the thread simply crashes without any error, and the client attempts to reconnect and opens a new socket. This happens repeatedly until you stop trying to send those values.

To Reproduce

  1. Start Socket.IO with socketio.AsyncServer(async_mode='tornado')
  2. Start Tornado
  3. Connect client
  4. Send any event with any of the above in it

Expected behavior
math.nan should become NaN and math.inf should become Infinity, and the thread shouldn't crash.

Server Logs

emitting event "ver" to vsc2o8XnBIjCQ-ffAAAB [/]
Connected to 127.0.0.1
emitting event "test" to all [/]
SENT
emitting event "ver" to B27r1WYIu-RJmJZ7AAAD [/]
Connected to 127.0.0.1
emitting event "test" to all [/]
SENT
emitting event "ver" to yXFrxHwK0uEOarWYAAAF [/]
Connected to 127.0.0.1
emitting event "test" to all [/]

It just disconnects with no errors logged on client or server. Event 'ver' is sent on connect (contains a version string), this sends to the client just fine. Event 'test' just sends math.inf a few seconds after connection as a test. The connection drops as soon as 'test' is sent, and it never reaches the client. Connection does NOT close if I send any other number, eg. 1.5.

Additional context
Using python-socketio v5.12.1, tornado v6.4.2, and socket.io client v4.8.1

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Mar 5, 2025

math.nan should become NaN and math.inf should become Infinity, and the thread shouldn't crash.

They probably do work as you say, but this is how these special values work in Python. JSON does not support them, so technically Python including them is not legal. JavaScript's JSON implementation does not have support for these values either.

The correct solution here is to change the Python side to raise an error if these are used, since these are illegal in JSON and JavaScript. So the behavior that you are seeing will remain, but at least the failure will not be silent, instead you will have a ValueError in your logs. Possibly as an option, since it would be fine to do this if Python is also on the other side.

@miguelgrinberg miguelgrinberg self-assigned this Mar 5, 2025
@CodeDook
Copy link
Author

CodeDook commented Mar 5, 2025

I wasn't aware that Socket.IO uses JSON internally but that makes sense. However I just tested what the Python JSON module does with these values, and it does not raise an error by default, it converts both nan and inf to JavaScript null. Which is probably a reasonable solution as well. Any of those is better than silently failing lol

Edit: Okay actually upon a second test to see what it is actually doing, the Python json dumps is converting them to the literal strings "NaN" and "Infinity" and converts them back from strings when doing loads. Seems like if you do json.loads("Infinity") it also works and you get math.inf out I'm not sure how "official" that is as far as JSON standards go, but that is what Python 3.12 seems to be doing.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Mar 5, 2025

It's not legal, the Python documentation indicates so in their allow_nan option:

allow_nan (bool) – If False, serialization of out-of-range float values (nan, inf, -inf) will result in a ValueError, in strict compliance with the JSON specification. If True (the default), their JavaScript equivalents (NaN, Infinity, -Infinity) are used.

(Copied from this page.)

This is great, but JavaScript's JSON parser does not recognize these as valid, so they can't be used when the client is JS.

@CodeDook
Copy link
Author

CodeDook commented Mar 5, 2025

I see. That's frustrating, but perhaps a similar option to allow_nan can be added to python-socketio to change whether it throws an error or attempts to convert (either to strings or to null)

@miguelgrinberg
Copy link
Owner

Converting to strings is not something I would consider, since a) it is incorrect and b) is not done by any other implementation (to my knowledge, at least). The option is going to be exactly as implemented in json.dumps(), either it is allowed or not allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants