Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

String "NaN" is coerced to float value when saving data objects (which breaks retrieving the objects) #4589

Closed
9 tasks done
samv-yazzoom opened this issue Feb 16, 2022 · 7 comments
Labels
question Question or problem question-migrate

Comments

@samv-yazzoom
Copy link

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the FastAPI documentation, with the integrated search.
  • I already searched in Google "How to X in FastAPI" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to FastAPI but to Pydantic.
  • I already checked if it is not related to FastAPI but to Swagger UI.
  • I already checked if it is not related to FastAPI but to ReDoc.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

from typing import List

from fastapi import FastAPI
from pydantic import BaseModel

app = FastAPI()


class DataModel(BaseModel):
    a_string_value: str
    a_float_value: float


stored_objects: List[DataModel] = []


@app.get("/", response_model=List[DataModel])
async def get_objects():
    return stored_objects


@app.post("/")
async def save_object(to_save: DataModel):
    return stored_objects.append(to_save)

Description

I have an application where one of the models contains a field that is a float value.
My database to back this is a mongodb instance.
In the example this is mocked by a simple list; make sure to use a single worker thread when trying to test this locally (uvicorn main:app --workers 1).

A user once tried to send a "NaN" float value and my application accepted it and stored it in mongodb.
What I think is happening is that the string value that is posted is being coerced into a float and float("NaN") gives this annoying value.
This stored value of NaN cannot be returned by subsequent get calls.

Example calls to show the problematic behavior when NaN values are accepted:
curl localhost:8000
[]
curl localhost:8000 -H 'Content-Type: application/json' -d '{"a_string_value": "object 1", "a_float_value": 0.5}'
curl localhost:8000
[{"a_string_value":"object 1","a_float_value":0.5}]
curl localhost:8000 -H 'Content-Type: application/json' -d '{"a_string_value": "object 1", "a_float_value": "NaN"}'
curl localhost:8000
Internal Server Error

ValueError: Out of range float values are not JSON compliant

I could add a validation on each of my models that use a float value or create an extension of a float that I then use in my models but this still feels like a risky way of fixing the issue.
If I forget checking for NaN value coercion on any of my models, a user could break the get behaviour by posting a "NaN" value.

This was cleared by @tiangolo for security issues.

Operating System

Linux

Operating System Details

Ubuntu 20

FastAPI Version

0.73.0

Python Version

3.9.10

Additional Context

This issue was first reported as a security issue but was cleared.

@samv-yazzoom samv-yazzoom added the question Question or problem label Feb 16, 2022
@samv-yazzoom
Copy link
Author

My goal is not to accept NaN values but to block the user from sending these to make sure the get api would always work.

@samv-yazzoom samv-yazzoom changed the title String "NaN" is coerced to float value when saving data objects String "NaN" is coerced to float value when saving data objects (which breaks retrieving the objects) Feb 16, 2022
@samv-yazzoom
Copy link
Author

Still occurs in fastapi 0.74.0

@samv-yazzoom
Copy link
Author

import math
from typing import List

import uvicorn
from fastapi import FastAPI
from pydantic import BaseModel

app = FastAPI()


class RegularFloat(float):
    @classmethod
    def __get_validators__(cls):
        yield cls.validate

    @classmethod
    def validate(cls, v):
        if not math.isfinite(float(v)):
            raise ValueError("Invalid float.")
        return cls(v)


class DataModel(BaseModel):
    a_string_value: str
    a_float_value: RegularFloat


stored_objects: List[DataModel] = []


@app.get("/", response_model=List[DataModel])
async def get_objects():
    return stored_objects


@app.post("/")
async def save_object(to_save: DataModel):
    return stored_objects.append(to_save)


if __name__ == '__main__':
    uvicorn.run(
        "main:app",
        workers=1
    )

Example code used to fix my application.

@samv-yazzoom
Copy link
Author

Note that math.isfinite was used to also catch "Inf", "-Inf" and similar values.

@yinziyan1206
Copy link

So how about using Decimal instead of float

@samv-yazzoom
Copy link
Author

samv-yazzoom commented Feb 22, 2022

Decimal and StrictFloat are indeed also working.
ConstrainedFloat does not block the special values.

I will change all floats on all my models to Decimal to assure that the objects can be returned.

Thank you for the help.

@tiangolo
Copy link
Owner

Thanks for the report @samv-yazzoom! And thanks for the help @yinziyan1206.

I see how this behavior feels unexpected, it does to me. I made a PR to Pydantic adding support for more restrictive floats that don't accept "NaN" and "inf" here: pydantic/pydantic#3994

I would like for floats to be able to have that behavior by default, but as Pydantic is used in other cases outside of FastAPI I suppose some people could need to support nan and inf in models, not sure. Anyway, I added a lot more comments in that PR, considering the alternative in Starlette/FastAPI, but also how/why I don't like that alternative that much, etc.

Let's see what happens with that PR to Pydantic.

@tiangolo tiangolo reopened this Feb 27, 2023
Repository owner locked and limited conversation to collaborators Feb 27, 2023
@tiangolo tiangolo converted this issue into discussion #6330 Feb 27, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Question or problem question-migrate
Projects
None yet
Development

No branches or pull requests

3 participants