Skip to content

🐛 Allow_inf_nan option for Param subclasses#11579

Closed
pat-lasswell wants to merge 3 commits intofastapi:masterfrom
pat-lasswell:master
Closed

🐛 Allow_inf_nan option for Param subclasses#11579
pat-lasswell wants to merge 3 commits intofastapi:masterfrom
pat-lasswell:master

Conversation

@pat-lasswell
Copy link
Copy Markdown

@pat-lasswell pat-lasswell commented May 15, 2024

There was a typo in the __init__ method of the Param class which misnamed the allow_inf_nan option in the kwargs passed to the underlying Pydantic FieldInfo initializer. This resulted in the validator failing to enforce the constraint, and hence inf and nan values were being passed to the route handler when they should not have.

See #11577 for the original question.

pat-lasswell and others added 2 commits May 15, 2024 06:53
There was a typo in the __init__ method of the Param class which
misnamed the allow_inf_nan option in the kwargs passed to the
underlying Pydantic FieldInfo initializer.  This resulted in the
validator failing to enforce the constraint, and hence inf and nan
values being passed to the route handler where they should not have.
@Kludex
Copy link
Copy Markdown
Member

Kludex commented May 15, 2024

@pat-lasswell
Copy link
Copy Markdown
Author

I just noticed that I put the test file in the root rather in the tests directory. I pushed a commit to fix that.

Copy link
Copy Markdown
Contributor

@iudeen iudeen left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@oandersonmagalhaes oandersonmagalhaes left a comment

Choose a reason for hiding this comment

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

LGTM!

@alejsdev alejsdev added bug Something isn't working p2 labels Jun 19, 2024
@alejsdev alejsdev changed the title fix: allow_inf_nan option for Param subclasses 🐛 Allow_inf_nan option for Param subclasses Jun 19, 2024
@svlandeg svlandeg self-assigned this Aug 19, 2024
@svlandeg
Copy link
Copy Markdown
Member

Thanks for the bug report and the fix, @pat-lasswell! Appreciate the detailed explanation in the original discussion and the extensive unit tests that you added here.

While this fix seems right, the newer PR #11867 covers a similar issue with Body, so I would be inclined to merge that one instead of this one. I'll leave the final decision with Tiangolo. Thanks again! 🙏

@tiangolo
Copy link
Copy Markdown
Member

Thanks for your contribution @pat-lasswell! 🚀

And thanks for the help here everyone. ☕

This was covered in #11867, so I'll close this one now, but thanks for the effort! 🍰

It will be available in FastAPI 0.112.2 released in the next few hours. 🎉

@tiangolo tiangolo closed this Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Query fields with numeric types appear not to enforce allow_inf_nan=False, whereas gt=0, etc are enforced

7 participants